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

Describe process for subscription updates #132

Closed
martinthomson opened this issue Mar 25, 2015 · 12 comments
Closed

Describe process for subscription updates #132

martinthomson opened this issue Mar 25, 2015 · 12 comments
Assignees

Comments

@martinthomson
Copy link
Member

Any time a subscription needs to change, we signal that with the subscriptionchange event. Some of these cases won't be as a result of a break and it would be good to describe how a subscription is seamlessly updated.

I think that the process is simple:

  1. Determine that a subscription update is needed for any reason. This might be expiration of the subscription, or a desire to roll out the old encryption keys (see Force confidentiality and integrity protection (encryption) #55).
  2. Create a new subscription with the new parameters, while leaving the old subscription in place.
  3. Fire the event as an ExtendableEvent.
  4. Await the waitUntil promise if that was set by the application.
  5. Clean up the state from the old subscription.

Note that the new subscription can share properties with the old subscription; but this process is necessary for any change where the application might have to share this information with the application server. Keeping the old information around until the application has confirmed that this has happened means that there won't need to be messages lost. We might also note that sometimes it can take some time for the update to happen.

@johnmellor
Copy link
Contributor

Currently the pushsubscriptionchange event is not specced to automatically create a new subscription, instead the spec just says:

A Service Worker SHOULD attempt to resubscribe while handling this event, in order to continue receiving push messages.

But it's unclear how an author can "resubscribe". Should they unsubscribe then subscribe again, or just call subscribe again?

A possible process for reliably updating the subscription, even on flaky networks, would be:

  1. Push service informs UA that the subscription will soon be invalidated.
  2. UA wakes up SW and fires pushsubscriptionchange.
  3. Calls to PushManager.getSubscription will still resolve with the old subscription for now.
  4. The next time PushManager.subscribe is called, the subscription stored by the UA is replaced by the updated subscription, and the subscribe promise is resolved with this new subscription. Subsequent calls to getSubscription will resolve with the new subscription.
  5. The website should now transmit the updated subscription to its server, retrying across restarts until it succeeds.
  6. Both the old and new subscriptions are simultaneously stored by the push service, and messages can be sent using either. But once the new subscription is used for the first time, the old subscription will be deleted from the push service (after a short timeout, to allow for propagation delay in the application server(s)).
  7. The old subscription will eventually expire even if no new subscription is made, or the new subscription is never used, but only after a generous grace period (weeks? months?).

Concretely, this would require changes like the following to the Push API:

  1. PushManager.subscribe method steps:
  • Replace "If the Service Worker is already subscribed" with "If the Service Worker is already subscribed, and the subscription is not marked as will-be-deactivated", and make successful subscription overwrite the will-be-deactivated subscription.

11.2 The pushsubscriptionchange event:

  • Replace "A Service Worker SHOULD attempt to resubscribe while handling this event, in order to continue receiving push messages" with "In order to continue receiving push messages, once this event has been received the webapp SHOULD call subscribe again, and update the subscription stored by its application server. Since the webapp may need to retry many times to transmit the updated subscription to its application server, the push service SHOULD continue to accept messages using the old subscription until messages start being sent using the new subscription".
  • Replace "If the previous push subscription is still active, perform the following steps in parallel: (...) Unsubscribe oldSubscription." with "If the previous push subscription is still active, mark it as will-be-deactivated". Accordingly, pushsubscriptionchange would no longer need to be an ExtendableEvent.

See also #120 and #116.

@mvano @beverloo @gauntface @kitcambridge

@ghost
Copy link

ghost commented Feb 26, 2016

I definitely agree this is underspecified. I interpreted that sentence as "it's up to the worker to call pushManager.subscribe() again, and send the new details to its app server." Forgetting to handle the event is a footgun, but, even if we resubscribed automatically, the worker would still need to send up the new subscription endpoint and keys.

If I understand your proposal correctly, that's why we'd need to store both subscriptions, until the new one is used for the first time. I think we'd still need an ExtendableEvent, though, unless there's another point at which the new details can be sent (the next time the page is reloaded and calls serviceWorkerRegistration.pushManager.subscribe()?)

I do have some concerns about this proposal, though:

  • The server may decide to drop all subscriptions for a particular browser. We're considering using this as a way to avoid excessive queueing of messages for a browser that's offline. In that case, the old URLs immediately cease to be valid, so there's no opportunity for a grace period.
  • The browser may fire pushsubscriptionchange for reasons other than a subscription expiring on the server. Currently, Firefox fires this event if the user revokes and then restores permission, or if they visit a site that previously exceeded the background message quota. In that case, the subscription is removed from the server immediately, and might not be restored for some time.
  • This incurs additional storage costs on the server, which now needs to maintain two subscriptions for a particular worker, along with state linking the two. In addition, until the worker sends up the new subscription, the app server can't transition to the new one. There should be no reason to retain the older, now invalid subscription URL.

This feels complex, but that's probably because I'm missing a key use case. It sounds like you're primarily concerned about the app failing to resubscribe due to network errors, and leaving the app server unable to send messages. Is that accurate?

+@bbangert @jrconlin

@martinthomson
Copy link
Member Author

So @johnmellor proposes an event with a different name, I think. That is, pushsubscriptiongoing_or_gone, perhaps (underscores added so that it didn't say "go in gorgon", which I found confusing).

I think that @johnmellor's proposal is more in the spirit of what we've learned regarding robustness. That is, that we let the application know as soon as possible when something is wrong, or might soon be. Then we have:

  1. subscription is gone: application can try to subscribe again, though this might fail; there is a narrow window of outage where messages won't be delivered or even stored
  2. subscription is going: application can try to subscribe again; if this succeeds, then there is no outage

The key here is that the action is the same. It is good to have most applications apply a consistent action for the same event. While getSubscription will reveal which of these states apply, is there any value in having a boolean flag?

Note that there is a small risk of misconception about this event. It might be perceived as an "offline" event. We can't trigger this event when we are offline, because offline is a normal part of the process. We need some sort of signal that the subscription is busted and we likely only get that while online. Text that explains this might help too.

@mohamedhafez
Copy link

mohamedhafez commented Jun 10, 2016

Specifically regarding the situation when a subscription needs to be refreshed, for no other reason than that the user has been subscribed for a long time, I'm worried about failures interrupting service for long term users that depend on the notifications. What if theres a network error when pushsubscriptionchange gets called? Every developer that wanted to ensure their service was reliable could put in code to catch that and then retry, and also to restart the retrying process in case of a reboot, but that seems like a lot of people repeating the same work, and probably getting it wrong half the time. The result would be a poorer end-user experience and developer frustration.

Perhaps there could be an option for the serviceworker to specify a URL to hit in order to notify a server of the refresh, that would automatically get retried for a few days until it got a 200 response? And both the old subscription and new subscription could remain valid until the server responded with a 200 so we are sure it knows about the change, at which point the old subscription could be invalidated?

@ghost
Copy link

ghost commented Jun 10, 2016

@mohamedhafez You bring up a good point, and I think it's relevant for the other cases, too. We don't want developers rolling their own backoff and retry logic for pushsubscriptionchange, and a web hook is in line with features like the CSP error reporting mechanism.

Another approach is for the browser is to retrigger pushsubscriptionchange with exponential backoff if the promise passed to pushSubscriptionChangeEvent.waitUntil() rejects. That way, scripts could implement logic for updating local state, too. Though, unscrupulous sites could game it by returning a rejected promise so that the worker periodically wakes up in the background.

@mohamedhafez
Copy link

@kitcambridge I think the best solution would be the following: there could be a general ServiceWorker function that takes a webhook as its parameter, and auto-retries it in a standard and reliable way. In the pushsubscriptionchange event, the developer would pass their particular webhook to this function, and then take care of any local state stuff. pushsubscriptionchange only gets triggered once to prevent abuse, but the webhook still gets retried reliably by the function.

@jrconlin
Copy link

What prevents evilonastick.com from setting a web hook that points to
kittens.com? There are legit reasons a site may pick a different domain for
user management.
On Jun 10, 2016 8:30 PM, "Mohamed Hafez" [email protected] wrote:

@kitcambridge https://github.com/kitcambridge I think the best solution
would be the following: there could be a general ServiceWorker function
that takes a webhook as its parameter, and auto-retries it in a standard
and reliable way. In the pushsubscriptionchange event, the developer
would pass their particular webhook to this function, and then take care of
any local state stuff. pushsubscriptionchange only gets triggered once to
prevent abuse, but the webhook still gets retried reliably by the function.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#132 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AACLq1Az3NXOCsThTayuNumFcfLV1dx4ks5qKg_HgaJpZM4D0OXG
.

@mohamedhafez
Copy link

mohamedhafez commented Jun 11, 2016

Good point, though I assume there is already some kind of policy in place to keep ServiceWorkers from launching/retrying tons of requests to other domains, since this issue wouldn't be unique to webhooks, right?

I'm not proposing giving any new abilities to the ServiceWorker when I'm talking about auto-retrying webhooks, I'm just proposing that an easy, reliable, standardized way be provided to do a common and hard-to-get-right task.

@martinthomson
Copy link
Member Author

There are no good solutions here without perfect prescience. The browser should (and probably will) notify in advance of expiration events. Though I don't believe that anyone plans these things enough in advance that we will have a usable warning (or that we will bother to build mechanisms to support that advance warning). More likely, we will only break a subscription rarely, but accept the outage when we do.

I say this on the basis that a wholly reliable push service is not something that applications should expect. I don't know what the actual numbers are, but I've heard reports that even the best-performing push services are not reliable enough to use as the sole means of synchronizing application state. So applications will always have to have ways to deal with desynchronization.

The background sync API in its more complete incarnation is a better answer for the robustness issue, I think.

@benlast
Copy link

benlast commented Oct 31, 2016

Excuse the late response... @martinthomson commented above that the background Sync API is an answer to the robustness issue. However, WICG/background-sync#114 indicates that a service worker may not register for sync if it has no open clients (and if I'm understanding https://wicg.github.io/BackgroundSync/spec/#syncmanager correctly, that's also in the spec). In which case, retry of a failed pushSubscriptionChange via sync isn't possible, unless the UA guarantees only to deliver pushSubscriptionChange events to a service worker when there is a client page open for it.

@martinthomson
Copy link
Member Author

@benlast, maybe we should take that up with the background sync folks. This seems like a reasonable time to request a sync.

@beverloo
Copy link
Member

I think PR #234 captures the gist of this issue, and specifically advocates for use of background sync to avoid rolling our own, duplicated logic. Issue #240 tracks the necessary changes to the background sync specification. By including newSubscription in the event, we've also defined that the user agent is responsible for actually updating the subscription with the push service.

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

No branches or pull requests

6 participants