fix(service-worker): treat 503 as offline (#35595)

Prior to this commit the service worker only treated 504 errors as "effectively offline".
This commit changes the behaviour to treat both 503 (Service Unavailable) and 504 as "offline".

Fixes #35571

PR Close #35595
This commit is contained in:
Alex Wiese 2020-02-24 12:04:22 +00:00 committed by Miško Hevery
parent 24f32e373d
commit 96cdf035d8
2 changed files with 18 additions and 3 deletions

View File

@ -684,7 +684,7 @@ export class Driver implements Debuggable, UpdateSource {
/** /**
* Retrieve a copy of the latest manifest from the server. * Retrieve a copy of the latest manifest from the server.
* Return `null` if `ignoreOfflineError` is true (default: false) and the server or client are * Return `null` if `ignoreOfflineError` is true (default: false) and the server or client are
* offline (detected as response status 504). * offline (detected as response status 503 (service unavailable) or 504 (gateway timeout)).
*/ */
private async fetchLatestManifest(ignoreOfflineError?: false): Promise<Manifest>; private async fetchLatestManifest(ignoreOfflineError?: false): Promise<Manifest>;
private async fetchLatestManifest(ignoreOfflineError: true): Promise<Manifest|null>; private async fetchLatestManifest(ignoreOfflineError: true): Promise<Manifest|null>;
@ -695,7 +695,7 @@ export class Driver implements Debuggable, UpdateSource {
if (res.status === 404) { if (res.status === 404) {
await this.deleteAllCaches(); await this.deleteAllCaches();
await this.scope.registration.unregister(); await this.scope.registration.unregister();
} else if (res.status === 504 && ignoreOfflineError) { } else if ((res.status === 503 || res.status === 504) && ignoreOfflineError) {
return null; return null;
} }
throw new Error(`Manifest fetch failed! (status: ${res.status})`); throw new Error(`Manifest fetch failed! (status: ${res.status})`);

View File

@ -12,7 +12,7 @@ import {Driver, DriverReadyState} from '../src/driver';
import {AssetGroupConfig, DataGroupConfig, Manifest} from '../src/manifest'; import {AssetGroupConfig, DataGroupConfig, Manifest} from '../src/manifest';
import {sha1} from '../src/sha1'; import {sha1} from '../src/sha1';
import {MockCache, clearAllCaches} from '../testing/cache'; import {MockCache, clearAllCaches} from '../testing/cache';
import {MockRequest} from '../testing/fetch'; import {MockRequest, MockResponse} from '../testing/fetch';
import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock'; import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTableForFs} from '../testing/mock';
import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
@ -872,6 +872,21 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
expect(await scope.caches.keys()).not.toEqual([]); expect(await scope.caches.keys()).not.toEqual([]);
}); });
it('does not unregister or change state when status code is 503 (service unavailable)',
async() => {
expect(await makeRequest(scope, '/foo.txt')).toEqual('this is foo');
await driver.initialized;
spyOn(server, 'fetch').and.callFake((req: Request) => new MockResponse(null, {
status: 503,
statusText: 'Service Unavailable'
}));
expect(await driver.checkForUpdate()).toEqual(false);
expect(driver.state).toEqual(DriverReadyState.NORMAL);
expect(scope.unregistered).toBeFalsy();
expect(await scope.caches.keys()).not.toEqual([]);
});
describe('cache naming', () => { describe('cache naming', () => {
// Helpers // Helpers
const cacheKeysFor = (baseHref: string) => const cacheKeysFor = (baseHref: string) =>