From ad9e22d05fc4207eef7e2466a895bea6a8b005ee Mon Sep 17 00:00:00 2001 From: morluto <76467478+morluto@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:58:16 +0800 Subject: [PATCH] fix(browser): honor cancellation during CDP cleanup Complete owned teardown before deciding whether a successful observation can be published. Classify an exactly propagated source-map abort reason as cancellation while preserving unrelated failures. Validation: all four cleanup/cancellation boundary tests pass, retaining detach, socket and target-liveness assertions. Based on #2893. --- src/browser/CdpBrowserProvider.ts | 16 +++++++++++++++- .../browser/cdpCleanupCancellation.test.ts | 8 +++++++- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/browser/CdpBrowserProvider.ts b/src/browser/CdpBrowserProvider.ts index 14757a9a68..65cb6eb044 100644 --- a/src/browser/CdpBrowserProvider.ts +++ b/src/browser/CdpBrowserProvider.ts @@ -167,6 +167,11 @@ export class CdpBrowserProvider implements BrowserObservationPort { ), ); } catch (cause: unknown) { + // fetchWebSourceMaps rethrows the exact abort reason. Preserve cancellation + // even when a caller supplied an AnalysisError as that reason. + if (options.signal?.aborted === true && cause === options.signal.reason) { + return err(new AnalysisCancelledError("analyze_web_bundle", { cause })); + } return err(providerError(cause, "analyze_web_bundle", options.signal)); } } @@ -305,6 +310,7 @@ const withTargetSession = async < options.signal, ); let targetSession: CdpTargetSession | undefined; + let output: Output; try { const target = authorizeTarget(endpoint.discovery, input); const scopedInput = scopeToTargetOrigin(input, target); @@ -322,7 +328,7 @@ const withTargetSession = async < operation, options.signal, ); - return await work({ + output = await work({ connection: targetSession.connection, sessionId: targetSession.sessionId, discovery: endpoint.discovery, @@ -341,6 +347,14 @@ const withTargetSession = async < // An unadopted browser socket is closed here; closing is idempotent. await endpoint.connection?.close(); } + + // Cancellation during teardown must not publish the completed observation. + // Errors from work or cleanup still propagate through the try/finally above. + if (options.signal?.aborted === true) + throw new AnalysisCancelledError(operation, { + cause: options.signal.reason, + }); + return output; }; const selectTargets = ( diff --git a/tests/boundary/browser/cdpCleanupCancellation.test.ts b/tests/boundary/browser/cdpCleanupCancellation.test.ts index 0d4c17a0fa..8495313132 100644 --- a/tests/boundary/browser/cdpCleanupCancellation.test.ts +++ b/tests/boundary/browser/cdpCleanupCancellation.test.ts @@ -73,7 +73,13 @@ it("releases cleanup when cancellation arrives after capture completes", async ( }); controller.abort(); const result = await pending; - expect(result.ok).toBe(true); + expect(result).toMatchObject({ + ok: false, + error: { + _tag: "AnalysisCancelledError", + operation: "observe_web_session", + }, + }); expect(browser.commands.map(({ method }) => method)).toContain( "Target.detachFromTarget", );