fix(browser): report cancellation after cleanup and source-map abort
An interrupt during CDP cleanup could still publish the captured page, and a source-map download abort was wrapped as a provider failure. Both now return AnalysisCancelledError for the requested operation.
This commit is contained in:
@@ -43,7 +43,10 @@ import {
|
||||
type WebScreenshotDiff,
|
||||
} from "../domain/webScreenshot.js";
|
||||
import { AnalysisError } from "../domain/analysisErrorBase.js";
|
||||
import { AnalysisInputError } from "../domain/analysisErrorCore.js";
|
||||
import {
|
||||
AnalysisCancelledError,
|
||||
AnalysisInputError,
|
||||
} from "../domain/analysisErrorCore.js";
|
||||
import { BrowserObservationError } from "../domain/browserObservationError.js";
|
||||
import { ProviderAdapterError } from "../domain/providerAdapterError.js";
|
||||
import type { BrowserObservationOperation } from "../domain/browserObservationErrors.js";
|
||||
@@ -170,6 +173,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options: ExecutionOptions = {},
|
||||
): Promise<Result<WebObservationSession, AnalysisError>> {
|
||||
let targetSession: CdpTargetSession | undefined;
|
||||
let result: Result<WebObservationSession, AnalysisError>;
|
||||
try {
|
||||
const discovery = await discoverCdpEndpoint(
|
||||
input.cdp_endpoint,
|
||||
@@ -184,7 +188,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
"observe_web_session",
|
||||
options.signal,
|
||||
);
|
||||
return ok(
|
||||
result = ok(
|
||||
webObservationSessionSchema.parse(
|
||||
await observeCdpSession({
|
||||
connection: targetSession.connection,
|
||||
@@ -200,7 +204,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
),
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
return err(providerError(cause, "observe_web_session"));
|
||||
result = err(providerError(cause, "observe_web_session"));
|
||||
} finally {
|
||||
if (targetSession !== undefined)
|
||||
await closeCdpTargetSession(
|
||||
@@ -209,6 +213,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options.signal,
|
||||
);
|
||||
}
|
||||
return resultAfterCleanup(result, options.signal, "observe_web_session");
|
||||
}
|
||||
|
||||
async discoverWebMcpTools(
|
||||
@@ -216,6 +221,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options: ExecutionOptions = {},
|
||||
): Promise<Result<WebMcpDiscovery, AnalysisError>> {
|
||||
let targetSession: CdpTargetSession | undefined;
|
||||
let result: Result<WebMcpDiscovery, AnalysisError>;
|
||||
try {
|
||||
const discovery = await discoverCdpEndpoint(
|
||||
input.cdp_endpoint,
|
||||
@@ -230,7 +236,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
"discover_webmcp_tools",
|
||||
options.signal,
|
||||
);
|
||||
return ok(
|
||||
result = ok(
|
||||
webMcpDiscoverySchema.parse(
|
||||
await discoverWebMcp({
|
||||
connection: targetSession.connection,
|
||||
@@ -246,7 +252,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
),
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
return err(providerError(cause, "discover_webmcp_tools"));
|
||||
result = err(providerError(cause, "discover_webmcp_tools"));
|
||||
} finally {
|
||||
if (targetSession !== undefined)
|
||||
await closeCdpTargetSession(
|
||||
@@ -255,6 +261,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options.signal,
|
||||
);
|
||||
}
|
||||
return resultAfterCleanup(result, options.signal, "discover_webmcp_tools");
|
||||
}
|
||||
|
||||
async compareCaptures(
|
||||
@@ -274,6 +281,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options: ExecutionOptions = {},
|
||||
): Promise<Result<WebScreenshot, AnalysisError>> {
|
||||
let targetSession: CdpTargetSession | undefined;
|
||||
let result: Result<WebScreenshot, AnalysisError>;
|
||||
try {
|
||||
const discovery = await discoverCdpEndpoint(
|
||||
input.cdp_endpoint,
|
||||
@@ -288,7 +296,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
"capture_web_screenshot",
|
||||
options.signal,
|
||||
);
|
||||
return ok(
|
||||
result = ok(
|
||||
webScreenshotSchema.parse(
|
||||
await captureCdpScreenshot({
|
||||
connection: targetSession.connection,
|
||||
@@ -304,7 +312,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
),
|
||||
);
|
||||
} catch (cause: unknown) {
|
||||
return err(providerError(cause, "capture_web_screenshot"));
|
||||
result = err(providerError(cause, "capture_web_screenshot"));
|
||||
} finally {
|
||||
if (targetSession !== undefined)
|
||||
await closeCdpTargetSession(
|
||||
@@ -313,6 +321,7 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
options.signal,
|
||||
);
|
||||
}
|
||||
return resultAfterCleanup(result, options.signal, "capture_web_screenshot");
|
||||
}
|
||||
|
||||
async compareScreenshots(
|
||||
@@ -370,6 +379,10 @@ export class CdpBrowserProvider implements BrowserObservationPort {
|
||||
CLEANUP_DOMAINS,
|
||||
options.signal,
|
||||
);
|
||||
// Cleanup can notice an interrupt and still return the captured page.
|
||||
// Refuse that page before inspection or bundle analysis publishes it.
|
||||
if (options.signal?.aborted === true)
|
||||
throw new AnalysisCancelledError(operation);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -454,6 +467,25 @@ const providerError = (
|
||||
? new BrowserObservationError(operation, cause.reason, { cause })
|
||||
: cause instanceof AnalysisError
|
||||
? cause
|
||||
: new ProviderAdapterError(CDP_BROWSER_PROVIDER_IDENTITY.id, operation, {
|
||||
cause,
|
||||
});
|
||||
: isAbortError(cause)
|
||||
? new AnalysisCancelledError(operation, { cause })
|
||||
: new ProviderAdapterError(
|
||||
CDP_BROWSER_PROVIDER_IDENTITY.id,
|
||||
operation,
|
||||
{
|
||||
cause,
|
||||
},
|
||||
);
|
||||
|
||||
/** An interrupt during cleanup must replace a result that was already built. */
|
||||
const resultAfterCleanup = <Value>(
|
||||
result: Result<Value, AnalysisError>,
|
||||
signal: AbortSignal | undefined,
|
||||
operation: BrowserObservationOperation,
|
||||
): Result<Value, AnalysisError> =>
|
||||
signal?.aborted === true
|
||||
? err(new AnalysisCancelledError(operation))
|
||||
: result;
|
||||
|
||||
const isAbortError = (cause: unknown): boolean =>
|
||||
cause instanceof Error && cause.name === "AbortError";
|
||||
|
||||
@@ -13,7 +13,105 @@ import {
|
||||
} from "../../fixtures/fakeCdpBrowser.js";
|
||||
import { trackBrowser } from "./cdpBrowserProvider.support.js";
|
||||
|
||||
const waitFor = async (ready: () => boolean, description: string) => {
|
||||
const deadline = Date.now() + 5_000;
|
||||
while (!ready()) {
|
||||
if (Date.now() > deadline) throw new Error(description);
|
||||
await new Promise((resolve) => setTimeout(resolve, 10));
|
||||
}
|
||||
};
|
||||
|
||||
describe("CdpBrowserProvider: cancellation and session events", () => {
|
||||
it("reports cancellation when an interrupt arrives during page cleanup", async () => {
|
||||
const browser = await startFakeCdpBrowser({
|
||||
hangOnMethod: "Network.disable",
|
||||
});
|
||||
trackBrowser(browser);
|
||||
const controller = new AbortController();
|
||||
const pending = new CdpBrowserProvider().inspectPage(
|
||||
inspectWebPageInputSchema.parse({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: "allowed-page",
|
||||
observation_ms: 0,
|
||||
}),
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
await waitFor(
|
||||
() => browser.commands.some(({ method }) => method === "Network.disable"),
|
||||
"page cleanup did not start",
|
||||
);
|
||||
controller.abort();
|
||||
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "inspect_web_page",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("reports cancellation when an interrupt arrives during session cleanup", async () => {
|
||||
const browser = await startFakeCdpBrowser({
|
||||
hangOnMethod: "Network.disable",
|
||||
});
|
||||
trackBrowser(browser);
|
||||
const controller = new AbortController();
|
||||
const pending = new CdpBrowserProvider().observeSession(
|
||||
observeWebSessionInputSchema.parse({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: "allowed-page",
|
||||
observation_ms: 1,
|
||||
}),
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
await waitFor(
|
||||
() => browser.commands.some(({ method }) => method === "Network.disable"),
|
||||
"session cleanup did not start",
|
||||
);
|
||||
controller.abort();
|
||||
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "observe_web_session",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("reports cancellation when an interrupt arrives during source-map download", async () => {
|
||||
const browser = await startFakeCdpBrowser({ hangSourceMap: true });
|
||||
trackBrowser(browser);
|
||||
const controller = new AbortController();
|
||||
const pending = new CdpBrowserProvider().analyzeBundle(
|
||||
analyzeWebBundleInputSchema.parse({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: "allowed-page",
|
||||
observation_ms: 0,
|
||||
fetch_source_maps: true,
|
||||
}),
|
||||
{ signal: controller.signal },
|
||||
);
|
||||
await waitFor(
|
||||
() =>
|
||||
browser.httpRequests.some(({ url }) => url.startsWith("/app.js.map")),
|
||||
"source-map download did not start",
|
||||
);
|
||||
controller.abort();
|
||||
|
||||
await expect(pending).resolves.toMatchObject({
|
||||
ok: false,
|
||||
error: {
|
||||
_tag: "AnalysisCancelledError",
|
||||
operation: "analyze_web_bundle",
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("honors cancellation during the final progress callback before publishing inspection", async () => {
|
||||
const browser = await startFakeCdpBrowser();
|
||||
trackBrowser(browser);
|
||||
|
||||
Vendored
+5
@@ -226,6 +226,11 @@ const serveSourceMap = (
|
||||
response: ServerResponse,
|
||||
options: FakeOptions,
|
||||
): boolean => {
|
||||
if (
|
||||
options.hangSourceMap === true &&
|
||||
request.url?.startsWith("/app.js.map") === true
|
||||
)
|
||||
return true;
|
||||
if (options.sourceMapBody === undefined) return false;
|
||||
if (request.url?.startsWith("/app.js.map") === true) {
|
||||
if (options.sourceMapRedirect !== undefined)
|
||||
|
||||
+2
@@ -93,6 +93,8 @@ export interface FakeOptions {
|
||||
readonly webSocketWithoutFrames?: boolean;
|
||||
readonly invalidBinaryWebSocketEvent?: boolean;
|
||||
readonly sourceMapBody?: string;
|
||||
/** Accept a source-map request and leave the response open. */
|
||||
readonly hangSourceMap?: boolean;
|
||||
readonly sourceMapRedirect?: string;
|
||||
readonly sessionTimeline?:
|
||||
| "same_origin"
|
||||
|
||||
Reference in New Issue
Block a user