Skip to content

Commit

Permalink
fix race condition getting wrong sw registration
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jkasten2 committed Nov 16, 2023
1 parent 5af7365 commit 42fb865
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 11 deletions.
2 changes: 1 addition & 1 deletion src/shared/helpers/SubscriptionHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
22 changes: 16 additions & 6 deletions src/shared/managers/ServiceWorkerManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean> {
// 1. Does the browser support ServiceWorkers?
if (!Environment.supportsServiceWorkers()) return false;
Expand Down Expand Up @@ -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...');
Expand All @@ -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}`,
Expand All @@ -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<ServiceWorkerRegistration> {
const BETA_WORKER_NAME = 'OneSignalSDK.sw.js';

const configWithBetaWorkerName: ServiceWorkerManagerConfig = {
Expand All @@ -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.
Expand All @@ -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) {
Expand Down
7 changes: 3 additions & 4 deletions src/shared/managers/SubscriptionManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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!');
}
Expand Down

0 comments on commit 42fb865

Please sign in to comment.