mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-27 12:56:01 -06:00
fix(browser): preserve HTTP status in node-proxied browser errors (#89086)
* fix(browser): preserve HTTP status in node-proxied browser errors
The node browser proxy (runBrowserProxyCommand) collapsed a >=400 browser-route
response into new Error(<body.error>), dropping the HTTP status. That error
crosses the node.invoke boundary as a plain string (Error properties are not
preserved over the RPC), so the gateway's stale-target retry classifier — which
keys off a leading <status>: token (msg.includes("404:") && msg.includes("tab
not found")) — never matches a node-proxied "tab not found". The drop-targetId
retry never fires and the stale-targetId error surfaces to the agent instead.
Prefix the status onto the message ("404: tab not found", "403: action
targetId must match request targetId") so the existing gateway classification
and retry work through the node proxy. Pure formatting change in the >=400
branch; validation/timeout error paths are untouched.
Tests: extensions/browser invoke-browser suite — 14/14 pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* fix(browser): harden node proxy status errors
* docs(changelog): credit browser proxy status fix
* chore: defer browser proxy release note
---------
Co-authored-by: rhclaw <260109027+rhclaw@users.noreply.github.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
@@ -1898,6 +1898,48 @@ describe("browser tool act stale target recovery", () => {
|
||||
expect((result?.details as { ok?: unknown } | undefined)?.ok).toBe(true);
|
||||
});
|
||||
|
||||
it("retries stale targetIds returned through the node browser proxy", async () => {
|
||||
mockSingleBrowserProxyNode();
|
||||
setResolvedBrowserProfiles({
|
||||
user: { driver: "existing-session", attachOnly: true, color: "#00AA00" },
|
||||
});
|
||||
gatewayMocks.callGatewayTool
|
||||
.mockRejectedValueOnce(new Error("INVALID_REQUEST: Error: 404: tab not found"))
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
payload: { result: { tabs: [{ targetId: "only-tab" }] } },
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
payload: { result: { ok: true, targetId: "only-tab" } },
|
||||
});
|
||||
|
||||
const tool = createBrowserTool();
|
||||
const result = await tool.execute?.("call-1", {
|
||||
action: "act",
|
||||
target: "node",
|
||||
profile: "user",
|
||||
request: {
|
||||
kind: "wait",
|
||||
targetId: "stale-tab",
|
||||
timeMs: 1,
|
||||
},
|
||||
});
|
||||
|
||||
expect(gatewayMocks.callGatewayTool).toHaveBeenCalledTimes(3);
|
||||
expect(nodeInvokeCall(0).request.params).toMatchObject({
|
||||
path: "/act",
|
||||
body: { kind: "wait", targetId: "stale-tab", timeMs: 1 },
|
||||
});
|
||||
expect(nodeInvokeCall(1).request.params?.path).toBe("/tabs");
|
||||
expect(nodeInvokeCall(2).request.params).toMatchObject({
|
||||
path: "/act",
|
||||
body: { kind: "wait", timeMs: 1 },
|
||||
});
|
||||
expect(nodeInvokeCall(2).request.params?.body).not.toHaveProperty("targetId");
|
||||
expect(result?.details).toMatchObject({ ok: true, targetId: "only-tab" });
|
||||
});
|
||||
|
||||
it("does not retry mutating user-browser act requests without targetId", async () => {
|
||||
browserActionsMocks.browserAct.mockRejectedValueOnce(new Error("404: tab not found"));
|
||||
browserClientMocks.browserTabs.mockResolvedValueOnce([{ targetId: "only-tab" }]);
|
||||
|
||||
@@ -289,11 +289,16 @@ describe("runBrowserProxyCommand", () => {
|
||||
await result;
|
||||
});
|
||||
|
||||
it("keeps non-timeout browser errors intact", async () => {
|
||||
dispatcherMocks.dispatch.mockResolvedValue({
|
||||
status: 500,
|
||||
body: { error: "tab not found" },
|
||||
});
|
||||
it.each([
|
||||
{ status: 500, body: { error: "tab not found" }, expected: "500: tab not found" },
|
||||
{
|
||||
status: 404,
|
||||
body: { error: 'tab not found: browser tab "abc"' },
|
||||
expected: "404: tab not found",
|
||||
},
|
||||
{ status: 503, body: { error: "" }, expected: "HTTP 503" },
|
||||
])("preserves browser response status in errors: $expected", async (response) => {
|
||||
dispatcherMocks.dispatch.mockResolvedValue(response);
|
||||
|
||||
await expect(
|
||||
runBrowserProxyCommand(
|
||||
@@ -304,7 +309,7 @@ describe("runBrowserProxyCommand", () => {
|
||||
timeoutMs: 50,
|
||||
}),
|
||||
),
|
||||
).rejects.toThrow("tab not found");
|
||||
).rejects.toThrow(response.expected);
|
||||
});
|
||||
|
||||
it("rejects unauthorized query.profile when allowProfiles is configured", async () => {
|
||||
|
||||
@@ -315,11 +315,13 @@ export async function runBrowserProxyCommand(paramsJSON?: string | null): Promis
|
||||
);
|
||||
}
|
||||
if (response.status >= 400) {
|
||||
const message =
|
||||
// node.invoke preserves only Error.message; keep the status there so gateway
|
||||
// retry classifiers see the same error shape as direct browser requests.
|
||||
const detail =
|
||||
response.body && typeof response.body === "object" && "error" in response.body
|
||||
? String((response.body as { error?: unknown }).error)
|
||||
: `HTTP ${response.status}`;
|
||||
throw new Error(message);
|
||||
? String((response.body as { error?: unknown }).error).trim()
|
||||
: "";
|
||||
throw new Error(detail ? `${response.status}: ${detail}` : `HTTP ${response.status}`);
|
||||
}
|
||||
|
||||
const result = response.body;
|
||||
|
||||
Reference in New Issue
Block a user