Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,12 @@
},
{
"path": "./build/releases/OneSignalSDK.page.es6.js",
"limit": "42.4 kB",
"limit": "42.7 kB",
"gzip": true
},
{
"path": "./build/releases/OneSignalSDK.sw.js",
"limit": "12.354 kB",
"limit": "12.65 kB",
"gzip": true
},
{
Expand Down
7 changes: 6 additions & 1 deletion preview/OneSignalSDKWorker.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
importScripts('https://localhost:4001/sdks/web/v16/Dev-OneSignalSDK.sw.js');
// Use self.location.origin so the worker imports from whichever origin
// served it (localhost during local dev, an ngrok/Cloudflare tunnel during
// device testing, etc.). Hardcoding localhost:4001 breaks any test that
// loads this worker from a non-localhost origin (the inner script is
// fetched as part of registration and produces a NetworkError otherwise).
importScripts(`${self.location.origin}/sdks/web/v16/Dev-OneSignalSDK.sw.js`);

// For testing on staging
// importScripts(
Expand Down
76 changes: 76 additions & 0 deletions preview/pageA.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>SDK-4336 Repro: Page A</title>
<link rel="shortcut icon" href="#" />
<link rel="manifest" href="manifest.json" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="apple-mobile-web-app-status-bar-style" content="default" />
<meta name="apple-mobile-web-app-title" content="SDK4336" />

<script src="sdks/web/v16/Dev-OneSignalSDK.page.js" defer></script>

<script>
console.log('!!!! [SDK-4336 PAGE A] OneSignal initialize');

function getUrlQueryParam(name) {
const urlParams = new URLSearchParams(window.location.search);
return urlParams.get(name);
}

// Resolve the app_id robustly. The in-page links to Page B/Page A don't
// carry the query string, and on an iOS standalone (home-screen) PWA the
// localStorage fallback isn't reliable across A->B->A navigations because
// storage is partitioned per context. Without a hardcoded fallback, the
// B->A load calls OneSignal.init with appId=null -> InvalidAppIdError.
const DEFAULT_APP_ID = 'b79087eb-8531-4d2d-a6f5-726f797891c7';
let appId = getUrlQueryParam('app_id');
try {
if (appId) localStorage.setItem('sdk4336_app_id', appId);
else appId = localStorage.getItem('sdk4336_app_id');
} catch (e) {}
appId = appId || DEFAULT_APP_ID;

const SERVICE_WORKER_PATH = 'push/onesignal/';

var OneSignalDeferred = window.OneSignalDeferred || [];
OneSignalDeferred.push(async function (OneSignal) {
const t0 = performance.now();
await OneSignal.init({
appId,
serviceWorkerParam: { scope: '/' + SERVICE_WORKER_PATH },
serviceWorkerPath: SERVICE_WORKER_PATH + 'OneSignalSDKWorker.js',
});
const elapsed = Math.round(performance.now() - t0);
console.log(`!!!! [SDK-4336 PAGE A] OneSignal initialized (${elapsed}ms)`);
});

async function requestNotificationPermission() {
window.OneSignalDeferred = window.OneSignalDeferred || [];
OneSignalDeferred.push(async function (OneSignal) {
await OneSignal.Notifications.requestPermission();
});
}
</script>
</head>
<body>
<h1>SDK-4336 Repro: Page A</h1>
<p>
<a href="./pageB.html">Go to Page B</a>
</p>
<p>
<button onclick="requestNotificationPermission()">Register</button>
</p>
<p>Repro steps (per the ticket):</p>
<ol>
<li>Add to Home Screen and open Page A.</li>
<li>Tap "Go to Page B".</li>
<li>Tap "Go to Page A".</li>
<li>Tap "Register" and accept the prompt.</li>
<li>Tap "Go to Page B".</li>
<li>Tap "Go to Page A" &mdash; <code>init()</code> should hang.</li>
</ol>
</body>
</html>
23 changes: 23 additions & 0 deletions preview/pageB.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>SDK-4336 Repro: Page B</title>
<link rel="shortcut icon" href="#" />
<link rel="manifest" href="manifest.json" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="apple-mobile-web-app-status-bar-style" content="default" />
<meta name="apple-mobile-web-app-title" content="SDK4336" />
</head>
<body>
<h1>SDK-4336 Repro: Page B</h1>
<p>
<a href="./pageA.html">Go to Page A</a>
</p>
<p>
This page intentionally does <em>not</em> initialize the SDK. It just navigates back to Page A
so we can exercise the multi-page navigation flow described in SDK-4336.
</p>
</body>
</html>
2 changes: 1 addition & 1 deletion preview/push/onesignal/OneSignalSDKWorker.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
importScripts('https://localhost:4001/sdks/web/v16/Dev-OneSignalSDK.sw.js');
importScripts(`${self.location.origin}/sdks/web/v16/Dev-OneSignalSDK.sw.js`);

// For testing on staging
// importScripts(
Expand Down
39 changes: 39 additions & 0 deletions preview/vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,25 @@ export default defineConfig({
// SDK fetch URL (e.g. `http://localhost:4000/...` in HTTP mode) lands here.
port: useHttps ? 4001 : 4000,
strictPort: true,
// HMR's WebSocket targets `wss://localhost:<port>`, which iOS devices on a
// tunnel (ngrok, etc.) can't reach. Disabling stops the runaway
// reconnect loop that floods the console with `ws.send` rejections.
hmr: false,
},
plugins: [
{
// Vite's dev server injects `<script src="/@vite/client">` into every
// HTML response, even when `server.hmr` is false. That client then
// retries a WebSocket forever, which on a tunneled iOS device floods
// the console with `ws.send` rejections and is heavy enough to
// skew our SDK timing. Strip it out for the preview server, where
// we don't need HMR.
name: 'strip-vite-client',
enforce: 'post',
transformIndexHtml(html) {
return html.replace(/<script[^>]*src="\/@vite\/client"[^>]*><\/script>\s*/g, '');
Comment thread
sherwinski marked this conversation as resolved.
Dismissed
},
},
...(useHttps
? [
mkcert({
Expand Down Expand Up @@ -87,9 +104,31 @@ export default defineConfig({
if (contentType) {
res.setHeader('Content-Type', contentType);
}
// Avoid stale SDK bundles being served from the browser HTTP cache,
// which is especially aggressive on iOS Safari/PWA. Without this,
// rebuilding the SDK during a debug session won't take effect on the
// device until you fully wipe website data.
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
res.setHeader('Pragma', 'no-cache');
fs.createReadStream(filePath).pipe(res);
});
},
},
{
// Same hygiene for the sandbox HTML (pageA/pageB/index) and root SW
// file. Vite's default dev-server cache policy is fine for source files
// but our HTML directly references the SDK and embeds repro state.
name: 'no-store-html',
configureServer(server) {
server.middlewares.use((req, res, next) => {
const url = req.url ?? '';
if (/\.(html|json|js)(\?|$)/.test(url) && !url.startsWith('/sdks/')) {
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
res.setHeader('Pragma', 'no-cache');
}
next();
});
},
},
],
});
40 changes: 38 additions & 2 deletions src/shared/database/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { APP_ID, EXTERNAL_ID, ONESIGNAL_ID } from '__test__/constants';
import { beforeEach, describe, expect, test, vi } from 'vite-plus/test';
import { afterEach, beforeEach, describe, expect, test, vi } from 'vite-plus/test';

import { SubscriptionType } from '../subscriptions/constants';
import { closeDb, getDb } from './client';
import { closeDb, db, getDb, isOptionsWriteWedged } from './client';
import { DATABASE_NAME } from './constants';
import type * as idbLite from './idb-lite';
import { wrapRequest } from './idb-lite';
Expand Down Expand Up @@ -323,6 +323,42 @@ describe('migrations', () => {
});
});

describe('Options write timeout', () => {
beforeEach(() => {
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
});

afterEach(() => {
vi.useRealTimers();
});

test('clears the timeout when an Options put resolves before it fires', async () => {
await getDb();
await db.put('Options', { key: 'userConsent', value: true });
expect(vi.getTimerCount()).toBe(0);
});

test('trips circuit breaker on Options put timeout, short-circuits subsequent writes', async () => {
expect(isOptionsWriteWedged()).toBe(false);

const _db = await getDb();
const realPut = _db.put.bind(_db);
vi.spyOn(_db, 'put').mockImplementation(((s: string, v: unknown) => {
if (s === 'Options') return new Promise(() => {});
return realPut(s as Parameters<typeof realPut>[0], v as never);
}) as typeof _db.put);

const first = db.put('Options', { key: 'isPushEnabled', value: true });
await vi.advanceTimersByTimeAsync(2001);
expect(await first).toBeUndefined();
expect(isOptionsWriteWedged()).toBe(true);

expect(await db.put('Options', { key: 'lastPushId', value: 'x' })).toBeUndefined();
await db.put('Ids', { type: 'appId', id: 'A' });
expect((await db.get('Ids', 'appId'))?.id).toBe('A');
});
});

test('should reopen db when terminated', async () => {
vi.resetModules();
openFn.mockClear();
Expand Down
45 changes: 42 additions & 3 deletions src/shared/database/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,42 @@ export const getDb = (version = VERSION) => {
return dbPromise;
};

// Export db object with the same API as before
// On iOS Safari PWA after a push subscription, `readwrite` requests on the
// `Options` object store can stall indefinitely (no success/error/abort).
// Other stores and reads are unaffected, and reopening the DB doesn't help.
// Without this guard, `OneSignal.init()` hangs until WebKit's watchdog
// eventually aborts the transaction (~30 minutes). Workaround: cap Options
// writes with a short timeout, then trip a page-scoped circuit breaker so
// subsequent writes short-circuit. The values that fail to persist are
// session metadata the SW reads with sensible fallbacks. Remove if WebKit
// ever fixes the underlying bug: https://bugs.webkit.org/show_bug.cgi?id=315804
const OPTIONS_WRITE_TIMEOUT_MS = 1500;
Comment thread
sherwinski marked this conversation as resolved.
let optionsWriteWedged = false;

export const isOptionsWriteWedged = () => optionsWriteWedged;

// `op` is invoked synchronously (callers await `dbPromise` first), so the
// timeout scopes only to the readwrite request, not DB open/upgrade. Once a
// write times out we trip a page-scoped circuit breaker so the rest of init's
// Options writes short-circuit instead of each paying the full timeout.
function guardOptionsWrite<T>(
Comment thread
sherwinski marked this conversation as resolved.
storeName: IDBStoreName,
label: string,
op: () => Promise<T>,
): Promise<T | undefined> {
if (storeName !== 'Options') return op();
if (optionsWriteWedged) return Promise.resolve(undefined);
let timer: ReturnType<typeof setTimeout>;
const timeout = new Promise<undefined>((resolve) => {
timer = setTimeout(() => {
optionsWriteWedged = true;
Log._warn(`db.${label} timed out`);
resolve(undefined);
}, OPTIONS_WRITE_TIMEOUT_MS);
});
return Promise.race([op(), timeout]).finally(() => clearTimeout(timer));
}
Comment thread
sherwinski marked this conversation as resolved.

export const db = {
async get<K extends IDBStoreName>(
storeName: K,
Expand All @@ -105,10 +140,14 @@ export const db = {
return (await dbPromise).getAll(storeName);
},
async put<K extends IDBStoreName>(storeName: K, value: IndexedDBSchema[K]['value']) {
return (await dbPromise).put(storeName, value);
const _db = await dbPromise;
return guardOptionsWrite(storeName, `put(${storeName})`, () => _db.put(storeName, value));
},
async delete<K extends IDBStoreName>(storeName: K, key: IndexedDBSchema[K]['key']) {
return (await dbPromise).delete(storeName, key);
const _db = await dbPromise;
return guardOptionsWrite(storeName, `delete(${storeName}/${key})`, () =>
_db.delete(storeName, key),
);
},
};

Expand Down
22 changes: 22 additions & 0 deletions src/shared/helpers/init.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,19 @@ import Context from 'src/page/models/Context';
import { type AppConfig } from 'src/shared/config/types';
import { beforeEach, describe, expect, test, vi, type MockInstance } from 'vite-plus/test';

import * as clientModule from '../database/client';
import { db } from '../database/client';
import { getAppState } from '../database/config';
import * as InitHelper from './init';

vi.mock('../database/client', async (importOriginal) => {
const actual = await importOriginal<typeof import('../database/client')>();
return {
...actual,
isOptionsWriteWedged: vi.fn(() => false),
};
});

let isSubscriptionExpiringSpy: MockInstance;

beforeEach(() => {
Expand Down Expand Up @@ -191,4 +200,17 @@ describe('initSaveState: App ID migration', () => {
const storedAppId = await db.get('Ids', 'appId');
expect(storedAppId?.id).toBe(NEW_APP_ID);
});

test('defers App ID commit when Options write breaker is tripped', async () => {
await seedStaleState();
await db.put('Ids', { type: 'userId', id: 'old-user-id' });

vi.mocked(clientModule.isOptionsWriteWedged).mockReturnValueOnce(true);

await InitHelper.initSaveState();

expect((await db.get('Ids', 'appId'))?.id).toBe(OLD_APP_ID);
expect((await db.get('Ids', 'registrationId'))?.id).toBe('old-reg-token');
expect((await db.get('Ids', 'userId'))?.id).toBe('old-user-id');
});
});
8 changes: 7 additions & 1 deletion src/shared/helpers/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ModelChangeTags } from 'src/core/types/models';
import Bell from '../../page/bell/Bell';
import type { AppConfig } from '../config/types';
import type { ContextInterface } from '../context/types';
import { db, getIdsValue } from '../database/client';
import { db, getIdsValue, isOptionsWriteWedged } from '../database/client';
import { getSubscription, setSubscription } from '../database/subscription';
import type { OptionKey } from '../database/types';
import Log from '../libraries/Log';
Expand Down Expand Up @@ -352,6 +352,12 @@ export async function initSaveState(overridingPageTitle?: string) {
await db.put('Options', { key: 'lastPushId', value: null });
await db.put('Options', { key: 'lastPushToken', value: null });
await db.put('Options', { key: 'lastOptedIn', value: null });
// Bail out if the Options reset got circuit-broken. Committing the new
// appId now would strand the previous app's metadata under it, and the
// `previousAppId !== appId` gate above would keep us out of this branch
// on later loads — leaving the stale values permanent. Skipping the
// appId commit instead lets a future non-wedged load complete the reset.
if (isOptionsWriteWedged()) return;
await db.put('Ids', { type: 'registrationId', id: null });
await db.put('Ids', { type: 'userId', id: null });
OneSignal._coreDirector._subscriptionModelStore._clear(ModelChangeTags._Hydrate);
Expand Down
Loading