From 42fb8653db3ec04f0c7ce2f8d8b9691491954da9 Mon Sep 17 00:00:00 2001 From: Josh Kasten Date: Wed, 15 Nov 2023 23:36:46 +0000 Subject: [PATCH] fix race condition getting wrong sw registration If you attempt to get the service worker registration immediately after registering it the browser can give you a different registration than exepected. This is due to the fact the new registeration may not be ready yet and there migth be sw in scope at a higher level. The problem noted above cause the OneSignal SDK to register for a push token on the wrong service worker. This in turn causes the SDK to attempt to create a user with a push susbcription but without a push token, which results in a 400 error on POST /users. To fix, instead of calling navigator.serviceWorker.getRegistration we are now simply using the ServiceWorkerRegistration give to us by navigator.serviceWorker.register to avoid this race condition completely --- src/shared/helpers/SubscriptionHelper.ts | 2 +- src/shared/managers/ServiceWorkerManager.ts | 22 +++++++++++++++------ src/shared/managers/SubscriptionManager.ts | 7 +++---- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/shared/helpers/SubscriptionHelper.ts b/src/shared/helpers/SubscriptionHelper.ts index 6e864c56a..f772a7ae4 100755 --- a/src/shared/helpers/SubscriptionHelper.ts +++ b/src/shared/helpers/SubscriptionHelper.ts @@ -41,7 +41,7 @@ export default class SubscriptionHelper { await PermissionUtils.triggerNotificationPermissionChanged(); await EventHelper.checkAndTriggerSubscriptionChanged(); } catch (e) { - Log.info(e); + Log.error(e); } break; case WindowEnvironmentKind.OneSignalSubscriptionPopup: { diff --git a/src/shared/managers/ServiceWorkerManager.ts b/src/shared/managers/ServiceWorkerManager.ts index 373de242c..c5af914ce 100644 --- a/src/shared/managers/ServiceWorkerManager.ts +++ b/src/shared/managers/ServiceWorkerManager.ts @@ -174,6 +174,8 @@ export class ServiceWorkerManager { }); } + // Returns true if a the OneSignal service worker can't be installed + // or is already installed and doesn't need updating. private async shouldInstallWorker(): Promise { // 1. Does the browser support ServiceWorkers? if (!Environment.supportsServiceWorkers()) return false; @@ -486,9 +488,11 @@ export class ServiceWorkerManager { * additional query parameters, but this must then stay consistent. */ - public async installWorker() { + public async installWorker(): Promise< + ServiceWorkerRegistration | undefined | null + > { if (!(await this.shouldInstallWorker())) { - return; + return this.getRegistration(); } Log.info('Installing worker...'); @@ -512,8 +516,10 @@ export class ServiceWorkerManager { Log.info( `[Service Worker Installation] Installing service worker ${workerHref} ${scope}.`, ); + + let registration: ServiceWorkerRegistration; try { - await navigator.serviceWorker.register(workerHref, { scope }); + return await navigator.serviceWorker.register(workerHref, { scope }); } catch (error) { Log.error( `[Service Worker Installation] Installing service worker failed ${error}`, @@ -527,14 +533,15 @@ export class ServiceWorkerManager { throw error; } - await this.fallbackToUserModelBetaWorker(); + registration = await this.fallbackToUserModelBetaWorker(); } Log.debug(`[Service Worker Installation] Service worker installed.`); await this.establishServiceWorkerChannel(); + return registration; } - async fallbackToUserModelBetaWorker() { + async fallbackToUserModelBetaWorker(): Promise { const BETA_WORKER_NAME = 'OneSignalSDK.sw.js'; const configWithBetaWorkerName: ServiceWorkerManagerConfig = { @@ -557,7 +564,9 @@ export class ServiceWorkerManager { ); try { - await navigator.serviceWorker.register(workerHref, { scope }); + const registration = await navigator.serviceWorker.register(workerHref, { + scope, + }); const DEPRECATION_ERROR = ` [Service Worker Installation] Successfully installed v16 Beta Worker. @@ -568,6 +577,7 @@ export class ServiceWorkerManager { `; Log.error(DEPRECATION_ERROR); + return registration; } catch (error) { const response = await fetch(workerHref); if (response.status === 403 || response.status === 404) { diff --git a/src/shared/managers/SubscriptionManager.ts b/src/shared/managers/SubscriptionManager.ts index bb8c28016..118428b92 100644 --- a/src/shared/managers/SubscriptionManager.ts +++ b/src/shared/managers/SubscriptionManager.ts @@ -487,8 +487,10 @@ export class SubscriptionManager { } /* Now that permissions have been granted, install the service worker */ + let workerRegistration: ServiceWorkerRegistration | undefined | null; try { - await this.context.serviceWorkerManager.installWorker(); + workerRegistration = + await this.context.serviceWorkerManager.installWorker(); } catch (err) { if (err instanceof ServiceWorkerRegistrationError) { if (err.status === 403) { @@ -506,9 +508,6 @@ export class SubscriptionManager { throw err; } - Log.debug('[Subscription Manager] Getting OneSignal service Worker...'); - const workerRegistration = - await this.context.serviceWorkerManager.getRegistration(); if (!workerRegistration) { throw new Error('OneSignal service worker not found!'); }