Skip to content
36 changes: 36 additions & 0 deletions apps/server/src/mcp/PreviewAutomationBroker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,42 @@ it.effect.each([
),
);

it.effect("names the click target that matched nothing when the server browser times out", () =>
Effect.scoped(
Effect.gen(function* () {
const broker = yield* makeBroker;
const clientId = SERVER_BROWSER_AUTOMATION_CLIENT_ID;
const requests = requestsFrom(yield* broker.connect(makeHost({ clientId })));
yield* Stream.runForEach(requests, (request) =>
broker.respond({
clientId,
connectionId: request.connectionId,
requestId: request.requestId,
ok: false,
error: {
_tag: "PreviewAutomationTimeoutError",
message:
'No element matches ".definitely-no-such-t3-target" after the timeout. The click may have happened and removed its target or navigated; check the page before retrying, then take a fresh snapshot and use its locator.',
},
}),
).pipe(Effect.forkScoped);
yield* Effect.yieldNow;

const error = yield* broker
.invoke<void>({
scope,
operation: "click",
input: { selector: ".definitely-no-such-t3-target", timeoutMs: 15_000 },
})
.pipe(Effect.flip);

expect(error._tag).toBe("PreviewAutomationTimeoutError");
expect(error.message).toContain(".definitely-no-such-t3-target");
expect(error.message).toMatch(/no (element|match)|matched no|not found/i);
}),
),
);

it.effect("distinguishes malformed remote failures", () =>
Effect.scoped(
Effect.gen(function* () {
Expand Down
13 changes: 10 additions & 3 deletions apps/server/src/mcp/PreviewAutomationBroker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,14 @@ function remoteDetailKind(detail: unknown): RemoteDetailKind {

const MAX_REASON_CHARS = 500;

const serverBrowserReason = (
context: PreviewAutomationRequestErrorContext,
error: NonNullable<PreviewAutomationResponse["error"]>,
) =>
context.clientId === SERVER_BROWSER_AUTOMATION_CLIENT_ID
? { reason: error.message.slice(0, MAX_REASON_CHARS) }
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
: {};

const classifyResponseError = (
context: PreviewAutomationRequestErrorContext,
error: NonNullable<PreviewAutomationResponse["error"]>,
Expand Down Expand Up @@ -257,6 +265,7 @@ const classifyResponseError = (
return new PreviewAutomationTimeoutError({
...context,
...remoteDiagnostics,
...serverBrowserReason(context, error),
});
case "PreviewAutomationControlInterruptedError": {
const reason = decodeControlReason(error.detail);
Expand Down Expand Up @@ -334,9 +343,7 @@ const classifyResponseError = (
return new PreviewAutomationExecutionError({
...context,
...remoteDiagnostics,
...(context.clientId === SERVER_BROWSER_AUTOMATION_CLIENT_ID
? { reason: error.message.slice(0, MAX_REASON_CHARS) }
: {}),
...serverBrowserReason(context, error),
});
}
};
Expand Down
21 changes: 21 additions & 0 deletions apps/server/src/preview/ServerBrowserPage.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,27 @@ describe("server browser element refs", () => {
expect(await page.locator("li").count()).toBe(5);
});

it("tells a click target that matches nothing from one that never becomes clickable", async () => {
await page.setContent(`<button hidden>hidden</button>`);
const failure = (selector: string) =>
ServerBrowserPage.click(page, { selector, timeoutMs: 500 }).then(
() => expect.unreachable(),
ServerBrowserPage.toOperationError,
);

const absent = await failure(".definitely-no-such-t3-target");
expect(absent.tag).toBe("PreviewAutomationTimeoutError");
expect(absent.message).toBe(
'No element matches ".definitely-no-such-t3-target" after the timeout. The click may have happened and removed its target or navigated; check the page before retrying, then take a fresh snapshot and use its locator.',
);
expect(absent.cause).toMatchObject({ name: "TimeoutError" });
const hidden = await failure("button");
expect(hidden.tag).toBe("PreviewAutomationTimeoutError");
expect(hidden.message).toBe(
'"button" still matches an element after the timeout. The click may have happened; check the page before retrying.',
);
});

it("does not retarget a removed ref to a replacement node", async () => {
await page.setContent("<button onclick=\"this.textContent='clicked'\">original</button>");
const locator = buttonLocator((await takeSnapshot()).accessibilityTree, "original");
Expand Down
51 changes: 49 additions & 2 deletions apps/server/src/preview/ServerBrowserPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ export class ServerBrowserOperationError extends Error {
readonly tag: string;
readonly detail: unknown;

constructor(tag: string, message: string, detail?: unknown) {
super(message);
constructor(tag: string, message: string, detail?: unknown, options?: ErrorOptions) {
super(message, options);
this.tag = tag;
this.detail = detail;
}
Expand Down Expand Up @@ -64,6 +64,7 @@ export const toOperationError = (cause: unknown): ServerBrowserOperationError =>
};

const DEFAULT_TIMEOUT_MS = 15_000;
const TARGET_COUNT_MS = 250;

const pageRefs = new WeakMap<Page, { generation: string; refs: Map<string, string> }>();
// A compact runtime namespace prevents old refs from aliasing after a server restart.
Expand Down Expand Up @@ -650,6 +651,52 @@ export const click = async (
): Promise<{ readonly x: number; readonly y: number }> => {
const timeout = input.timeoutMs ?? DEFAULT_TIMEOUT_MS;
const locator = targetLocator(page, input);
try {
return await clickTarget(page, locator, input, timeout, pointer);
} catch (cause) {
throw locator === null ? cause : await namedTargetTimeout(locator, input, cause);
}
};

/**
* Playwright's timeout names neither the target nor why it waited. Counting only after the
* wait failed keeps auto-wait intact, but the count describes the page after the timeout, not
* what the click did: Playwright can dispatch the click and then time out waiting for the
* navigation it started.
*/
const namedTargetTimeout = async (
locator: Locator,
input: { readonly locator?: string | undefined; readonly selector?: string | undefined },
cause: unknown,
) => {
if (!(cause instanceof Error && cause.name === "TimeoutError")) return cause;
const target = JSON.stringify(input.locator ?? input.selector);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
// The host's reply must still beat the broker's short grace after the operation timeout.
let timer: ReturnType<typeof setTimeout> | undefined;
const matches = await Promise.race([
locator.count().catch(() => undefined),
new Promise<undefined>((resolve) => {
timer = setTimeout(() => resolve(undefined), TARGET_COUNT_MS);
}),
]).finally(() => clearTimeout(timer));
if (matches === undefined) return cause;
Comment thread
juliusmarminge marked this conversation as resolved.
Resolved
return new ServerBrowserOperationError(
"PreviewAutomationTimeoutError",
matches === 0
? `No element matches ${target} after the timeout. The click may have happened and removed its target or navigated; check the page before retrying, then take a fresh snapshot and use its locator.`
: `${target} still matches an element after the timeout. The click may have happened; check the page before retrying.`,
undefined,
{ cause },
);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
};

const clickTarget = async (
page: Page,
locator: Locator | null,
input: PreviewAutomationClickInput,
timeout: number,
pointer: PointerReporter,
) => {
const point = await targetPoint(page, locator, input, timeout);
await pointer(point, "click");
const options = { button: input.button ?? "left", clickCount: input.clickCount ?? 1 } as const;
Expand Down
3 changes: 2 additions & 1 deletion packages/contracts/src/previewAutomation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -946,11 +946,12 @@ export class PreviewAutomationTimeoutError extends Schema.TaggedError<PreviewAut
{
...PreviewAutomationRequestErrorFields,
...PreviewAutomationOptionalRemoteDiagnosticFields,
reason: Schema.optional(Schema.String),
},
) {
override get message(): string {
const summary = `Preview automation ${this.operation} timed out after ${this.timeoutMs}ms.`;
return summary;
return this.reason === undefined ? summary : `${summary} ${this.reason}`;
}
}

Expand Down
Loading