Skip to content

Commit 07d28de

Browse files
authored
fix: [SDK-4336] guard IndexedDB Options writes from iOS Safari PWA wedge (#1468)
1 parent fe0ecb5 commit 07d28de

10 files changed

Lines changed: 256 additions & 10 deletions

File tree

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,12 @@
7777
},
7878
{
7979
"path": "./build/releases/OneSignalSDK.page.es6.js",
80-
"limit": "42.4 kB",
80+
"limit": "42.7 kB",
8181
"gzip": true
8282
},
8383
{
8484
"path": "./build/releases/OneSignalSDK.sw.js",
85-
"limit": "12.354 kB",
85+
"limit": "12.65 kB",
8686
"gzip": true
8787
},
8888
{

preview/OneSignalSDKWorker.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
importScripts('https://localhost:4001/sdks/web/v16/Dev-OneSignalSDK.sw.js');
1+
// Use self.location.origin so the worker imports from whichever origin
2+
// served it (localhost during local dev, an ngrok/Cloudflare tunnel during
3+
// device testing, etc.). Hardcoding localhost:4001 breaks any test that
4+
// loads this worker from a non-localhost origin (the inner script is
5+
// fetched as part of registration and produces a NetworkError otherwise).
6+
importScripts(`${self.location.origin}/sdks/web/v16/Dev-OneSignalSDK.sw.js`);
27

38
// For testing on staging
49
// importScripts(

preview/pageA.html

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
6+
<title>SDK-4336 Repro: Page A</title>
7+
<link rel="shortcut icon" href="#" />
8+
<link rel="manifest" href="manifest.json" />
9+
<meta name="apple-mobile-web-app-capable" content="yes" />
10+
<meta name="apple-mobile-web-app-status-bar-style" content="default" />
11+
<meta name="apple-mobile-web-app-title" content="SDK4336" />
12+
13+
<script src="sdks/web/v16/Dev-OneSignalSDK.page.js" defer></script>
14+
15+
<script>
16+
console.log('!!!! [SDK-4336 PAGE A] OneSignal initialize');
17+
18+
function getUrlQueryParam(name) {
19+
const urlParams = new URLSearchParams(window.location.search);
20+
return urlParams.get(name);
21+
}
22+
23+
// Resolve the app_id robustly. The in-page links to Page B/Page A don't
24+
// carry the query string, and on an iOS standalone (home-screen) PWA the
25+
// localStorage fallback isn't reliable across A->B->A navigations because
26+
// storage is partitioned per context. Without a hardcoded fallback, the
27+
// B->A load calls OneSignal.init with appId=null -> InvalidAppIdError.
28+
const DEFAULT_APP_ID = 'b79087eb-8531-4d2d-a6f5-726f797891c7';
29+
let appId = getUrlQueryParam('app_id');
30+
try {
31+
if (appId) localStorage.setItem('sdk4336_app_id', appId);
32+
else appId = localStorage.getItem('sdk4336_app_id');
33+
} catch (e) {}
34+
appId = appId || DEFAULT_APP_ID;
35+
36+
const SERVICE_WORKER_PATH = 'push/onesignal/';
37+
38+
var OneSignalDeferred = window.OneSignalDeferred || [];
39+
OneSignalDeferred.push(async function (OneSignal) {
40+
const t0 = performance.now();
41+
await OneSignal.init({
42+
appId,
43+
serviceWorkerParam: { scope: '/' + SERVICE_WORKER_PATH },
44+
serviceWorkerPath: SERVICE_WORKER_PATH + 'OneSignalSDKWorker.js',
45+
});
46+
const elapsed = Math.round(performance.now() - t0);
47+
console.log(`!!!! [SDK-4336 PAGE A] OneSignal initialized (${elapsed}ms)`);
48+
});
49+
50+
async function requestNotificationPermission() {
51+
window.OneSignalDeferred = window.OneSignalDeferred || [];
52+
OneSignalDeferred.push(async function (OneSignal) {
53+
await OneSignal.Notifications.requestPermission();
54+
});
55+
}
56+
</script>
57+
</head>
58+
<body>
59+
<h1>SDK-4336 Repro: Page A</h1>
60+
<p>
61+
<a href="./pageB.html">Go to Page B</a>
62+
</p>
63+
<p>
64+
<button onclick="requestNotificationPermission()">Register</button>
65+
</p>
66+
<p>Repro steps (per the ticket):</p>
67+
<ol>
68+
<li>Add to Home Screen and open Page A.</li>
69+
<li>Tap "Go to Page B".</li>
70+
<li>Tap "Go to Page A".</li>
71+
<li>Tap "Register" and accept the prompt.</li>
72+
<li>Tap "Go to Page B".</li>
73+
<li>Tap "Go to Page A" &mdash; <code>init()</code> should hang.</li>
74+
</ol>
75+
</body>
76+
</html>

preview/pageB.html

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!doctype html>
2+
<html lang="en">
3+
<head>
4+
<meta charset="utf-8" />
5+
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
6+
<title>SDK-4336 Repro: Page B</title>
7+
<link rel="shortcut icon" href="#" />
8+
<link rel="manifest" href="manifest.json" />
9+
<meta name="apple-mobile-web-app-capable" content="yes" />
10+
<meta name="apple-mobile-web-app-status-bar-style" content="default" />
11+
<meta name="apple-mobile-web-app-title" content="SDK4336" />
12+
</head>
13+
<body>
14+
<h1>SDK-4336 Repro: Page B</h1>
15+
<p>
16+
<a href="./pageA.html">Go to Page A</a>
17+
</p>
18+
<p>
19+
This page intentionally does <em>not</em> initialize the SDK. It just navigates back to Page A
20+
so we can exercise the multi-page navigation flow described in SDK-4336.
21+
</p>
22+
</body>
23+
</html>

preview/push/onesignal/OneSignalSDKWorker.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
importScripts('https://localhost:4001/sdks/web/v16/Dev-OneSignalSDK.sw.js');
1+
importScripts(`${self.location.origin}/sdks/web/v16/Dev-OneSignalSDK.sw.js`);
22

33
// For testing on staging
44
// importScripts(

preview/vite.config.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,25 @@ export default defineConfig({
4343
// SDK fetch URL (e.g. `http://localhost:4000/...` in HTTP mode) lands here.
4444
port: useHttps ? 4001 : 4000,
4545
strictPort: true,
46+
// HMR's WebSocket targets `wss://localhost:<port>`, which iOS devices on a
47+
// tunnel (ngrok, etc.) can't reach. Disabling stops the runaway
48+
// reconnect loop that floods the console with `ws.send` rejections.
49+
hmr: false,
4650
},
4751
plugins: [
52+
{
53+
// Vite's dev server injects `<script src="/@vite/client">` into every
54+
// HTML response, even when `server.hmr` is false. That client then
55+
// retries a WebSocket forever, which on a tunneled iOS device floods
56+
// the console with `ws.send` rejections and is heavy enough to
57+
// skew our SDK timing. Strip it out for the preview server, where
58+
// we don't need HMR.
59+
name: 'strip-vite-client',
60+
enforce: 'post',
61+
transformIndexHtml(html) {
62+
return html.replace(/<script[^>]*src="\/@vite\/client"[^>]*><\/script>\s*/g, '');
63+
},
64+
},
4865
...(useHttps
4966
? [
5067
mkcert({
@@ -87,9 +104,31 @@ export default defineConfig({
87104
if (contentType) {
88105
res.setHeader('Content-Type', contentType);
89106
}
107+
// Avoid stale SDK bundles being served from the browser HTTP cache,
108+
// which is especially aggressive on iOS Safari/PWA. Without this,
109+
// rebuilding the SDK during a debug session won't take effect on the
110+
// device until you fully wipe website data.
111+
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
112+
res.setHeader('Pragma', 'no-cache');
90113
fs.createReadStream(filePath).pipe(res);
91114
});
92115
},
93116
},
117+
{
118+
// Same hygiene for the sandbox HTML (pageA/pageB/index) and root SW
119+
// file. Vite's default dev-server cache policy is fine for source files
120+
// but our HTML directly references the SDK and embeds repro state.
121+
name: 'no-store-html',
122+
configureServer(server) {
123+
server.middlewares.use((req, res, next) => {
124+
const url = req.url ?? '';
125+
if (/\.(html|json|js)(\?|$)/.test(url) && !url.startsWith('/sdks/')) {
126+
res.setHeader('Cache-Control', 'no-store, no-cache, must-revalidate');
127+
res.setHeader('Pragma', 'no-cache');
128+
}
129+
next();
130+
});
131+
},
132+
},
94133
],
95134
});

src/shared/database/client.test.ts

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { APP_ID, EXTERNAL_ID, ONESIGNAL_ID } from '__test__/constants';
2-
import { beforeEach, describe, expect, test, vi } from 'vite-plus/test';
2+
import { afterEach, beforeEach, describe, expect, test, vi } from 'vite-plus/test';
33

44
import { SubscriptionType } from '../subscriptions/constants';
5-
import { closeDb, getDb } from './client';
5+
import { closeDb, db, getDb, isOptionsWriteWedged } from './client';
66
import { DATABASE_NAME } from './constants';
77
import type * as idbLite from './idb-lite';
88
import { wrapRequest } from './idb-lite';
@@ -323,6 +323,42 @@ describe('migrations', () => {
323323
});
324324
});
325325

326+
describe('Options write timeout', () => {
327+
beforeEach(() => {
328+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] });
329+
});
330+
331+
afterEach(() => {
332+
vi.useRealTimers();
333+
});
334+
335+
test('clears the timeout when an Options put resolves before it fires', async () => {
336+
await getDb();
337+
await db.put('Options', { key: 'userConsent', value: true });
338+
expect(vi.getTimerCount()).toBe(0);
339+
});
340+
341+
test('trips circuit breaker on Options put timeout, short-circuits subsequent writes', async () => {
342+
expect(isOptionsWriteWedged()).toBe(false);
343+
344+
const _db = await getDb();
345+
const realPut = _db.put.bind(_db);
346+
vi.spyOn(_db, 'put').mockImplementation(((s: string, v: unknown) => {
347+
if (s === 'Options') return new Promise(() => {});
348+
return realPut(s as Parameters<typeof realPut>[0], v as never);
349+
}) as typeof _db.put);
350+
351+
const first = db.put('Options', { key: 'isPushEnabled', value: true });
352+
await vi.advanceTimersByTimeAsync(2001);
353+
expect(await first).toBeUndefined();
354+
expect(isOptionsWriteWedged()).toBe(true);
355+
356+
expect(await db.put('Options', { key: 'lastPushId', value: 'x' })).toBeUndefined();
357+
await db.put('Ids', { type: 'appId', id: 'A' });
358+
expect((await db.get('Ids', 'appId'))?.id).toBe('A');
359+
});
360+
});
361+
326362
test('should reopen db when terminated', async () => {
327363
vi.resetModules();
328364
openFn.mockClear();

src/shared/database/client.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,42 @@ export const getDb = (version = VERSION) => {
9393
return dbPromise;
9494
};
9595

96-
// Export db object with the same API as before
96+
// On iOS Safari PWA after a push subscription, `readwrite` requests on the
97+
// `Options` object store can stall indefinitely (no success/error/abort).
98+
// Other stores and reads are unaffected, and reopening the DB doesn't help.
99+
// Without this guard, `OneSignal.init()` hangs until WebKit's watchdog
100+
// eventually aborts the transaction (~30 minutes). Workaround: cap Options
101+
// writes with a short timeout, then trip a page-scoped circuit breaker so
102+
// subsequent writes short-circuit. The values that fail to persist are
103+
// session metadata the SW reads with sensible fallbacks. Remove if WebKit
104+
// ever fixes the underlying bug: https://bugs.webkit.org/show_bug.cgi?id=315804
105+
const OPTIONS_WRITE_TIMEOUT_MS = 1500;
106+
let optionsWriteWedged = false;
107+
108+
export const isOptionsWriteWedged = () => optionsWriteWedged;
109+
110+
// `op` is invoked synchronously (callers await `dbPromise` first), so the
111+
// timeout scopes only to the readwrite request, not DB open/upgrade. Once a
112+
// write times out we trip a page-scoped circuit breaker so the rest of init's
113+
// Options writes short-circuit instead of each paying the full timeout.
114+
function guardOptionsWrite<T>(
115+
storeName: IDBStoreName,
116+
label: string,
117+
op: () => Promise<T>,
118+
): Promise<T | undefined> {
119+
if (storeName !== 'Options') return op();
120+
if (optionsWriteWedged) return Promise.resolve(undefined);
121+
let timer: ReturnType<typeof setTimeout>;
122+
const timeout = new Promise<undefined>((resolve) => {
123+
timer = setTimeout(() => {
124+
optionsWriteWedged = true;
125+
Log._warn(`db.${label} timed out`);
126+
resolve(undefined);
127+
}, OPTIONS_WRITE_TIMEOUT_MS);
128+
});
129+
return Promise.race([op(), timeout]).finally(() => clearTimeout(timer));
130+
}
131+
97132
export const db = {
98133
async get<K extends IDBStoreName>(
99134
storeName: K,
@@ -105,10 +140,14 @@ export const db = {
105140
return (await dbPromise).getAll(storeName);
106141
},
107142
async put<K extends IDBStoreName>(storeName: K, value: IndexedDBSchema[K]['value']) {
108-
return (await dbPromise).put(storeName, value);
143+
const _db = await dbPromise;
144+
return guardOptionsWrite(storeName, `put(${storeName})`, () => _db.put(storeName, value));
109145
},
110146
async delete<K extends IDBStoreName>(storeName: K, key: IndexedDBSchema[K]['key']) {
111-
return (await dbPromise).delete(storeName, key);
147+
const _db = await dbPromise;
148+
return guardOptionsWrite(storeName, `delete(${storeName}/${key})`, () =>
149+
_db.delete(storeName, key),
150+
);
112151
},
113152
};
114153

src/shared/helpers/init.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,19 @@ import Context from 'src/page/models/Context';
66
import { type AppConfig } from 'src/shared/config/types';
77
import { beforeEach, describe, expect, test, vi, type MockInstance } from 'vite-plus/test';
88

9+
import * as clientModule from '../database/client';
910
import { db } from '../database/client';
1011
import { getAppState } from '../database/config';
1112
import * as InitHelper from './init';
1213

14+
vi.mock('../database/client', async (importOriginal) => {
15+
const actual = await importOriginal<typeof import('../database/client')>();
16+
return {
17+
...actual,
18+
isOptionsWriteWedged: vi.fn(() => false),
19+
};
20+
});
21+
1322
let isSubscriptionExpiringSpy: MockInstance;
1423

1524
beforeEach(() => {
@@ -191,4 +200,17 @@ describe('initSaveState: App ID migration', () => {
191200
const storedAppId = await db.get('Ids', 'appId');
192201
expect(storedAppId?.id).toBe(NEW_APP_ID);
193202
});
203+
204+
test('defers App ID commit when Options write breaker is tripped', async () => {
205+
await seedStaleState();
206+
await db.put('Ids', { type: 'userId', id: 'old-user-id' });
207+
208+
vi.mocked(clientModule.isOptionsWriteWedged).mockReturnValueOnce(true);
209+
210+
await InitHelper.initSaveState();
211+
212+
expect((await db.get('Ids', 'appId'))?.id).toBe(OLD_APP_ID);
213+
expect((await db.get('Ids', 'registrationId'))?.id).toBe('old-reg-token');
214+
expect((await db.get('Ids', 'userId'))?.id).toBe('old-user-id');
215+
});
194216
});

src/shared/helpers/init.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { ModelChangeTags } from 'src/core/types/models';
33
import Bell from '../../page/bell/Bell';
44
import type { AppConfig } from '../config/types';
55
import type { ContextInterface } from '../context/types';
6-
import { db, getIdsValue } from '../database/client';
6+
import { db, getIdsValue, isOptionsWriteWedged } from '../database/client';
77
import { getSubscription, setSubscription } from '../database/subscription';
88
import type { OptionKey } from '../database/types';
99
import Log from '../libraries/Log';
@@ -352,6 +352,12 @@ export async function initSaveState(overridingPageTitle?: string) {
352352
await db.put('Options', { key: 'lastPushId', value: null });
353353
await db.put('Options', { key: 'lastPushToken', value: null });
354354
await db.put('Options', { key: 'lastOptedIn', value: null });
355+
// Bail out if the Options reset got circuit-broken. Committing the new
356+
// appId now would strand the previous app's metadata under it, and the
357+
// `previousAppId !== appId` gate above would keep us out of this branch
358+
// on later loads — leaving the stale values permanent. Skipping the
359+
// appId commit instead lets a future non-wedged load complete the reset.
360+
if (isOptionsWriteWedged()) return;
355361
await db.put('Ids', { type: 'registrationId', id: null });
356362
await db.put('Ids', { type: 'userId', id: null });
357363
OneSignal._coreDirector._subscriptionModelStore._clear(ModelChangeTags._Hydrate);

0 commit comments

Comments
 (0)