Skip to content

Commit

Permalink
feat(service-worker): recover from EXISTING_CLIENTS_ONLY mode when …
Browse files Browse the repository at this point in the history
…there is a valid update (#31865)

Previously, when the ServiceWorker entered a degraded mode
(`EXISTING_CLIENTS_ONLY` or `SAFE_MODE`) it would remain in that mode
for the rest of the lifetime of ServiceWorker instance. Note that
ServiceWorkers are stopped by the browser after a certain period of
inactivity and a new instance is created as soon as the ServiceWorker
needs to handle an event (such as a request from the page). Those new
instances would start from the `NORMAL` mode.

The reason for this behavior is to err on the side of caution: If we
can't be sure why the ServiceWorker entered the degraded mode, it is
risky to try recovering on the same instance and might lead to
unexpected behavior.

However, it turns out that the `EXISTING_CLIENTS_ONLY` mode can only be
a result of some error happening with the latest version (e.g. a hash
mismatch in the manifest). Therefore, it is safe to recover from that
mode once a new, valid update is successfully installed and to start
accepting new clients.

This commit ensures that the mode is set back to `NORMAL`, when (a) an
update is successfully installed and (b) the current mode is
`EXISTING_CLIENTS_ONLY`.

Besides making the behavior more predictable (instead of relying on the
browser to decide when to terminate the current ServiceWorker instance
and create a new one), this change can also improve the developer
experience:
When people notice the error during debugging and fix it by deploying a
new version (either to production or locally), it is confusing that the
ServiceWorker will fetch and install the update (as seen by the requests
in the Network panel in DevTools) but not serve it to clients. With this
change, the update will be served to new clients as soon as it is
installed.

Fixes #31109

PR Close #31865
  • Loading branch information
gkalpak authored and mhevery committed Sep 4, 2019
1 parent bda2b4e commit 094538c
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 10 deletions.
26 changes: 16 additions & 10 deletions packages/service-worker/worker/src/driver.ts
Expand Up @@ -802,8 +802,15 @@ export class Driver implements Debuggable, UpdateSource {
// Future new clients will use this hash as the latest version.
this.latestHash = hash;

// If we are in `EXISTING_CLIENTS_ONLY` mode (meaning we didn't have a clean copy of the last
// latest version), we can now recover to `NORMAL` mode and start accepting new clients.
if (this.state === DriverReadyState.EXISTING_CLIENTS_ONLY) {
this.state = DriverReadyState.NORMAL;
this.stateMessage = '(nominal)';
}

await this.sync();
await this.notifyClientsAboutUpdate();
await this.notifyClientsAboutUpdate(newVersion);
}

async checkForUpdate(): Promise<boolean> {
Expand Down Expand Up @@ -959,19 +966,19 @@ export class Driver implements Debuggable, UpdateSource {

async lookupResourceWithoutHash(url: string): Promise<CacheState|null> {
await this.initialized;
const version = this.versions.get(this.latestHash !) !;
return version.lookupResourceWithoutHash(url);
const version = this.versions.get(this.latestHash !);
return version ? version.lookupResourceWithoutHash(url) : null;
}

async previouslyCachedResources(): Promise<string[]> {
await this.initialized;
const version = this.versions.get(this.latestHash !) !;
return version.previouslyCachedResources();
const version = this.versions.get(this.latestHash !);
return version ? version.previouslyCachedResources() : [];
}

recentCacheStatus(url: string): Promise<UpdateCacheStatus> {
const version = this.versions.get(this.latestHash !) !;
return version.recentCacheStatus(url);
async recentCacheStatus(url: string): Promise<UpdateCacheStatus> {
const version = this.versions.get(this.latestHash !);
return version ? version.recentCacheStatus(url) : UpdateCacheStatus.NOT_CACHED;
}

private mergeHashWithAppData(manifest: Manifest, hash: string): {hash: string, appData: Object} {
Expand All @@ -981,11 +988,10 @@ export class Driver implements Debuggable, UpdateSource {
};
}

async notifyClientsAboutUpdate(): Promise<void> {
async notifyClientsAboutUpdate(next: AppVersion): Promise<void> {
await this.initialized;

const clients = await this.scope.clients.matchAll();
const next = this.versions.get(this.latestHash !) !;

await clients.reduce(async(previous, client) => {
await previous;
Expand Down
16 changes: 16 additions & 0 deletions packages/service-worker/worker/test/happy_spec.ts
Expand Up @@ -1269,6 +1269,22 @@ import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope';
brokenLazyServer.assertNoOtherRequests();
});

it('recovers from degraded `EXISTING_CLIENTS_ONLY` mode as soon as there is a valid update',
async() => {
await driver.initialized;
expect(driver.state).toBe(DriverReadyState.NORMAL);

// Install a broken version.
scope.updateServerState(brokenServer);
await driver.checkForUpdate();
expect(driver.state).toBe(DriverReadyState.EXISTING_CLIENTS_ONLY);

// Install a good version.
scope.updateServerState(serverUpdate);
await driver.checkForUpdate();
expect(driver.state).toBe(DriverReadyState.NORMAL);
});

it('ignores invalid `only-if-cached` requests ', async() => {
const requestFoo = (cache: RequestCache | 'only-if-cached', mode: RequestMode) =>
makeRequest(scope, '/foo.txt', undefined, {cache, mode});
Expand Down

0 comments on commit 094538c

Please sign in to comment.