Skip to content

Commit 4061c4a

Browse files
fix(server): agent navigation probes alongside goto, not before it
Probing before goto took time from a cross-site navigation that would otherwise have left the hung page in a new renderer, so it could miss a deadline that main met. goto now starts at once with the caller's full timeout, and the probe runs alongside it to stop a hung same-site script. AI-assisted; verified with the tests and tools listed above. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 1678414 commit 4061c4a

2 files changed

Lines changed: 11 additions & 32 deletions

File tree

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

Lines changed: 7 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,6 @@ function makeRenderer() {
100100
return {
101101
/** A page script that never yields; Chromium stops it on `Runtime.terminateExecution`. */
102102
busyScript: null as PromiseWithResolvers<void> | null,
103-
gotoStarted: Promise.withResolvers<void>(),
104-
/** Rejecting these stands in for Playwright's goto timeout while the script runs. */
105-
stalledGotos: [] as Array<(cause: Error) => void>,
106103
};
107104
}
108105

@@ -229,14 +226,8 @@ function makeContext(onClose?: (context: BrowserContext) => void, initialUrl = "
229226
viewport = size;
230227
}),
231228
goto: vi.fn(async (next: string, _options?: { timeout?: number }) => {
232-
const busyScript = renderer.busyScript;
233-
renderer.gotoStarted.resolve();
234-
if (busyScript) {
235-
await new Promise<void>((resolve, reject) => {
236-
renderer.stalledGotos.push(reject);
237-
void busyScript.promise.then(resolve);
238-
});
239-
}
229+
// A same-site goto commits only once the page's script stops.
230+
await renderer.busyScript?.promise;
240231
url = next;
241232
events.emit("load");
242233
const pending = initialNavigation;
@@ -979,31 +970,21 @@ it.live.each(["load", "none"] as const)(
979970
sessions.flatMap((session) => session.send.mock.calls.map(([method]) => method)),
980971
).not.toContain("Runtime.terminateExecution");
981972
renderer.busyScript = Promise.withResolvers<void>();
982-
renderer.gotoStarted = Promise.withResolvers<void>();
983973

984-
const navigation = yield* broker
974+
const navigated = yield* broker
985975
.invoke<PreviewAutomationStatus>({
986976
scope,
987977
tabId,
988978
operation: "navigate",
989979
input: { url: "http://localhost:5173/next", readiness, timeoutMs: 200 },
990980
})
991-
.pipe(Effect.exit, Effect.forkScoped);
992-
yield* Effect.promise(() => renderer.gotoStarted.promise);
993-
for (const timeOut of renderer.stalledGotos.splice(0)) {
994-
timeOut(
995-
Object.assign(new Error("page.goto: Timeout 200ms exceeded."), {
996-
name: "TimeoutError",
997-
}),
998-
);
999-
}
1000-
const navigated = yield* Fiber.join(navigation);
981+
.pipe(Effect.exit);
1001982
const waited = yield* broker
1002983
.invoke({
1003984
scope,
1004985
tabId,
1005986
operation: "waitFor",
1006-
input: { urlIncludes: "/next", timeoutMs: 100 },
987+
input: { urlIncludes: "/next", timeoutMs: 1_000 },
1007988
})
1008989
.pipe(Effect.exit);
1009990
const status = yield* broker.invoke<PreviewAutomationStatus>({
@@ -1015,10 +996,9 @@ it.live.each(["load", "none"] as const)(
1015996

1016997
expect(status.url).toBe("http://localhost:5173/next");
1017998
expect(navigated).toMatchObject({ _tag: "Success" });
1018-
// Waiting on the busy script spends half the navigation's own budget, not all of it.
999+
// A cross-site goto can leave a hung page by itself, so the probe takes none of its budget.
10191000
const [, gotoOptions] = contexts[0]!.page.goto.mock.calls.at(-1)!;
1020-
expect(gotoOptions?.timeout).toBeLessThan(200);
1021-
expect(gotoOptions?.timeout).toBeGreaterThan(1);
1001+
expect(gotoOptions?.timeout).toBe(200);
10221002
expect(waited).toMatchObject({ _tag: "Success" });
10231003
}),
10241004
).pipe(Effect.provide(layer)),

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1784,19 +1784,18 @@ const make = Effect.gen(function* () {
17841784
readiness: "load" | "domContentLoaded" | "none",
17851785
timeout: number,
17861786
) => {
1787-
const probeStartedAt = performance.now();
1788-
// Half the budget at most: a cross-site goto leaves a hung page by itself and needs the rest.
1789-
await ServerBrowserPage.stopUnresponsiveScript(tab.cdp, timeout / 2);
17901787
const navigation = tab.page.goto(url, {
1791-
// The probe spends the caller's budget; the broker evicts a host that answers late.
1792-
timeout: Math.max(1, timeout - (performance.now() - probeStartedAt)),
1788+
timeout,
17931789
waitUntil:
17941790
readiness === "domContentLoaded"
17951791
? "domcontentloaded"
17961792
: readiness === "none"
17971793
? "commit"
17981794
: "load",
17991795
});
1796+
// Alongside goto, not before it: a cross-site goto leaves a hung page by itself in a new
1797+
// renderer, while a same-site one commits once the hung script is stopped.
1798+
void ServerBrowserPage.stopUnresponsiveScript(tab.cdp, timeout / 2);
18001799
if (readiness === "none") {
18011800
tab.control.track(navigation);
18021801
return;

0 commit comments

Comments
 (0)