diff --git a/CHANGELOG.md b/CHANGELOG.md index 23444570..203c5d18 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,11 @@ notes. Entries describe what users experience, not internal refactors. broken. A tab still on an older build now moves onto the new one on its next load, without asking and without interrupting anything: it happens while the page is still loading, once, and never over unsaved edits. +- **An update could leave the editor blank.** When a new version took over + while the page was still starting up, the request for the editor itself was + cancelled mid-flight and nothing asked for it again -- an empty white page + that reloading was the only way out of. The page now always reloads itself + after it switches versions, which is what repairs it. - The four home-screen buttons ("View/Edit Document", "New Word"...) no longer flash behind the spinner while a document named in the URL is loading. - Chinese, Japanese and Korean text in an exported PDF came out blank. The diff --git a/CLAUDE.md b/CLAUDE.md index 5e91e62e..42362052 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -527,6 +527,20 @@ docs/explorations/2026-08-19-ci-e2e-sharding.md。 `DEPLOY_COUPLED`（由 `hosting-contract.test.ts` / `sw-routing.test.ts` 钉住）。 `open-local.js` / `landing-prefetch.js` 同理，2026-08-20 起补齐——它们从路由 拆分起一直漏在 SWR 上，改这两个文件的部署，落地页会一直跑旧的那份。 + **谁提升谁负责 reload**（2026-08-23）：`promoteWaitingWorker` 在 `register()` 一 + resolve 就跑，比编辑器实例存在早几百毫秒，于是它看到"没有文档打开"并提升；等 + `controllerchange` 到达时文档已经打开，`shouldReloadOnControllerChange` 重新问一遍 + 就拒绝 reload——而那次交接已经终止了旧 worker，它手上 in-flight 的 fetch 全部失败， + 其中就有编辑器 iframe 自己的文档请求，没人重试，标签页永远白屏。所以 + `wireServiceWorkerUpdates` 有 `onPromoted` 回调，`index.ts` 用 + `promotedFromThisTab` 标志位统一两条提升路径，见到它就 reload（只有未保存改动 + 一票否决）。别把这个回调当可选参数省掉。**同时必须有另一半**：`hasOpenDocument()` + 要把 URL 也算进去（`documentIsExpected`：`?new=` / `?file=` / `?src=` / `?open=` / + `?saved=` / `?embed=`），否则——因为厂商 worker 的来回切换让我们的 sw.js 几乎每次 + 加载都在 `waiting`——上面那条规则会变成"每次加载都刷新一次"（实测把 + `autosave-recovery` 的 reload 用例打红）。这两条路由上的等待交给静默自愈， + 它会先 `isUnseenBuild()` 确认真是新构建。见 + docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md。 落地页那侧的提升要覆盖三种到达方式：已经 `waiting`、`installing` 中途、 以及 `updatefound` 时已经 `installed`（`statechange` 只报此后的迁移， 漏掉这一支等于整页生命周期内再没人提升它）。 diff --git a/docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md b/docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md new file mode 100644 index 00000000..42b36d1d --- /dev/null +++ b/docs/explorations/2026-08-23-promotion-without-reload-blank-editor.md @@ -0,0 +1,102 @@ +# 谁提升了 worker，谁就得负责那次 reload + +日期：2026-08-23 +现场：CI run 32620431825，`E2E shard 1` 的 @serial 那趟挂在 +`sw-silent-update.spec.ts:92` 的 `settleEditor`，90s 超时。这条用例在本地 16s 通过， +在 CI 已经是第二次红了。 + +## 不是慢，是白屏 + +失败截图是**纯白页**。把 trace 拆开看，时间轴很清楚： + +``` +24.1 page.reload() ← 测试写完新 sw.js 之后的那次刷新 +24.2 console: SW registered +24.3 console: Creating new editor instance +24.35 GET /web-apps/apps/documenteditor/main/index.html → status -1（失败） +... 之后 90 秒什么都没有 +114.3 超时 +``` + +编辑器 iframe 自己的文档请求**失败了**，而且没有任何东西会重试它。页面就停在 +"创建了编辑器实例、iframe 永远空着"的半成品状态。 + +请求为什么会失败：那一刻 service worker 换了人。激活新 worker 会终止旧 worker， +旧 worker 手上所有 in-flight 的 fetch 事件一起失败——其中就有这个 iframe 的导航请求。 + +**这本来不是问题**，因为换 worker 之后页面会 reload 一次，重新加载就好了。 +问题是那次 reload 没有发生。 + +## 提升的判据和 reload 的判据，问的是同一个问题的两个时刻 + +`index.ts` 里两件事： + +```ts +wireServiceWorkerUpdates(registration, hasOpenDocument, ownScriptURL); +// ... +navigator.serviceWorker.addEventListener('controllerchange', () => { + if (!shouldReloadOnControllerChange({ ..., hasOpenDocument: hasOpenDocument() })) return; + window.location.reload(); +}); +``` + +`promoteWaitingWorker` 在 `register()` 一 resolve 就跑，而在编辑器路由上， +**那比编辑器实例存在早几百毫秒**（trace 里 SW registered 在 24.2、 +Creating new editor instance 在 24.3）。于是它看到"没有文档打开"，提升。 + +等 `controllerchange` 真的到达时，文档已经打开了。`shouldReloadOnControllerChange` +重新问一遍 `hasOpenDocument()`，答案变成了 true，于是**拒绝 reload**—— +可这时候页面早就被那次交接撕成两半了。 + +`promoteWaitingWorker` 的返回值一直被 `wireServiceWorkerUpdates` 丢掉， +所以页面从来不知道"是我自己让它换的"。 + +CLAUDE.md 里记着这条路径上相反方向的那个 bug（"打开流程排在 SW 注册之前， +`hasOpenDocument()` 永远为真，于是等待中的 worker 从不被提升"）。修好之后 +竞态翻了个面：注册有时候赢，提升发生了，而修复它的 reload 被拒。 + +## 改法 + +**谁提升谁负责 reload。** `wireServiceWorkerUpdates` 多一个 `onPromoted` 回调， +真的发出 `SKIP_WAITING` 时通知调用方；`index.ts` 把标志位从 `healingStaleBuild` +改名成 `promotedFromThisTab`，两条路径（普通更新提升、静默自愈）共用它； +`shouldReloadOnControllerChange` 见到这个标志就 reload，**不再问有没有文档打开**—— +只有未保存改动仍然一票否决。 + +理由不是偏好而是修复：交接已经发生了，拒绝 reload 不会把它撤回来， +只会把标签页留在白屏上。 + +## 只补第一半会变成"每次加载都刷新一次" + +第一版只做了上面那件事，`E2E (Cloudflare Pages semantics)` 的 +`autosave-recovery` "a reload comes back to the same document" 立刻红（重试也红， +同一分片在没有这个改动的另一个 PR 上是绿的）。 + +原因 CLAUDE.md 里其实写着：**"有 worker 在等"不等于"有新版本"**。厂商的编辑器 iframe +会往同一个 scope 注册它自己的 worker，一个 scope 只有一个 registration，于是脚本在 +我们的 sw.js 与它之间来回换——**编辑器路由上我们的 worker 几乎每次加载都躺在 +`waiting` 里，跟有没有新构建无关**。原先那条"有文档打开就不 reload"顺带当了刹车； +把它拆掉，每一次这种交接都变成一次刷新。 + +所以第二半：**文档在路上的时候根本不要提升**。`hasOpenDocument()` 读的是 store， +而 store 要等编辑器实例建好才有值——比 `register()` resolve 晚几百毫秒。URL 早就知道了： +`?new=` / `?file=` / `?src=` / `?open=` / `?saved=` 任意一个在，就是"这一页要开文档"。 +`?embed=`／`?embedded=` 也算，而且理由更硬：嵌入模式下宿主随时可能推一个文档进来， +那次 reload 扔掉的是宿主页面的东西。 + +这些路由上等待的 worker 就老老实实等着——这不是丢失更新，正是静默自愈存在的那个场景， +而自愈这条路**会先用 `isUnseenBuild()` 确认真的是另一个构建**才交接，不会被厂商 worker +的来回切换骗到。 + +## 用例与反向验证 + +`test/unit/sw-update.test.ts` 新增四条：`onPromoted` 在到达时提升、在稍后安装后提升、 +留在 waiting 时不报；以及那条竞态本身——"在提升与交接之间打开的文档也要 reload"。 + +另加三条钉住 `documentIsExpected`：认得每个会挂文档的路由、把 embed 也算进去、 +没东西可开的页面照常接更新。 + +反向验证：`git stash` 掉 `lib/sw-update.ts` 与 `index.ts`，前四条同时变红 +（两条因为 `onPromoted` 不存在，两条因为标志位读不到）；单独 stash `lib/sw-update.ts`， +`documentIsExpected` 三条变红。E2E `autosave-recovery`（三条）+ `sw-silent-update` + +`sw-warm` 本地全绿——其中 `autosave-recovery` 的 reload 那条正是只补第一半时红的那条。 diff --git a/index.ts b/index.ts index 19751fc2..494e7623 100644 --- a/index.ts +++ b/index.ts @@ -1,4 +1,5 @@ import { + documentIsExpected, healStaleController, onWaitingWorker, shouldReloadOnControllerChange, @@ -233,11 +234,16 @@ if ('serviceWorker' in navigator) { // no document is open, then takes over and the page reloads once. const hadController = !!navigator.serviceWorker.controller; let reloadingForUpdate = false; - // Set when this tab asked an older worker to step aside (see below): the - // reload that follows is the point, so it is not blocked by having a - // document open the way an ordinary update is. - let healingStaleBuild = false; - const hasOpenDocument = () => Boolean(getDocmentObj().fileName); + // Set when this tab told a waiting worker to take over -- either the + // ordinary update below or the stale-build heal. The reload that follows is + // then repair, not a preference: the swap kills whatever the outgoing worker + // was still fetching, so it is not blocked by having a document open. + let promotedFromThisTab = false; + // "Is a document open?" is the wrong tense at boot: the store fills in a few + // hundred milliseconds after register() resolves, so a page opening a + // document answers "no" for exactly as long as it takes to promote a worker + // into the middle of its own load. The URL already knows. + const hasOpenDocument = () => Boolean(getDocmentObj().fileName) || documentIsExpected(window.location.search); navigator.serviceWorker.addEventListener('controllerchange', () => { if ( @@ -245,7 +251,7 @@ if ('serviceWorker' in navigator) { hadController, alreadyReloading: reloadingForUpdate, hasOpenDocument: hasOpenDocument(), - healingStaleBuild, + promotedFromThisTab, hasUnsavedChanges: hasUnsavedChanges(), }) ) { @@ -265,7 +271,15 @@ if ('serviceWorker' in navigator) { .register('./sw.js') .then((registration) => { console.log('SW registered: ', registration); - wireServiceWorkerUpdates(registration, hasOpenDocument, ownScriptURL); + // The fourth argument is the whole point: promotion happens here, a + // few hundred milliseconds before the editor instance exists, so it + // sees "nothing open" and promotes -- and the controllerchange that + // follows arrives after the document is open. Without knowing this tab + // asked for it, the reload is refused and the tab is left on a blank + // editor whose iframe request the swap aborted. + wireServiceWorkerUpdates(registration, hasOpenDocument, ownScriptURL, () => { + promotedFromThisTab = true; + }); // Promotion above is refused while a document is open, and this page // is usually opened with one (?new=, ?file=, ?saved=). Without an // offer, such a visitor never leaves the build their worker cached -- @@ -289,7 +303,7 @@ if ('serviceWorker' in navigator) { hadController, storage: window.sessionStorage, }).then((started) => { - if (started) healingStaleBuild = true; + if (started) promotedFromThisTab = true; }); }, ownScriptURL, diff --git a/lib/sw-update.ts b/lib/sw-update.ts index 2e864b5e..bd7e966b 100644 --- a/lib/sw-update.ts +++ b/lib/sw-update.ts @@ -51,6 +51,32 @@ export function isOwnWorker(reg: RegistrationLike, worker: SwLike, ownScriptURL? export const SKIP_WAITING_MESSAGE = { type: 'SKIP_WAITING' } as const; +/** + * Is this page going to have a document in it? + * + * `hasOpenDocument()` reads the store, which is empty until the editor + * instance exists -- a few hundred milliseconds after `register()` resolves. + * Asking it that early answers "nothing open" about a page whose whole purpose + * is to open something, and the promotion that follows lands in the middle of + * the editor booting. + * + * The URL knows sooner. Every route that mounts a document says so in its + * query string, and embed mode says the host may push one at any moment -- + * which is also why an embedded editor must never promote: the reload that + * follows would throw away a document the host page owns. + * + * On these routes a waiting worker simply stays waiting. That is not a lost + * update: it is the ordinary case the silent heal exists for, and unlike this + * path the heal checks that the waiting worker is genuinely a different build + * before it swaps -- which matters, because the vendored editor registers a + * worker of its own into this scope from inside the iframe, so ours is left + * `waiting` on almost every editor load with no new build in sight. + */ +export function documentIsExpected(search: string): boolean { + const params = new URLSearchParams(search); + return ['new', 'file', 'src', 'open', 'saved', 'embed', 'embedded'].some((key) => params.has(key)); +} + /** Tell a waiting worker to take over -- only if nothing is open. Returns whether it did. */ export function promoteWaitingWorker( reg: RegistrationLike, @@ -68,18 +94,27 @@ export function promoteWaitingWorker( * future ones as soon as they finish installing (both gated on "no document * open"). A worker that stays waiting because a document is open activates on * the next visit, when the landing page calls this again. + * + * `onPromoted` fires when this tab actually told a worker to take over, and + * the caller must not drop it: the swap tears down whatever the outgoing + * worker was still fetching, so the reload afterwards is not a nicety. See + * shouldReloadOnControllerChange for what happened when it was dropped. */ export function wireServiceWorkerUpdates( reg: RegistrationLike, hasOpenDocument: () => boolean, ownScriptURL?: string, + onPromoted?: () => void, ): void { - promoteWaitingWorker(reg, hasOpenDocument, ownScriptURL); + const promote = (): void => { + if (promoteWaitingWorker(reg, hasOpenDocument, ownScriptURL)) onPromoted?.(); + }; + promote(); reg.addEventListener('updatefound', () => { const installing = reg.installing; if (!installing) return; installing.addEventListener('statechange', () => { - if (installing.state === 'installed') promoteWaitingWorker(reg, hasOpenDocument, ownScriptURL); + if (installing.state === 'installed') promote(); }); }); } @@ -118,17 +153,32 @@ export function onWaitingWorker( * Whether a controllerchange should reload the page: only when a worker was * already in control at startup (so this is an update, not the first * install), only once, and never with a document open (unsaved edits). + * + * Unless this tab is the one that asked for the swap -- then it reloads + * regardless of what is open, short of unsaved edits. That is not a + * preference, it is repair. Activating a worker terminates the outgoing one, + * and every request it still had in flight fails: on the editor route that is + * the vendored iframe's own document, which nothing retries, so the tab is + * left staring at a blank editor forever. + * + * The two decisions used to be made at different times against a predicate + * that changes underneath them. `promoteWaitingWorker` runs the moment + * `register()` resolves, which on the editor route is a few hundred + * milliseconds BEFORE the editor instance exists -- so "no document open" is + * true and it promotes. By the time the swap lands, the document is open, and + * the reload was refused. The page had already been torn in half by then. + * Whoever promotes owns the reload. */ export function shouldReloadOnControllerChange(state: { hadController: boolean; alreadyReloading: boolean; hasOpenDocument: boolean; - /** This tab asked an older worker to step aside; the reload is the point. */ - healingStaleBuild?: boolean; + /** This tab told a waiting worker to take over -- an update or a heal. */ + promotedFromThisTab?: boolean; hasUnsavedChanges?: boolean; }): boolean { if (!state.hadController || state.alreadyReloading) return false; - if (state.healingStaleBuild) return !state.hasUnsavedChanges; + if (state.promotedFromThisTab) return !state.hasUnsavedChanges; return !state.hasOpenDocument; } diff --git a/test/unit/sw-update.test.ts b/test/unit/sw-update.test.ts index 7255e8de..99e63b09 100644 --- a/test/unit/sw-update.test.ts +++ b/test/unit/sw-update.test.ts @@ -8,6 +8,7 @@ import { healStaleController, isUnseenBuild, onWaitingWorker, + documentIsExpected, promoteWaitingWorker, shouldReloadOnControllerChange, wireServiceWorkerUpdates, @@ -93,6 +94,62 @@ describe('wireServiceWorkerUpdates', () => { w.listeners.forEach((cb) => cb()); expect(w.postMessage).not.toHaveBeenCalled(); }); + + /** + * The caller has to learn that this tab caused the swap, because the reload + * that follows is repair rather than a courtesy -- see the race described on + * shouldReloadOnControllerChange. + */ + it('reports a promotion it made on arrival', () => { + const w = worker('installed'); + const onPromoted = vi.fn(); + wireServiceWorkerUpdates(registration(w), () => false, undefined, onPromoted); + expect(onPromoted).toHaveBeenCalled(); + }); + + it('reports a promotion it made after a later install', () => { + const r = registration(); + const onPromoted = vi.fn(); + wireServiceWorkerUpdates(r, () => false, undefined, onPromoted); + const w = worker('installing'); + r.installing = w; + r.updateListeners.forEach((cb) => cb()); + w.state = 'installed'; + r.waiting = w; + w.listeners.forEach((cb) => cb()); + expect(onPromoted).toHaveBeenCalled(); + }); + + it('reports nothing when it left the worker waiting', () => { + const onPromoted = vi.fn(); + wireServiceWorkerUpdates(registration(worker('installed')), () => true, undefined, onPromoted); + expect(onPromoted).not.toHaveBeenCalled(); + }); +}); + +/** + * The brake on the rule above. Promotion runs before the editor exists, so + * "nothing is open" is true on a page that is opening something -- and the + * vendored editor keeps our worker in `waiting` on almost every editor load + * without any new build existing, so treating each of those as a promotion + * worth reloading for is a reload on every load. + */ +describe('documentIsExpected', () => { + it('recognises every route that mounts a document', () => { + for (const search of ['?new=docx', '?file=https://x/a.docx', '?src=https://x/a.docx', '?open=local', '?saved=abc']) + expect(documentIsExpected(search), search).toBe(true); + }); + + it('counts embed mode, where the host can push one at any moment', () => { + // Reloading an embedded editor would throw away the host page's document. + expect(documentIsExpected('?embed=1')).toBe(true); + expect(documentIsExpected('?embedded=1')).toBe(true); + }); + + it('leaves a page with nothing to open free to take an update', () => { + expect(documentIsExpected('')).toBe(false); + expect(documentIsExpected('?locale=ja')).toBe(false); + }); }); describe('shouldReloadOnControllerChange', () => { @@ -713,14 +770,33 @@ describe('healStaleController', () => { }); }); -describe('shouldReloadOnControllerChange during a heal', () => { +describe('shouldReloadOnControllerChange when this tab asked for the swap', () => { it('reloads even with a document open -- the reload is the point', () => { expect( shouldReloadOnControllerChange({ hadController: true, alreadyReloading: false, hasOpenDocument: true, - healingStaleBuild: true, + promotedFromThisTab: true, + }), + ).toBe(true); + }); + + /** + * The exact race that left a tab on a blank editor: promotion runs when + * `register()` resolves, a few hundred milliseconds before the editor + * instance exists, so it sees nothing open and promotes. The document is + * open by the time the swap lands. Re-asking "is a document open?" at that + * point refuses the reload -- and the swap has already aborted the vendored + * iframe's own request, which nothing retries. + */ + it('reloads a document that was opened between the promotion and the swap', () => { + expect( + shouldReloadOnControllerChange({ + hadController: true, + alreadyReloading: false, + hasOpenDocument: true, + promotedFromThisTab: true, }), ).toBe(true); }); @@ -731,7 +807,7 @@ describe('shouldReloadOnControllerChange during a heal', () => { hadController: true, alreadyReloading: false, hasOpenDocument: true, - healingStaleBuild: true, + promotedFromThisTab: true, hasUnsavedChanges: true, }), ).toBe(false);