diff --git a/src/browser/CdpBrowserProvider.ts b/src/browser/CdpBrowserProvider.ts index f4d898a68..50eb4fac0 100644 --- a/src/browser/CdpBrowserProvider.ts +++ b/src/browser/CdpBrowserProvider.ts @@ -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> { let targetSession: CdpTargetSession | undefined; + let result: Result; 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> { let targetSession: CdpTargetSession | undefined; + let result: Result; 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> { let targetSession: CdpTargetSession | undefined; + let result: Result; 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 = ( + result: Result, + signal: AbortSignal | undefined, + operation: BrowserObservationOperation, +): Result => + signal?.aborted === true + ? err(new AnalysisCancelledError(operation)) + : result; + +const isAbortError = (cause: unknown): boolean => + cause instanceof Error && cause.name === "AbortError"; diff --git a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts index 7c0d07445..7943583c8 100644 --- a/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts +++ b/tests/boundary/browser/cdpBrowserProvider.lifecycle_failures.test.ts @@ -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); diff --git a/tests/fixtures/fakeCdpBrowser.ts b/tests/fixtures/fakeCdpBrowser.ts index 191b4569c..3e7936d14 100644 --- a/tests/fixtures/fakeCdpBrowser.ts +++ b/tests/fixtures/fakeCdpBrowser.ts @@ -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) diff --git a/tests/fixtures/fakeCdpBrowserTypes.ts b/tests/fixtures/fakeCdpBrowserTypes.ts index 07a882a1e..9730eb36f 100644 --- a/tests/fixtures/fakeCdpBrowserTypes.ts +++ b/tests/fixtures/fakeCdpBrowserTypes.ts @@ -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"