From 6dbda4424efffa043fbfde936cf5ce8f7d578af2 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 00:09:44 +0100 Subject: [PATCH] fix: stop a stuck push setup from holding up every tab and login --- lib/__tests__/web-push.test.ts | 54 ++++++++++++++++++++++++++++++++++ lib/web-push.ts | 22 +++++++++++--- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/lib/__tests__/web-push.test.ts b/lib/__tests__/web-push.test.ts index 969504d09..e9ad7ee95 100644 --- a/lib/__tests__/web-push.test.ts +++ b/lib/__tests__/web-push.test.ts @@ -903,6 +903,60 @@ describe('a subscription id the server gave to another device', () => { }); }); +describe('a setup that gets stuck', () => { + afterEach(() => { + vi.useRealTimers(); + delete (navigator as unknown as { locks?: unknown }).locks; + }); + + it('fails in the end and lets the next one through', async () => { + vi.useFakeTimers(); + let tail: Promise = Promise.resolve(); + Object.defineProperty(navigator, 'locks', { + configurable: true, + value: { request: (_n: string, run: () => Promise) => { const next = tail.then(run, run); tail = next.catch(() => undefined); return next; } }, + }); + const { registration } = installPushBrowser(); + (registration.pushManager.getSubscription as ReturnType).mockResolvedValueOnce(null); + (registration.pushManager.subscribe as ReturnType).mockReturnValueOnce(new Promise(() => {})); + const client = makeClient([]); + installFetch({}); + + const stuck = enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }).then(() => 'done', () => 'failed'); + await vi.advanceTimersByTimeAsync(1_000); + const second = enableWebPush({ client, relayBaseUrl: RELAY, inboxOnly: false }).then(() => 'done', () => 'failed'); + await vi.advanceTimersByTimeAsync(200_000); + + expect(await stuck).toBe('failed'); + expect(await second).toBe('done'); + }); + + it('turns off for the account it was asked about, even if the client disconnected while waiting', async () => { + let tail: Promise = Promise.resolve(); + let release = () => {}; + tail = new Promise((resolve) => { release = resolve; }); + Object.defineProperty(navigator, 'locks', { + configurable: true, + value: { request: (_n: string, run: () => Promise) => { const next = tail.then(run, run); tail = next.catch(() => undefined); return next; } }, + }); + localStorage.setItem(DEVICE_KEY, THIS_DEVICE); + localStorage.setItem(SUB_KEY, 'push-mine'); + localStorage.setItem(RELAY_BASE_KEY, RELAY); + const client = makeClient([sub('push-mine', THIS_DEVICE)]); + let accountId = ACCOUNT_ID; + (client as unknown as { getAccountId: () => string }).getAccountId = () => accountId; + const calls = installFetch({}); + + const off = disableWebPush({ client, relayBaseUrl: RELAY }).catch(() => undefined); + accountId = ''; + release(); + await off; + + expect(localStorage.getItem(SUB_KEY)).toBeNull(); + expect(calls).toContainEqual({ url: `${RELAY}/api/push/register/${THIS_DEVICE}`, method: 'DELETE' }); + }); +}); + describe('resyncWebPush when the relay changed (#903)', () => { afterEach(() => setNoThirdPartyTraffic(false)); diff --git a/lib/web-push.ts b/lib/web-push.ts index 818cbc5e6..7daf97d71 100644 --- a/lib/web-push.ts +++ b/lib/web-push.ts @@ -775,8 +775,21 @@ async function refreshSubscriptionExpires( // verifying. Tabs take turns, in every browser that has Web Locks. Whoever comes // second finds the first one's work and only refreshes it. function oneTabAtATime(run: () => Promise): Promise { + const limited = () => withinPushSetupLimit(run); const locks = typeof navigator !== 'undefined' ? navigator.locks : undefined; - return locks ? locks.request('bulwark-web-push', run) as Promise : run(); + return locks ? locks.request('bulwark-web-push', limited) as Promise : limited(); +} + +// A worker that never gets ready or a subscribe() that never answers would hold up +// every tab and every login, for good. Verification alone can take 80 s. +const PUSH_SETUP_LIMIT_MS = 120_000; + +function withinPushSetupLimit(run: () => Promise): Promise { + let timer: ReturnType | undefined; + const limit = new Promise((_resolve, reject) => { + timer = setTimeout(() => reject(new WebPushStepError('verification-timeout', 'Push setup took too long')), PUSH_SETUP_LIMIT_MS); + }); + return Promise.race([run(), limit]).finally(() => clearTimeout(timer)); } export async function enableWebPush( @@ -1018,12 +1031,13 @@ async function dropRelayRecord(relayBaseUrl: string, deviceClientId: string): Pr // a later page load won't bring push back. But a server or relay that couldn't be // told keeps pinging this browser for days, so the caller hears about it at the end. export function disableWebPush(params: DisableWebPushParams): Promise { - return oneTabAtATime(() => disableWebPushForThisTab(params)); + // Read now: a client signing out loses it while this waits its turn. + const accountId = params.client.getAccountId(); + return oneTabAtATime(() => disableWebPushForThisTab(params, accountId)); } -async function disableWebPushForThisTab(params: DisableWebPushParams): Promise { +async function disableWebPushForThisTab(params: DisableWebPushParams, accountId: string): Promise { const relayBaseUrl = (params.relayBaseUrl ?? DEFAULT_RELAY_BASE_URL).replace(/\/+$/, ''); - const accountId = params.client.getAccountId(); const subIdKey = subscriptionIdKey(accountId); const devIdKey = deviceClientIdKey(accountId);