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

Sync push subscriptions on every page load #370

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Jul 29, 2023

Close #354 (hopefully)

I tested this code using this patch:

diff --git a/sw/index.js b/sw/index.js
index c8dbee1..38c41ed 100644
--- a/sw/index.js
+++ b/sw/index.js
@@ -86,10 +86,12 @@ async function handlePushSubscriptionChange (oldSubscription, newSubscription) {
   newSubscription ??= await self.registration.pushManager.getSubscription()
   if (!newSubscription) {
     // no subscription exists at the moment
+    console.log('no active push subscription found')
     return
   }
   if (oldSubscription?.endpoint === newSubscription.endpoint) {
     // subscription did not change. no need to sync with server
+    console.log('push subscription did not change. no need for sync. skipping')
     return
   }
   // convert keys from ArrayBuffer to string
@@ -107,6 +109,7 @@ async function handlePushSubscriptionChange (oldSubscription, newSubscription) {
     }
   }`
   const body = JSON.stringify({ query, variables })
+  console.log('syncing new push subscription with server:', newSubscription)
   await fetch('/api/graphql', {
     method: 'POST',
     headers: {
@@ -118,5 +121,6 @@ async function handlePushSubscriptionChange (oldSubscription, newSubscription) {
 }

 self.addEventListener('pushsubscriptionchange', (event) => {
+  console.log('pushsubscriptionchange fired')
   event.waitUntil(handlePushSubscriptionChange(event.oldSubscription, event.newSubscription))
 }, false)

You can then test the combination of page reloads, manually calling navigator.serviceWorker.controller.postMessage({ action: 'SYNC_SUBSCRIPTION' }) and manually deleting the entry in IndexedDB to trigger synchronization.

However, this solution is not perfect as mentioned in this article which was used for reference:

This solution is not perfect, the user could lose some push notifications if he doesn’t open the webapp for a long time.

The article also mentions usage of background sync to also trigger (delayed) synchronization if the user loads a page which is available offline:

If the page could be loaded offline, then it’s possible to use background sync API to refresh subscription when browser will be online again.

However, we don't have pages which users would regularly visit and are available offline at the moment. I also don't think adding background sync is worth it at the moment. I think the current changes should already fix 99% of cases why push subscriptions are lost.

@ekzyis ekzyis force-pushed the 354-push-subscriptions-are-lost branch 2 times, most recently from 5d058cf to 1c97caf Compare July 30, 2023 21:53
@ekzyis ekzyis added the bug label Aug 2, 2023
Most browsers don't support the pushsubscriptionchange event.

We workaround this by saving the current push subscription in IndexedDB so we can check during every page load if the push subscription changed.

If that is the case, we manually sync the push subscription with the server.

However, this solution is not perfect as mentioned in https://medium.com/@madridserginho/how-to-handle-webpush-api-pushsubscriptionchange-event-in-modern-browsers-6e47840d756f which was used for reference:

> This solution is not perfect, the user could lose some push notifications if he doesn’t open the webapp for a long time.
@huumn huumn force-pushed the 354-push-subscriptions-are-lost branch from 1c97caf to 32a14cb Compare August 8, 2023 00:58
@huumn huumn merged commit e3c60d1 into stackernews:master Aug 8, 2023
1 check passed
@ekzyis ekzyis deleted the 354-push-subscriptions-are-lost branch August 9, 2023 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Push subscriptions are lost
2 participants