Skip to content

Commit 989a080

Browse files
fix(server): bound the preview status title read
Navigate now returns the committed page while load is pending, which reads the title on a page that may still be running script. page.title() waits on the renderer's main thread, so a busy page held the reply past the broker's grace and evicted the shared server preview host. Cap the read at 250 ms and report no title when it does not answer. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent b8a7a3d commit 989a080

2 files changed

Lines changed: 51 additions & 33 deletions

File tree

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

Lines changed: 41 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -440,42 +440,51 @@ it.live("preview_open reports its navigation timeout instead of the request budg
440440
).pipe(Effect.provide(layer)),
441441
);
442442

443-
it.live("navigate returns the committed page while its load event is still pending", () =>
444-
Effect.scoped(
445-
Effect.gen(function* () {
446-
const { broker, tabId } = yield* ready;
447-
const page = contexts[0]!.page;
448-
const heavy = "http://localhost:5173/heavy";
449-
const timedOut = (method: string, timeout: number | undefined) =>
450-
Object.assign(new Error(`${method}: Timeout ${timeout}ms exceeded.`), {
451-
name: "TimeoutError",
443+
it.live.each([
444+
{ renderer: "idle", title: "Heavy page" },
445+
{ renderer: "busy", title: null },
446+
])(
447+
"navigate returns the committed page while its load event is still pending ($renderer renderer)",
448+
({ renderer, title }) =>
449+
Effect.scoped(
450+
Effect.gen(function* () {
451+
const { broker, tabId } = yield* ready;
452+
const page = contexts[0]!.page;
453+
// A busy main thread never answers title(); the status must not wait past the broker's grace.
454+
page.title.mockImplementation(() =>
455+
renderer === "busy" ? new Promise<string>(() => {}) : Promise.resolve("Heavy page"),
456+
);
457+
const heavy = "http://localhost:5173/heavy";
458+
const timedOut = (method: string, timeout: number | undefined) =>
459+
Object.assign(new Error(`${method}: Timeout ${timeout}ms exceeded.`), {
460+
name: "TimeoutError",
461+
});
462+
// The document commits and parses, but a stalled subresource holds back `load`.
463+
page.goto.mockImplementation(async (next, options) => {
464+
page.commit(next);
465+
page.emit("request", {
466+
url: () => next,
467+
isNavigationRequest: () => true,
468+
frame: () => page,
469+
});
470+
if (options?.waitUntil === "load") throw timedOut("page.goto", options.timeout);
452471
});
453-
// The document commits and parses, but a stalled subresource holds back `load`.
454-
page.goto.mockImplementation(async (next, options) => {
455-
page.commit(next);
456-
page.emit("request", {
457-
url: () => next,
458-
isNavigationRequest: () => true,
459-
frame: () => page,
472+
page.waitForLoadState.mockImplementation(async (state, options) => {
473+
if (state === undefined || state === "load") {
474+
throw timedOut("page.waitForLoadState", options?.timeout);
475+
}
460476
});
461-
if (options?.waitUntil === "load") throw timedOut("page.goto", options.timeout);
462-
});
463-
page.waitForLoadState.mockImplementation(async (state, options) => {
464-
if (state === undefined || state === "load") {
465-
throw timedOut("page.waitForLoadState", options?.timeout);
466-
}
467-
});
468477

469-
const status = yield* broker.invoke<PreviewAutomationStatus>({
470-
scope,
471-
tabId,
472-
operation: "navigate",
473-
input: { url: heavy, timeoutMs: 4_000 },
474-
});
478+
const status = yield* broker.invoke<PreviewAutomationStatus>({
479+
scope,
480+
tabId,
481+
operation: "navigate",
482+
input: { url: heavy, timeoutMs: 4_000 },
483+
});
475484

476-
expect(status).toMatchObject({ url: heavy, loading: true });
477-
}),
478-
).pipe(Effect.provide(layer)),
485+
expect(status).toMatchObject({ url: heavy, title, loading: true });
486+
}),
487+
).pipe(Effect.provide(layer)),
479488
);
480489

481490
it.live("navigate fails when the page never commits within its timeout", () =>

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

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ const EDITABLE_AT_POINT_SCRIPT = `(x, y) => {
123123
}`;
124124
const UNATTACHED_FILL_VIEWPORT = { width: 1280, height: 800 } as const;
125125
const NAVIGATION_TIMEOUT_MS = 15_000;
126+
// page.title() runs in the renderer; a busy page must not hold a status past the broker's grace.
127+
const TITLE_READ_MS = 250;
126128
const VIEWER_NAVIGATION_OPTIONS = { waitUntil: "commit", timeout: NAVIGATION_TIMEOUT_MS } as const;
127129
const ACTION_TIMELINE_LIMIT = 50;
128130

@@ -1401,7 +1403,14 @@ const make = Effect.gen(function* () {
14011403
})),
14021404
};
14031405
if (status.url === null || tab.dialog) return status;
1404-
return { ...status, title: (await tab.page.title().catch(() => "")) || null };
1406+
let timer: ReturnType<typeof setTimeout> | undefined;
1407+
const late = new Promise<string>((resolve) => {
1408+
timer = setTimeout(() => resolve(""), TITLE_READ_MS);
1409+
});
1410+
const title = await Promise.race([tab.page.title().catch(() => ""), late]).finally(() =>
1411+
clearTimeout(timer),
1412+
);
1413+
return { ...status, title: title || null };
14051414
};
14061415

14071416
const resolveNavigationUrl = (input: PreviewAutomationNavigateInput) => {

0 commit comments

Comments
 (0)