From c0e3fd610f449765158ea00ad7e2354d900d2e4e Mon Sep 17 00:00:00 2001 From: Chris Peyer Date: Mon, 18 May 2026 10:11:51 -0400 Subject: [PATCH] fix(prose): block y-websocket auto-reconnect during async IMS refresh The 4401 close handler awaits adobeIMS.refreshToken() before deciding whether to retry. y-websocket's onclose schedules its own setTimeout(setupWS, ...) synchronously; on a 4401 the upgrade returned 101 + immediate close, so onopen briefly fired and wsUnsuccessfulReconnects stayed at 0 - backoff is 2^0*100=100ms, shorter than a typical IMS refresh round-trip. The auto-reconnect fired with the still-stale token and burned 2-3 HEAD 401s on da-admin per token expiry per tab. Flip provider.shouldConnect=false synchronously before the first await so the pending setTimeout becomes a no-op, then drive the reconnect ourselves via provider.connect() once we have the fresh token. Same fix removes the bug 2 race where two concurrent close handlers could mutate lastSentToken out from under each other and stop reconnection on a successful refresh. Also skip the createAwarenessStatusWidget checkDoc HEAD on auth-close codes - those signal auth failure not doc-deleted, and the daFetch refresh-and-retry doubles the HEAD 401 traffic for no benefit. Co-Authored-By: Claude Opus 4.7 --- blocks/edit/prose/index.js | 6 +- test/unit/blocks/edit/prose/index.test.js | 79 +++++++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/blocks/edit/prose/index.js b/blocks/edit/prose/index.js index ebe8cf4e..ee3563ba 100644 --- a/blocks/edit/prose/index.js +++ b/blocks/edit/prose/index.js @@ -62,13 +62,13 @@ export async function createConnection(path) { return; } if (event?.code === 4401) { + provider.shouldConnect = false; // Force imslib to attempt a refresh before deciding to give up. try { await window.adobeIMS?.refreshToken?.(); } catch { /* ignore */ } const fresh = await getAuthToken(); if (!fresh || fresh === lastSentToken) { // No new token to try — retrying would loop on the same 4401. Stop // the reconnect loop, and surface the modal if the user was signed in. - provider.shouldConnect = false; if (lastSentToken) { try { const { showAuthBanner } = await import('../../shared/da-auth-banner/da-auth-banner.js'); @@ -79,6 +79,7 @@ export async function createConnection(path) { } provider.protocols = ['yjs', fresh]; lastSentToken = fresh; + provider.connect(); return; } const fresh = await getAuthToken(); @@ -284,7 +285,8 @@ function handleAwarenessUpdates(wsProvider, daTitle, win, path) { if (wsProvider.wsconnected) daTitle.collabStatus = 'connected'; else daTitle.collabStatus = 'connecting'; - wsProvider.on('connection-close', async () => { + wsProvider.on('connection-close', async (event) => { + if (event?.code === 4401 || event?.code === 4403) return; const resp = await checkDoc(path); if (resp.status === 404) { const { hash } = window.location; diff --git a/test/unit/blocks/edit/prose/index.test.js b/test/unit/blocks/edit/prose/index.test.js index 8ad80a88..c14df754 100644 --- a/test/unit/blocks/edit/prose/index.test.js +++ b/test/unit/blocks/edit/prose/index.test.js @@ -229,6 +229,53 @@ describe('prose/index createConnection', () => { } }); + it('Blocks y-websocket auto-reconnect during the in-flight refresh on 4401', async () => { + // Regression: y-websocket's onclose schedules setTimeout(setupWS, 100ms) + // synchronously; if shouldConnect stays true through the await + // refreshToken() round-trip the auto-reconnect fires with the stale token + // and burns a HEAD 401 on da-admin. shouldConnect must flip to false + // synchronously before the first await. + window.localStorage.setItem('nx-ims', 'true'); + const savedIMS = window.adobeIMS; + let refreshResolve; + const refreshPromise = new Promise((r) => { refreshResolve = r; }); + let tokenIndex = 0; + const tokens = ['T-old', 'T-new']; + window.adobeIMS = { + getAccessToken: () => ({ token: tokens[tokenIndex] }), + refreshToken: () => refreshPromise, + }; + + try { + const { wsProvider, ydoc } = await createConnection('https://admin.da.live/source/org/repo/page.html'); + expect(wsProvider.shouldConnect).to.equal(true); + + // Fire 4401 — handler runs sync up to first await, then yields. + wsProvider.emit('connection-close', [{ code: 4401, reason: 'auth' }, wsProvider]); + // Microtask boundary: handler has hit `await refreshToken()` and yielded. + await Promise.resolve(); + + // During the in-flight refresh, shouldConnect MUST be false so a stale + // y-websocket reconnect timer fires as a no-op. + expect(wsProvider.shouldConnect).to.equal(false); + + // Resolve the refresh; rotate the token so getAuthToken() returns T-new. + tokenIndex = 1; + refreshResolve(); + await new Promise((r) => { setTimeout(r, 0); }); + + // After refresh: fresh token applied and reconnect explicitly re-enabled. + expect(wsProvider.protocols).to.deep.equal(['yjs', 'T-new']); + expect(wsProvider.shouldConnect).to.equal(true); + + wsProvider.disconnect({ data: 'Client navigation' }); + wsProvider.destroy?.(); + ydoc.destroy(); + } finally { + if (savedIMS === undefined) delete window.adobeIMS; else window.adobeIMS = savedIMS; + } + }); + it('Non-auth close with no token reconnects as anonymous', async () => { window.localStorage.removeItem('nx-ims'); const savedIMS = window.adobeIMS; @@ -306,6 +353,38 @@ describe('prose/index createAwarenessStatusWidget', () => { expect(fakeTitle.collabStatus).to.equal('disconnected'); }); + it('Skips checkDoc HEAD on auth-close codes (4401/4403)', async () => { + // checkDoc is intended to detect doc-deleted-externally (404). On 4401/4403 + // the doc is fine — da-collab signalled an auth failure. Firing checkDoc + // here just doubles the HEAD 401 traffic to da-admin via daFetch's + // refresh-and-retry, so it must be skipped. + const provider = buildFakeWsProvider(); + const fakeWin = { document, addEventListener: () => {} }; + const fetchCalls = []; + const savedFetch = window.fetch; + window.fetch = (...args) => { + fetchCalls.push(args); + return Promise.resolve(new Response('', { status: 200, headers: {} })); + }; + try { + createAwarenessStatusWidget(provider, fakeWin, 'https://admin.da.live/source/o/r/p.html'); + + provider._emit('connection-close', { code: 4401, reason: 'auth' }, provider); + provider._emit('connection-close', { code: 4403, reason: 'forbidden' }, provider); + await new Promise((r) => { setTimeout(r, 0); }); + + expect(fetchCalls.filter(([, o]) => o?.method === 'HEAD')) + .to.have.lengthOf(0); + + // Sanity: a non-auth close still does fire checkDoc. + provider._emit('connection-close', { code: 1006 }, provider); + await new Promise((r) => { setTimeout(r, 0); }); + expect(fetchCalls.some(([, o]) => o?.method === 'HEAD')).to.equal(true); + } finally { + window.fetch = savedFetch; + } + }); + it('On focus reconnects, on blur schedules disconnect', async () => { const provider = buildFakeWsProvider(); const winListeners = new Map();