Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalizing a timeout API #1292

Open
asakusuma opened this issue Mar 23, 2018 · 8 comments
Open

Formalizing a timeout API #1292

asakusuma opened this issue Mar 23, 2018 · 8 comments

Comments

@asakusuma
Copy link

asakusuma commented Mar 23, 2018

It looks like the idea has already been brought up and is already implemented in some fashion in at least Firefox and Chrome, but it would be nice if there was some spec guarantee on guarding async operations with a timeout, and some way to customize behavior.

In theory, it's not hard to create a service worker than can never be removed, by creating some aysnc operation that can never finish:

e.waitUntil(new Promise(() => {}));
addEventListener('install', (e) => {
  e.waitUntil(registration.unregister());
});

A few ideas for public APIs to prevent these issues and provide fine-grain control:

Additional timeout param for waitUntil/respondWith

// Abort if promise doesn't resolve within 5 seconds.
e.waitUntil(new Promise(() => {}), { timeout: 5000 });

API for adding a default timeout for functional events

Not quite sure where this could be done. Perhaps during worker registration? I feel like adding a worker specific option to addEventListener would be too intrusive.

Allow installing worker to cutoff active worker

Specifically, allow skipWaiting to provide an “ultimatum” timeout to the active worker to finish any async operations.

// Give the active worker 5 seconds to terminate any operations before killing
self.skipWaiting({
  gracePeriod: 5000
});
@jakearchibald
Copy link
Contributor

I don't think we need a timeout for waitUntil, as you can do that with Promise.race already. Something like skipWaiting({ force: true }) is interesting though.

@asakusuma
Copy link
Author

@jakearchibald true. A respondWith/waitUntil timeout can certainly be constructed manually, but if a big enough percentage of implementations are guarding respondWith/waitUntil with a timeout, it might make sense to consider adding it as a built-in feature.

At the end of the day, it would be nice to have some sort of built-in safeguard against locking up a service worker with a never-ending operation. If we just pick one safeguard, I think skipWaiting({ force: true }) approach makes the most sense, as it doesn't require preemptively guarding.

@mfalken
Copy link
Member

mfalken commented Oct 10, 2018

Agree skipWaiting(timeout) or skipWaiting(force) would be interesting. Chrome has a built-in timeout, as mentioned in #1295

@asakusuma
Copy link
Author

It would also be nice if the skipWaiting could be executed from any context (window, active worker, or installing worker). In other words, any part of the system should be able to force update to the latest worker within a certain time guarantee.

@mfalken
Copy link
Member

mfalken commented Oct 27, 2018

F2F: This and related issues was discussed at the F2F, see a summary at #1303 (comment)

I'll add that if we're removing resurrection then having unregister({force: true}) would provide a means of avoiding the new worker stomping on the storage of an old but still used worker.

@asakusuma
Copy link
Author

At the F2F, we also discussed a built-in API for adding a timeout for waitUntil. Both end-users represented are already doing this manually (facebook and linkedin).

cc @n8schloss

@annevk
Copy link
Member

annevk commented Oct 30, 2018

Would it make sense to reuse AbortController here in some fashion? Is there a more formal description of skipWaiting somewhere?

@asakusuma
Copy link
Author

@annevk sorry I neglected to respond to your comment. Not sure it makes sense to use AbortController since skipWaiting() isn't really about a request. Although you could definitely describe skipWaiting() as a way to "abort" the existing worker.

The only formal description of skipWaiting I've seen is in the spec. The tl;dr: is

  1. Set the skipWaiting flag
  2. Run the try activate algorithm, which will actually activate once there are no pending events in the existing worker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants
@jakearchibald @asakusuma @annevk @mfalken and others