Skip to content

Commit b78eff7

Browse files
fix(server): a server browser tab survives a renderer crash, and a service worker badge call no longer causes one
chrome-headless-shell binds no BadgeService for service workers, so a site whose worker calls navigator.setAppBadge had its renderer killed for a bad Mojo message. The crash handler then deleted the tab and closed its session, so the Browser tab vanished and the agent's next call found no tab. The server browser now launches with --disable-kill-after-bad-ipc, so the unbound call is rejected without killing the renderer and such sites load. When a renderer does crash, the tab stays: its session reports the crash as a failed load, a fresh page replaces the dead one, viewers reconnect to it, and Reload loads the crashed URL again. Agent status reports the crash as the tab's load error. Fixes #21022 Co-Authored-By: Claude Fable 5.2 <[email protected]>
1 parent 145a35b commit b78eff7

3 files changed

Lines changed: 66 additions & 7 deletions

File tree

‎apps/server/src/preview/ServerBrowser.test.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,15 +1809,23 @@ it.effect("closing a tab with an unsaved-changes guard still closes it", () =>
18091809
it.effect("a tab whose renderer crashes stays open instead of vanishing", () =>
18101810
Effect.scoped(
18111811
Effect.gen(function* () {
1812-
const { broker, tabId } = yield* ready;
1813-
contexts[0]!.page.emit("crash");
1812+
const { browser, broker, tabId } = yield* ready;
1813+
const url = "https://example.com/app";
1814+
contexts[0]!.page.commit(url);
1815+
yield* Effect.promise(() => contexts[0]!.page.emitAsync("crash"));
18141816
const status = yield* broker.invoke<PreviewAutomationStatus>({
18151817
scope,
18161818
tabId,
18171819
operation: "status",
18181820
input: {},
18191821
});
18201822
expect(status.tabId).toBe(tabId);
1823+
expect(status.loadError).toEqual({ url, code: -2, description: "Page crashed" });
1824+
// The crashed page cannot load anything again, so Reload goes to the page that replaced it.
1825+
const viewer = yield* browser.attachViewer(viewerInput(tabId, true));
1826+
yield* viewer.input({ type: "takeControl" });
1827+
yield* viewer.input({ type: "reload" });
1828+
expect(contexts[1]!.page.goto).toHaveBeenCalledWith(url, expect.anything());
18211829
}),
18221830
).pipe(Effect.provide(layer)),
18231831
);

‎apps/server/src/preview/ServerBrowser.ts‎

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,8 @@ interface ServerTab {
377377
desktopResize: PromiseWithResolvers<void>;
378378
loading: boolean;
379379
loadError: PreviewAutomationLoadError | undefined;
380+
/** The renderer crash this page died of, or the one the page it replaced died of. */
381+
crash: PreviewAutomationLoadError | undefined;
380382
closing: boolean;
381383
recording: Recording | null;
382384
initialNavigation: Promise<void> | null;
@@ -586,6 +588,34 @@ const make = Effect.gen(function* () {
586588
},
587589
});
588590

591+
// Playwright cannot reload a crashed page, so a fresh page takes over its tab.
592+
const reopenCrashedTab = async (tab: ServerTab) => {
593+
if (tab.desktop) return dropTab(tab, true);
594+
if (tabs.get(tabKey(tab.threadId, tab.tabId)) !== tab) return;
595+
tab.crash = {
596+
url: ServerBrowserPage.withoutDataPayload(tab.page.url()).slice(0, 2048),
597+
code: -2,
598+
description: "Page crashed",
599+
};
600+
// The session must name the crash before a new page reads it, or it would load the URL again.
601+
await runPromise(
602+
manager
603+
.reportStatus({
604+
threadId: tab.threadId,
605+
tabId: tab.tabId,
606+
serverControlled: true,
607+
navStatus: { _tag: "LoadFailed", title: "", ...tab.crash },
608+
canGoBack: false,
609+
canGoForward: false,
610+
})
611+
.pipe(Effect.ignore),
612+
);
613+
dropTab(tab, false, true);
614+
// A context that is still closing would take the new page down with it.
615+
if (tab.isolatedContext) await tab.page.context().close().catch(constVoid);
616+
await runPromise(findTab(tab.threadId, tab.tabId).pipe(Effect.ignore));
617+
};
618+
589619
const dialogStatus = (tab: ServerTab) =>
590620
tab.dialog
591621
? {
@@ -638,6 +668,8 @@ const make = Effect.gen(function* () {
638668
.send("Page.getNavigationHistory")
639669
.catch(() => null)
640670
.then((history) => {
671+
// A status the dead page still had in flight must not replace the crash.
672+
if (tab.crash) return;
641673
const index = history?.currentIndex ?? 0;
642674
const count = history?.entries.length ?? 0;
643675
runFork(
@@ -832,15 +864,16 @@ const make = Effect.gen(function* () {
832864
clearTimeout(timeout);
833865
};
834866

835-
const dropTab = (tab: ServerTab, closeSession: boolean) => {
867+
const dropTab = (tab: ServerTab, closeSession: boolean, reopening = false) => {
836868
const key = tabKey(tab.threadId, tab.tabId);
837869
if (tabs.get(key) !== tab) return;
838870
tabs.delete(key);
839871
tab.closing = true;
840872
tab.viewerAttached?.resolve();
841873
clearAbortedNavigation(tab);
842874
// A desktop page outlives the connection unless its session closed with it.
843-
const end = tab.desktop && !closeSession && sessionOpen(tab) ? "reconnect" : "gone";
875+
const end =
876+
(tab.desktop || reopening) && !closeSession && sessionOpen(tab) ? "reconnect" : "gone";
844877
for (const viewer of tab.viewers) viewer.push({ _tag: end });
845878
void tab.control.close().catch(constVoid);
846879
if (tab.desktop) void tab.desktop.close().catch(constVoid);
@@ -855,7 +888,8 @@ const make = Effect.gen(function* () {
855888
const forceClose = setTimeout(() => void page.close().catch(constVoid), PAGE_CLOSE_GRACE_MS);
856889
void closed.then(() => clearTimeout(forceClose));
857890
void page
858-
.close({ runBeforeUnload: tab.dialog === null })
891+
// A crashed page never answers `beforeunload`, so that close would not finish.
892+
.close({ runBeforeUnload: tab.dialog === null && tab.crash === undefined })
859893
.catch(() => page.close())
860894
.catch(constVoid);
861895
if (tab.isolatedContext) void closed.then(() => page.context().close()).catch(constVoid);
@@ -1009,6 +1043,15 @@ const make = Effect.gen(function* () {
10091043
desktopResize: Promise.withResolvers<void>(),
10101044
loading: false,
10111045
loadError: undefined,
1046+
// The page a crash left behind is blank; the session still names what crashed.
1047+
crash:
1048+
!desktop && !adopted && snapshot.navStatus._tag === "LoadFailed"
1049+
? {
1050+
url: snapshot.navStatus.url,
1051+
code: snapshot.navStatus.code,
1052+
description: snapshot.navStatus.description,
1053+
}
1054+
: undefined,
10121055
closing: false,
10131056
recording: null,
10141057
recordingStart: null,
@@ -1060,6 +1103,7 @@ const make = Effect.gen(function* () {
10601103
clearAbortedNavigation(tab);
10611104
tab.loading = true;
10621105
tab.loadError = undefined;
1106+
tab.crash = undefined;
10631107
if (stoppedInitialRequest) {
10641108
tab.stoppedGeneration = tab.navigationGeneration;
10651109
settleStoppedNavigation(tab, tab.navigationGeneration);
@@ -1158,14 +1202,13 @@ const make = Effect.gen(function* () {
11581202
updateFileChooserInterception(tab);
11591203
// Popups become tabs and keep `window.opener`, so sign-in popups can report back.
11601204
page.on("popup", (popup) => void adoptPopup(tab, popup));
1161-
// Playwright cannot reload a crashed page, so it must leave the tab list.
11621205
const key = tabKey(tab.threadId, tab.tabId);
11631206
if (closedPendingTabs.delete(key)) {
11641207
await control.close().catch(constVoid);
11651208
throw new ServerBrowserTabNotFoundError({ threadId: tab.threadId, tabId: tab.tabId });
11661209
}
11671210
page.on("close", () => dropTab(tab, true));
1168-
page.on("crash", () => dropTab(tab, true));
1211+
page.on("crash", () => reopenCrashedTab(tab));
11691212
tabs.set(key, tab);
11701213
reportLiveTabs();
11711214
// A desktop page navigates on its own and can finish loading before these listeners attach.
@@ -1828,6 +1871,7 @@ const make = Effect.gen(function* () {
18281871
};
18291872

18301873
const loadErrorOf = (tab: ServerTab): PreviewAutomationLoadError | undefined => {
1874+
if (tab.crash) return tab.crash;
18311875
if (!tab.page.url().startsWith("chrome-error://")) return undefined;
18321876
// A desktop failure from before CDP attached is superseded by any navigation we observe.
18331877
if (tab.loadError || !tab.desktop || tab.navigationGeneration > 0) return tab.loadError;
@@ -2908,6 +2952,10 @@ const make = Effect.gen(function* () {
29082952
}
29092953
case "reload":
29102954
tab.pendingInitialRequestUrl = null;
2955+
if (tab.crash) {
2956+
tab.control.trackUntilHandoff(tab.page.goto(tab.crash.url, VIEWER_NAVIGATION_OPTIONS));
2957+
return;
2958+
}
29112959
if (message.ignoreCache === true) {
29122960
await session.send("Page.reload", { ignoreCache: true });
29132961
return;

‎apps/server/src/preview/ServerBrowserContexts.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,9 @@ export class ServerBrowserContexts {
9999
"--disable-gpu",
100100
"--force-device-scale-factor=2",
101101
"--disable-blink-features=AutomationControlled",
102+
// The headless shell binds no BadgeService for service workers and would kill the
103+
// renderer of any site whose worker calls navigator.setAppBadge.
104+
"--disable-kill-after-bad-ipc",
102105
],
103106
ignoreDefaultArgs: ["--enable-automation"],
104107
headless: true,

0 commit comments

Comments
 (0)