fix(browser): keep target detail when attributing CDP failures (#6223)
Authorize CDP targets with the caller's operation instead of a fixed inspect_web_page label, and reuse the authorization owner's derived origin scope. When a browser observation failure is attributed to a different operation, keep its producer detail, remediation, cleanup, diagnostics, partial observation and original cause rather than rebuilding it from the reason alone. Both the CDP browser and CDP Electron providers use the same domain-owned relabel. Fixes #6122 Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
26356dc34a
commit
ec0e663cca
5 files changed
+202
-25
No files matched your search
@@ -386,8 +386,16 @@ const withTargetSession = async <
|
||||
let outcome: Result<Output, unknown>;
|
||||
let cleanupFailure: AnalysisError | undefined;
|
||||
try {
|
||||
const target = authorizeTarget(endpoint.discovery, input);
|
||||
const scopedInput = scopeToTargetOrigin(input, target);
|
||||
const { target, allowedOrigins } = authorizeCdpTarget(
|
||||
endpoint.discovery,
|
||||
input,
|
||||
operation,
|
||||
);
|
||||
// The authorization owner already derived the target origin scope.
|
||||
const scopedInput =
|
||||
input.allowed_origins.length > 0
|
||||
? input
|
||||
: { ...input, allowed_origins: [...allowedOrigins] };
|
||||
targetSession =
|
||||
endpoint.connection === undefined
|
||||
? await openCdpTargetSession(
|
||||
@@ -548,31 +556,13 @@ const selectTargets = (
|
||||
};
|
||||
};
|
||||
|
||||
const authorizeTarget = (
|
||||
discovery: CdpEndpointDiscovery,
|
||||
input: Pick<InspectWebPageInput, "target_id" | "allowed_origins">,
|
||||
): CdpEndpointTarget => {
|
||||
return authorizeCdpTarget(discovery, input, "inspect_web_page").target;
|
||||
};
|
||||
|
||||
const scopeToTargetOrigin = <T extends { allowed_origins: readonly string[] }>(
|
||||
input: T,
|
||||
target: CdpEndpointTarget,
|
||||
): T => {
|
||||
if (input.allowed_origins.length > 0) return input;
|
||||
const origin = sanitizeBrowserUrl(target.url).origin;
|
||||
if (origin === null)
|
||||
throw new BrowserObservationError("inspect_web_page", "target_not_allowed");
|
||||
return { ...input, allowed_origins: [origin] };
|
||||
};
|
||||
|
||||
const providerError = (
|
||||
cause: unknown,
|
||||
operation: BrowserObservationOperation,
|
||||
signal?: AbortSignal,
|
||||
): AnalysisError =>
|
||||
cause instanceof BrowserObservationError && cause.operation !== operation
|
||||
? new BrowserObservationError(operation, cause.reason, { cause })
|
||||
cause instanceof BrowserObservationError
|
||||
? cause.forOperation(operation)
|
||||
: cause instanceof AnalysisError
|
||||
? cause
|
||||
: signal?.aborted === true
|
||||
|
||||
@@ -471,9 +471,8 @@ const providerError = (
|
||||
operation,
|
||||
);
|
||||
if (fileFailure !== undefined) return fileFailure;
|
||||
return cause instanceof BrowserObservationError &&
|
||||
cause.operation !== operation
|
||||
? new BrowserObservationError(operation, cause.reason, { cause })
|
||||
return cause instanceof BrowserObservationError
|
||||
? cause.forOperation(operation)
|
||||
: cause instanceof AnalysisError
|
||||
? cause
|
||||
: new ProviderAdapterError(CDP_ELECTRON_PROVIDER_IDENTITY.id, operation, {
|
||||
|
||||
@@ -525,6 +525,83 @@ describe("analysis error projection: browser provider failures", () => {
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps producer facts when a browser failure is attributed to another operation", () => {
|
||||
const partial = { kind: "browser-test-partial" } as never;
|
||||
const original = new BrowserObservationError(
|
||||
"inspect_web_page",
|
||||
"cleanup_failed",
|
||||
{
|
||||
detail: "Target page-1 closed while REA released its session.",
|
||||
remediationAction: "Release session page-1, then retry.",
|
||||
cleanup: { reason: "socket close failed", resources: ["page-1"] },
|
||||
diagnostics: { endpoint: "loopback" },
|
||||
partialObservation: partial,
|
||||
cause: new AggregateError([
|
||||
new BrowserObservationError("inspect_web_page", "target_changed"),
|
||||
new Error("socket close failed"),
|
||||
]),
|
||||
},
|
||||
);
|
||||
const relabelled = original.forOperation("capture_web_screenshot");
|
||||
|
||||
expect(relabelled).toMatchObject({
|
||||
operation: "capture_web_screenshot",
|
||||
reason: "cleanup_failed",
|
||||
cause: original.cause,
|
||||
diagnostics: { endpoint: "loopback" },
|
||||
partialObservation: partial,
|
||||
});
|
||||
const { details, ...projected } = projectAnalysisError(relabelled);
|
||||
const { details: originalDetails, ...originalProjected } =
|
||||
projectAnalysisError(original);
|
||||
expect(projected).toEqual(originalProjected);
|
||||
expect(projected).toMatchObject({
|
||||
code: "cleanup_incomplete",
|
||||
message: "Target page-1 closed while REA released its session.",
|
||||
remediation: { action: "Release session page-1, then retry." },
|
||||
});
|
||||
expect(details).toEqual({
|
||||
...originalDetails,
|
||||
operation: "capture_web_screenshot",
|
||||
});
|
||||
expect(details).toMatchObject({
|
||||
resources: ["page-1"],
|
||||
primary_reason: "target_changed",
|
||||
});
|
||||
});
|
||||
|
||||
it("derives default browser guidance for the attributed operation", () => {
|
||||
const original = new BrowserObservationError(
|
||||
"inspect_web_page",
|
||||
"target_not_found",
|
||||
);
|
||||
expect(original.forOperation("inspect_web_page")).toBe(original);
|
||||
|
||||
const relabelled = original.forOperation("discover_webmcp_tools");
|
||||
const projected = projectAnalysisError(relabelled);
|
||||
expect(projected).toMatchObject({
|
||||
code: "target_unavailable",
|
||||
message: expect.stringMatching(/^discover_webmcp_tools could not find/),
|
||||
details: {
|
||||
operation: "discover_webmcp_tools",
|
||||
reason: "target_not_found",
|
||||
},
|
||||
});
|
||||
expect(projected.message).not.toContain("inspect_web_page");
|
||||
expect(
|
||||
projectAnalysisError(
|
||||
new BrowserObservationError("inspect_web_page", "timeout").forOperation(
|
||||
"observe_web_session",
|
||||
),
|
||||
),
|
||||
).toMatchObject({
|
||||
code: projectAnalysisError(
|
||||
new BrowserObservationError("observe_web_session", "timeout"),
|
||||
).code,
|
||||
details: { operation: "observe_web_session", reason: "timeout" },
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("analysis error projection: Hopper provider failures", () => {
|
||||
|
||||
@@ -14,6 +14,8 @@ export class BrowserObservationError extends AnalysisError {
|
||||
override cleanupIncomplete: boolean;
|
||||
override readonly userCategory: "cancelled" | undefined;
|
||||
override readonly userMessage: string | undefined;
|
||||
/** Producer-supplied explanation; undefined selects the operation default. */
|
||||
readonly detail: string | undefined;
|
||||
readonly remediationAction: string | undefined;
|
||||
|
||||
constructor(
|
||||
@@ -25,6 +27,7 @@ export class BrowserObservationError extends AnalysisError {
|
||||
},
|
||||
) {
|
||||
super(`Browser observation ${operation} failed: ${reason}`, options);
|
||||
this.detail = options?.detail;
|
||||
this.userMessage =
|
||||
options?.detail ?? observationFailureMessage(operation, reason);
|
||||
this.remediationAction = options?.remediationAction;
|
||||
@@ -42,6 +45,28 @@ export class BrowserObservationError extends AnalysisError {
|
||||
? "cancelled"
|
||||
: undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Attribute this failure to the caller's operation without discarding
|
||||
* producer facts; the original cause keeps the primary failure chain intact.
|
||||
*/
|
||||
forOperation(
|
||||
operation: BrowserObservationOperation,
|
||||
): BrowserObservationError {
|
||||
if (operation === this.operation) return this;
|
||||
return new BrowserObservationError(operation, this.reason, {
|
||||
cause: this.cause,
|
||||
...(this.detail === undefined ? {} : { detail: this.detail }),
|
||||
...(this.remediationAction === undefined
|
||||
? {}
|
||||
: { remediationAction: this.remediationAction }),
|
||||
cleanupPrimary: this.cleanupPrimary,
|
||||
diagnostics: this.diagnostics,
|
||||
capturedOutput: this.capturedOutput,
|
||||
cleanup: this.cleanup,
|
||||
partialObservation: this.partialObservation,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
const observationFailureMessage = (
|
||||
|
||||
@@ -1,10 +1,15 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { CdpBrowserProvider } from "../../../src/browser/CdpBrowserProvider.js";
|
||||
import { projectAnalysisError } from "../../../src/domain/analysisErrorProjection.js";
|
||||
import {
|
||||
inspectWebPageInputSchema,
|
||||
listBrowserTargetsInputSchema,
|
||||
} from "../../../src/domain/browserObservation.js";
|
||||
import { observeWebSessionInputSchema } from "../../../src/domain/browserSession.js";
|
||||
import { analyzeWebBundleInputSchema } from "../../../src/domain/webBundleAnalysis.js";
|
||||
import { discoverWebMcpToolsInputSchema } from "../../../src/domain/webMcpDiscovery.js";
|
||||
import { captureWebScreenshotInputSchema } from "../../../src/domain/webScreenshot.js";
|
||||
import {
|
||||
startFakeCdpBrowser,
|
||||
type FakeOptions,
|
||||
@@ -361,3 +366,84 @@ describe("CdpBrowserProvider: discovery authorization 2", () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("CdpBrowserProvider: target selection detail", () => {
|
||||
it("keeps target selection detail for every target operation", async () => {
|
||||
const browser = await startFakeCdpBrowser();
|
||||
trackBrowser(browser);
|
||||
const provider = new CdpBrowserProvider();
|
||||
const operations = [
|
||||
[
|
||||
"inspect_web_page",
|
||||
(input) =>
|
||||
provider.inspectPage(
|
||||
inspectWebPageInputSchema.parse({ ...input, observation_ms: 0 }),
|
||||
),
|
||||
],
|
||||
[
|
||||
"analyze_web_bundle",
|
||||
(input) =>
|
||||
provider.analyzeBundle(
|
||||
analyzeWebBundleInputSchema.parse({ ...input, observation_ms: 0 }),
|
||||
),
|
||||
],
|
||||
[
|
||||
"observe_web_session",
|
||||
(input) =>
|
||||
provider.observeSession(observeWebSessionInputSchema.parse(input)),
|
||||
],
|
||||
[
|
||||
"discover_webmcp_tools",
|
||||
(input) =>
|
||||
provider.discoverWebMcpTools(
|
||||
discoverWebMcpToolsInputSchema.parse(input),
|
||||
),
|
||||
],
|
||||
[
|
||||
"capture_web_screenshot",
|
||||
(input) =>
|
||||
provider.captureScreenshot(
|
||||
captureWebScreenshotInputSchema.parse(input),
|
||||
),
|
||||
],
|
||||
] as const satisfies readonly (readonly [
|
||||
string,
|
||||
(input: Record<string, unknown>) => Promise<{ readonly ok: boolean }>,
|
||||
])[];
|
||||
const selections = [
|
||||
[
|
||||
"missing",
|
||||
"target_not_found",
|
||||
"Target missing was not found at the selected endpoint.",
|
||||
],
|
||||
[
|
||||
"disallowed-page",
|
||||
"target_not_allowed",
|
||||
"Target disallowed-page must be an HTTP(S) page in the selected origin scope.",
|
||||
],
|
||||
] as const;
|
||||
const inspected = new Map<string, unknown>();
|
||||
for (const [operation, run] of operations)
|
||||
for (const [targetId, reason, detail] of selections) {
|
||||
const result = await run({
|
||||
cdp_endpoint: browser.endpoint,
|
||||
allowed_origins: [browser.allowedOrigin],
|
||||
target_id: targetId,
|
||||
});
|
||||
if (result.ok) throw new Error(`${operation} accepted ${targetId}`);
|
||||
expect(result.error).toMatchObject({ operation, reason });
|
||||
const { details, ...projected } = projectAnalysisError(result.error);
|
||||
expect(details).toEqual({ operation, reason });
|
||||
expect(projected).toMatchObject({
|
||||
code: "target_unavailable",
|
||||
message: detail,
|
||||
});
|
||||
// Only the attributed operation differs from the inspect projection.
|
||||
if (operation === "inspect_web_page")
|
||||
inspected.set(targetId, projected);
|
||||
else expect(projected).toEqual(inspected.get(targetId));
|
||||
}
|
||||
expect(browser.commands).toHaveLength(0);
|
||||
expect(browser.openedConnectionCount()).toBe(0);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user