fix(browser): preserve operation-aware cancellation errors
This commit is contained in:
parent
3743962071
commit
0aac8aaba5
|
|
@ -328,6 +328,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
|||
return await capturePage({
|
||||
connection: targetSession.connection,
|
||||
sessionId: targetSession.sessionId,
|
||||
operation,
|
||||
discovery,
|
||||
target,
|
||||
input,
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import {
|
||||
AnalysisCancelledError,
|
||||
BrowserObservationError,
|
||||
type BrowserObservationOperation,
|
||||
} from "../domain/errors.js";
|
||||
import { sanitizeBrowserUrl } from "../domain/browserObservation.js";
|
||||
import { createWebTextArtifact } from "../domain/webContentArtifact.js";
|
||||
|
|
@ -74,11 +75,11 @@ export const sourceExcluded = (
|
|||
|
||||
export const delayWithCancellation = async (
|
||||
durationMs: number,
|
||||
operation: BrowserObservationOperation,
|
||||
signal?: AbortSignal,
|
||||
): Promise<void> => {
|
||||
if (durationMs === 0) return;
|
||||
if (signal?.aborted === true)
|
||||
throw new AnalysisCancelledError("inspect_web_page");
|
||||
if (signal?.aborted === true) throw new AnalysisCancelledError(operation);
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
signal?.removeEventListener("abort", onAbort);
|
||||
|
|
@ -87,7 +88,7 @@ export const delayWithCancellation = async (
|
|||
const onAbort = (): void => {
|
||||
clearTimeout(timer);
|
||||
signal?.removeEventListener("abort", onAbort);
|
||||
reject(new AnalysisCancelledError("inspect_web_page"));
|
||||
reject(new AnalysisCancelledError(operation));
|
||||
};
|
||||
signal?.addEventListener("abort", onAbort, { once: true });
|
||||
if (signal?.aborted === true) onAbort();
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import {
|
|||
AnalysisError,
|
||||
AnalysisTimeoutError,
|
||||
BrowserObservationError,
|
||||
type BrowserObservationOperation,
|
||||
} from "../domain/errors.js";
|
||||
|
||||
export interface CdpEvent {
|
||||
|
|
@ -25,7 +26,7 @@ const COMMAND_TIMEOUT_MS = 5_000;
|
|||
const MAX_PAYLOAD_BYTES = 16 * 1_024 * 1_024;
|
||||
const MAX_PENDING_COMMANDS = 128;
|
||||
|
||||
/** Correlated, bounded JSON-RPC transport for a browser-level CDP socket. */
|
||||
/** Correlated, bounded JSON-RPC transport for one CDP target operation. */
|
||||
export class CdpConnection {
|
||||
readonly #pending = new Map<number, PendingCommand>();
|
||||
readonly #listeners = new Set<(event: CdpEvent) => void>();
|
||||
|
|
@ -33,26 +34,29 @@ export class CdpConnection {
|
|||
#closed = false;
|
||||
#protocolFailed = false;
|
||||
|
||||
private constructor(private readonly socket: WebSocket) {
|
||||
private constructor(
|
||||
private readonly socket: WebSocket,
|
||||
private readonly operation: BrowserObservationOperation,
|
||||
) {
|
||||
socket.on("message", (data) => this.#receive(data));
|
||||
socket.on("close", () => this.#failPending("disconnected"));
|
||||
socket.on("error", () => this.#failPending("disconnected"));
|
||||
}
|
||||
|
||||
/** Connect to one already-validated loopback browser WebSocket. */
|
||||
/** Connect to one already-validated loopback CDP WebSocket. */
|
||||
static async connect(
|
||||
url: string,
|
||||
operation: BrowserObservationOperation,
|
||||
signal?: AbortSignal,
|
||||
): Promise<CdpConnection> {
|
||||
if (signal?.aborted === true)
|
||||
throw new AnalysisCancelledError("inspect_web_page");
|
||||
if (signal?.aborted === true) throw new AnalysisCancelledError(operation);
|
||||
const socket = new WebSocket(url, {
|
||||
handshakeTimeout: CONNECT_TIMEOUT_MS,
|
||||
maxPayload: MAX_PAYLOAD_BYTES,
|
||||
perMessageDeflate: false,
|
||||
});
|
||||
await waitForOpen(socket, signal);
|
||||
return new CdpConnection(socket);
|
||||
await waitForOpen(socket, operation, signal);
|
||||
return new CdpConnection(socket, operation);
|
||||
}
|
||||
|
||||
/** Subscribe to validated CDP event envelopes. */
|
||||
|
|
@ -69,13 +73,13 @@ export class CdpConnection {
|
|||
signal?: AbortSignal,
|
||||
): Promise<unknown> {
|
||||
if (this.#protocolFailed)
|
||||
throw new BrowserObservationError("inspect_web_page", "protocol_error");
|
||||
throw new BrowserObservationError(this.operation, "protocol_error");
|
||||
if (this.#closed || this.socket.readyState !== WebSocket.OPEN)
|
||||
throw new BrowserObservationError("inspect_web_page", "disconnected");
|
||||
throw new BrowserObservationError(this.operation, "disconnected");
|
||||
if (this.#pending.size >= MAX_PENDING_COMMANDS)
|
||||
throw new BrowserObservationError("inspect_web_page", "payload_limit");
|
||||
throw new BrowserObservationError(this.operation, "payload_limit");
|
||||
if (signal?.aborted === true)
|
||||
throw new AnalysisCancelledError("inspect_web_page");
|
||||
throw new AnalysisCancelledError(this.operation);
|
||||
const id = this.#nextId;
|
||||
this.#nextId += 1;
|
||||
return await new Promise((resolve, reject) => {
|
||||
|
|
@ -85,15 +89,13 @@ export class CdpConnection {
|
|||
clearTimeout(pending.timer);
|
||||
pending.removeAbort();
|
||||
this.#pending.delete(id);
|
||||
reject(new AnalysisCancelledError("inspect_web_page"));
|
||||
reject(new AnalysisCancelledError(this.operation));
|
||||
};
|
||||
signal?.addEventListener("abort", onAbort, { once: true });
|
||||
const timer = setTimeout(() => {
|
||||
signal?.removeEventListener("abort", onAbort);
|
||||
this.#pending.delete(id);
|
||||
reject(
|
||||
new AnalysisTimeoutError("inspect_web_page", COMMAND_TIMEOUT_MS),
|
||||
);
|
||||
reject(new AnalysisTimeoutError(this.operation, COMMAND_TIMEOUT_MS));
|
||||
}, COMMAND_TIMEOUT_MS);
|
||||
this.#pending.set(id, {
|
||||
resolve,
|
||||
|
|
@ -114,7 +116,7 @@ export class CdpConnection {
|
|||
if (pending === undefined) return;
|
||||
this.#complete(id, pending);
|
||||
reject(
|
||||
new BrowserObservationError("inspect_web_page", "disconnected", {
|
||||
new BrowserObservationError(this.operation, "disconnected", {
|
||||
cause: error,
|
||||
}),
|
||||
);
|
||||
|
|
@ -191,7 +193,7 @@ export class CdpConnection {
|
|||
this.#complete(id, pending);
|
||||
if ("error" in message) {
|
||||
pending.reject(
|
||||
new BrowserObservationError("inspect_web_page", "protocol_error"),
|
||||
new BrowserObservationError(this.operation, "protocol_error"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
|
@ -209,7 +211,7 @@ export class CdpConnection {
|
|||
else this.#protocolFailed = true;
|
||||
for (const [id, pending] of this.#pending) {
|
||||
this.#complete(id, pending);
|
||||
pending.reject(new BrowserObservationError("inspect_web_page", reason));
|
||||
pending.reject(new BrowserObservationError(this.operation, reason));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -226,13 +228,14 @@ const isRecord = (value: unknown): value is Record<string, unknown> =>
|
|||
|
||||
const waitForOpen = async (
|
||||
socket: WebSocket,
|
||||
operation: BrowserObservationOperation,
|
||||
signal?: AbortSignal,
|
||||
): Promise<void> =>
|
||||
await new Promise((resolve, reject) => {
|
||||
const timer = setTimeout(() => {
|
||||
cleanup();
|
||||
terminateSocket(socket);
|
||||
reject(new AnalysisTimeoutError("inspect_web_page", CONNECT_TIMEOUT_MS));
|
||||
reject(new AnalysisTimeoutError(operation, CONNECT_TIMEOUT_MS));
|
||||
}, CONNECT_TIMEOUT_MS);
|
||||
const onOpen = (): void => {
|
||||
cleanup();
|
||||
|
|
@ -242,17 +245,15 @@ const waitForOpen = async (
|
|||
cleanup();
|
||||
reject(
|
||||
signal?.aborted === true
|
||||
? new AnalysisCancelledError("inspect_web_page")
|
||||
: new BrowserObservationError(
|
||||
"inspect_web_page",
|
||||
"endpoint_unreachable",
|
||||
{ cause },
|
||||
),
|
||||
? new AnalysisCancelledError(operation)
|
||||
: new BrowserObservationError(operation, "endpoint_unreachable", {
|
||||
cause,
|
||||
}),
|
||||
);
|
||||
};
|
||||
const onAbort = (): void => {
|
||||
terminateSocket(socket);
|
||||
onFailure(new AnalysisCancelledError("inspect_web_page"));
|
||||
onFailure(new AnalysisCancelledError(operation));
|
||||
};
|
||||
const cleanup = (): void => {
|
||||
clearTimeout(timer);
|
||||
|
|
|
|||
|
|
@ -95,7 +95,11 @@ export const inspectCdpElectronPage = async (
|
|||
context.sessionId,
|
||||
context.signal,
|
||||
);
|
||||
await delayWithCancellation(context.input.observation_ms, context.signal);
|
||||
await delayWithCancellation(
|
||||
context.input.observation_ms,
|
||||
"inspect_electron_page",
|
||||
context.signal,
|
||||
);
|
||||
await report(context.progress, 2, "Capturing Electron structure");
|
||||
const resources = await captureResources(context, roots, completeness);
|
||||
const dom = await captureDom(context, roots, completeness);
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ export class CdpElectronProvider implements ElectronObservationPort {
|
|||
const roots = await canonicalElectronRoots(input.allowed_file_roots);
|
||||
const discovery = await discoverCdpEndpoint(
|
||||
input.cdp_endpoint,
|
||||
"list_browser_targets",
|
||||
"list_electron_targets",
|
||||
options.signal,
|
||||
);
|
||||
const allowed = [];
|
||||
|
|
|
|||
|
|
@ -3,7 +3,10 @@ import type {
|
|||
ObserveWebSessionInput,
|
||||
WebObservationSession,
|
||||
} from "../domain/browserSession.js";
|
||||
import { BrowserObservationError } from "../domain/errors.js";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
BrowserObservationError,
|
||||
} from "../domain/errors.js";
|
||||
import type { CdpEndpointDiscovery, CdpEndpointTarget } from "./CdpEndpoint.js";
|
||||
import { CdpConnection, type CdpEvent } from "./CdpConnection.js";
|
||||
import { CdpCaptureCompleteness } from "./CdpCaptureCompleteness.js";
|
||||
|
|
@ -321,7 +324,7 @@ const waitForWindow = async (
|
|||
settled = true;
|
||||
clearTimeout(timer);
|
||||
removeEnd();
|
||||
reject(signal?.reason ?? new Error("browser observation cancelled"));
|
||||
reject(new AnalysisCancelledError("observe_web_session"));
|
||||
};
|
||||
const timer = setTimeout(() => finish("window_elapsed"), durationMs);
|
||||
removeEnd = capture.onEnd(finish);
|
||||
|
|
@ -347,7 +350,7 @@ const authorizedFrame = async (
|
|||
"inspect_web_page",
|
||||
"target_not_allowed",
|
||||
);
|
||||
await delayWithCancellation(25, context.signal);
|
||||
await delayWithCancellation(25, "observe_web_session", context.signal);
|
||||
}
|
||||
throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
|
||||
};
|
||||
|
|
|
|||
|
|
@ -3,7 +3,10 @@ import type {
|
|||
WebPageInspection,
|
||||
} from "../domain/browserObservation.js";
|
||||
import type { ProgressReporter } from "../application/ProgressReporter.js";
|
||||
import { BrowserObservationError } from "../domain/errors.js";
|
||||
import {
|
||||
BrowserObservationError,
|
||||
type BrowserObservationOperation,
|
||||
} from "../domain/errors.js";
|
||||
import {
|
||||
reconcileCapturedWebScript,
|
||||
stableWebResources,
|
||||
|
|
@ -38,6 +41,10 @@ import type { WebSourceMapRequest } from "./WebSourceMapFetcher.js";
|
|||
interface CaptureContext {
|
||||
readonly connection: CdpConnection;
|
||||
readonly sessionId: string | undefined;
|
||||
readonly operation: Extract<
|
||||
BrowserObservationOperation,
|
||||
"inspect_web_page" | "analyze_web_bundle"
|
||||
>;
|
||||
readonly discovery: CdpEndpointDiscovery;
|
||||
readonly target: CdpEndpointTarget;
|
||||
readonly input: InspectWebPageInput;
|
||||
|
|
@ -225,7 +232,11 @@ const authorizeObservationWindow = async (
|
|||
events.beginAuthorizedFrame(mainFrame.frame_id);
|
||||
await enableObservationDomains(connection, sessionId, signal);
|
||||
await report(context.progress, 2, "Observing page events");
|
||||
await delayWithCancellation(context.input.observation_ms, signal);
|
||||
await delayWithCancellation(
|
||||
context.input.observation_ms,
|
||||
context.operation,
|
||||
signal,
|
||||
);
|
||||
if (events.originViolation)
|
||||
throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
|
||||
};
|
||||
|
|
@ -323,7 +334,7 @@ const authorizedFrameTree = async (
|
|||
"inspect_web_page",
|
||||
"target_not_allowed",
|
||||
);
|
||||
await delayWithCancellation(25, context.signal);
|
||||
await delayWithCancellation(25, context.operation, context.signal);
|
||||
}
|
||||
throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
|
||||
};
|
||||
|
|
|
|||
|
|
@ -23,7 +23,11 @@ export const openCdpTargetSession = async (
|
|||
signal?: AbortSignal,
|
||||
): Promise<CdpTargetSession> => {
|
||||
const webSocket = cdpTargetWebSocket(discovery, target, operation);
|
||||
const connection = await CdpConnection.connect(webSocket.url, signal);
|
||||
const connection = await CdpConnection.connect(
|
||||
webSocket.url,
|
||||
operation,
|
||||
signal,
|
||||
);
|
||||
if (webSocket.scope === "page") return { connection, sessionId: undefined };
|
||||
try {
|
||||
const attached = await connection.send(
|
||||
|
|
|
|||
|
|
@ -96,6 +96,7 @@ export const discoverWebMcp = async (
|
|||
// CDP sends the enable response before the required toolsAdded replay.
|
||||
await delayWithCancellation(
|
||||
Math.max(context.input.observation_ms, 25),
|
||||
"discover_webmcp_tools",
|
||||
context.signal,
|
||||
);
|
||||
await assertStableAuthorizedFrame(context, initialUrl, origins);
|
||||
|
|
|
|||
|
|
@ -109,6 +109,82 @@ describe("CdpBrowserProvider", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("projects direct-session cancellation onto the requested operation", async () => {
|
||||
const browser = await startFakeCdpBrowser({
|
||||
pageScopedVersionWebSocket: true,
|
||||
omitTargetWebSocket: true,
|
||||
});
|
||||
browsers.push(browser);
|
||||
const controller = new AbortController();
|
||||
const result = await new CdpBrowserProvider().observeSession(
|
||||
observeWebSessionInputSchema.parse({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: "allowed-page",
|
||||
approved: true,
|
||||
observation_ms: 1_000,
|
||||
}),
|
||||
{
|
||||
signal: controller.signal,
|
||||
progress: {
|
||||
report(update) {
|
||||
if (update.completed === 1) controller.abort();
|
||||
return Promise.resolve();
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "observe_web_session",
|
||||
},
|
||||
});
|
||||
const methods = browser.commands.map(({ method }) => method);
|
||||
expect(methods).not.toContain("Target.attachToTarget");
|
||||
expect(methods).not.toContain("Target.detachFromTarget");
|
||||
expect(methods).not.toContain("Target.closeTarget");
|
||||
expect(methods).not.toContain("Browser.close");
|
||||
});
|
||||
|
||||
it("projects capture-window cancellation onto bundle analysis", async () => {
|
||||
const browser = await startFakeCdpBrowser({
|
||||
pageScopedVersionWebSocket: true,
|
||||
omitTargetWebSocket: true,
|
||||
});
|
||||
browsers.push(browser);
|
||||
const controller = new AbortController();
|
||||
const result = await new CdpBrowserProvider().analyzeBundle(
|
||||
analyzeWebBundleInputSchema.parse({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: "allowed-page",
|
||||
approved: true,
|
||||
source_capture_approved: true,
|
||||
observation_ms: 1_000,
|
||||
}),
|
||||
{
|
||||
signal: controller.signal,
|
||||
progress: {
|
||||
report(update) {
|
||||
if (update.completed === 2) controller.abort();
|
||||
return Promise.resolve();
|
||||
},
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "analyze_web_bundle",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects empty browser attachment session identifiers", async () => {
|
||||
const browser = await startFakeCdpBrowser({ invalidAttachedSession: true });
|
||||
browsers.push(browser);
|
||||
|
|
@ -1198,7 +1274,10 @@ describe("CdpBrowserProvider", () => {
|
|||
const result = await pending;
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: { _tag: "AnalysisCancelledError" },
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "inspect_web_page",
|
||||
},
|
||||
});
|
||||
const methods = browser.commands.map(({ method }) => method);
|
||||
expect(methods).toContain("Target.detachFromTarget");
|
||||
|
|
|
|||
|
|
@ -19,7 +19,10 @@ describe("CDP connection", () => {
|
|||
it("correlates concurrent command responses over a real WebSocket", async () => {
|
||||
const browser = await startFakeCdpBrowser();
|
||||
browsers.push(browser);
|
||||
const connection = await CdpConnection.connect(browser.browserWebSocketUrl);
|
||||
const connection = await CdpConnection.connect(
|
||||
browser.browserWebSocketUrl,
|
||||
"inspect_web_page",
|
||||
);
|
||||
try {
|
||||
const [attached, frames] = await Promise.all([
|
||||
connection.send("Target.attachToTarget"),
|
||||
|
|
@ -37,13 +40,16 @@ describe("CDP connection", () => {
|
|||
it("returns a typed timeout for an unresponsive CDP command", async () => {
|
||||
const browser = await startFakeCdpBrowser({ hangOnMethod: "Page.enable" });
|
||||
browsers.push(browser);
|
||||
const connection = await CdpConnection.connect(browser.browserWebSocketUrl);
|
||||
const connection = await CdpConnection.connect(
|
||||
browser.browserWebSocketUrl,
|
||||
"observe_web_session",
|
||||
);
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const pending = connection.send("Page.enable");
|
||||
const assertion = expect(pending).rejects.toMatchObject({
|
||||
_tag: "AnalysisTimeoutError",
|
||||
operation: "inspect_web_page",
|
||||
operation: "observe_web_session",
|
||||
timeoutMs: 5_000,
|
||||
});
|
||||
await vi.advanceTimersByTimeAsync(5_000);
|
||||
|
|
|
|||
|
|
@ -142,6 +142,28 @@ describe("CdpElectronProvider", () => {
|
|||
).toBe(true);
|
||||
});
|
||||
|
||||
it("maps cancelled Electron discovery to the Electron operation", async () => {
|
||||
const root = await electronFixture();
|
||||
const controller = new AbortController();
|
||||
controller.abort();
|
||||
const result = await new CdpElectronProvider().listTargets(
|
||||
listElectronTargetsInputSchema.parse({
|
||||
cdp_endpoint: "http://127.0.0.1:9222",
|
||||
allowed_file_roots: [root],
|
||||
approved: true,
|
||||
}),
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
|
||||
expect(result).toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "list_electron_targets",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("captures Electron script source only after separate approval", async () => {
|
||||
const root = await electronFixture();
|
||||
const browser = await startFakeCdpBrowser({
|
||||
|
|
|
|||
Loading…
Reference in New Issue