From 2533bb06fccb3a993f56b5758fca49526fa9d605 Mon Sep 17 00:00:00 2001 From: "Juan V." Date: Wed, 24 Jun 2026 15:32:02 +0200 Subject: [PATCH 1/7] fix(http): surface the server's error body on a failed tool call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a non-2xx response, axios throws an AxiosError whose `.message` is the generic "Request failed with status code 403" — the response body, where servers put the actual reason (e.g. { "error": "..." }), lives on `error.response.data` and is lost to every caller that only reads `.message`. It is also dropped entirely when the error crosses a serialization boundary (e.g. JSON.stringify inside an isolated-vm tool runner like @utcp/code-mode), because `Error.message` is non-enumerable and AxiosError doesn't serialize its `response`. The caller is left with just a status code and no explanation. callTool now normalizes a failed HTTP call into an Error that folds the status and server body into the message AND attaches enumerable `status` / `data` fields, so the reason survives both `.message` readers and structured serialization. Non-HTTP errors (network, timeout) pass through unchanged. Adds a /forbidden test route (403 + JSON body) and a callTool test asserting the thrown error carries the body in its message and round-trips through JSON.stringify with status + data intact. --- .../http/src/http_communication_protocol.ts | 38 ++++++++++++++++++- .../tests/http_communication_protocol.test.ts | 33 ++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index 7538947..0503a88 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -222,10 +222,46 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { return response.data; } catch (error: any) { this._logError(`Error calling HTTP tool '${toolName}':`, error); - throw error; + throw this._normalizeToolError(toolName, error); } } + /** + * Turn a raw axios error into one that actually carries the server's response. + * + * On a non-2xx, axios throws an `AxiosError` whose `.message` is the generic + * `"Request failed with status code 403"` — the response BODY (where servers + * put the real reason, e.g. `{ "error": "..." }`) sits on `error.response.data` + * and is otherwise lost to every caller that only reads `.message`. That body + * is also dropped entirely when the error is serialized across a boundary + * (e.g. JSON.stringify in an isolated-vm tool runner), because `Error.message` + * is non-enumerable and `AxiosError` doesn't serialize its `response`. + * + * This folds the status + body into the message AND attaches enumerable + * `status` / `data` fields, so the reason survives both `.message` readers and + * structured serialization. Non-HTTP errors (network, timeout) pass through. + */ + private _normalizeToolError(toolName: string, error: any): Error { + if (!axios.isAxiosError(error) || !error.response) { + return error instanceof Error ? error : new Error(String(error)); + } + const status = error.response.status; + const data = error.response.data; + const detail = + typeof data === 'string' + ? data + : data == null + ? error.message + : (data.error ?? data.message ?? data.detail ?? JSON.stringify(data)); + const normalized = new Error( + `HTTP ${status} calling tool '${toolName}': ${detail}`, + ) as Error & { status: number; data: unknown }; + // Enumerable so they survive JSON.stringify out of an isolated-vm runner. + normalized.status = status; + normalized.data = data; + return normalized; + } + /** * Executes a tool call through this transport streamingly. * For standard HTTP, this typically means fetching the full response and yielding it as a single chunk. diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index b7e166a..37a2733 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -62,6 +62,12 @@ beforeAll(async () => { res.status(500).json({ error: "Internal Server Error" }); }); + // Returns a non-2xx with a descriptive JSON body, like a real API does when it + // refuses a call. Used to prove callTool surfaces that body, not just the status. + app.post("/forbidden", (req, res) => { + res.status(403).json({ error: "You are not allowed to do that, and here is exactly why." }); + }); + await new Promise((resolve) => { server = app.listen(0, () => { serverPort = (server.address() as any).port; @@ -188,5 +194,32 @@ describe("HttpCommunicationProtocol", () => { const result = await protocol.callTool(mockClient, "test.tool", {}, callTemplate); expect(result.result).toBe("success"); }); + + test("should surface the server's error body, not just the status code", async () => { + const callTemplate: HttpCallTemplate = { + name: "forbidden_server", + call_template_type: "http", + url: `http://localhost:${serverPort}/forbidden`, + http_method: "POST", + }; + + let thrown: any; + try { + await protocol.callTool(mockClient, "test.forbidden", {}, callTemplate); + } catch (e) { + thrown = e; + } + + expect(thrown).toBeInstanceOf(Error); + // The reason the server sent (not just "status code 403") must be present. + expect(thrown.message).toContain("You are not allowed to do that, and here is exactly why."); + expect(thrown.message).toContain("403"); + // Enumerable status/data survive structured serialization out of a sandbox. + expect(thrown.status).toBe(403); + expect(thrown.data).toEqual({ error: "You are not allowed to do that, and here is exactly why." }); + const roundTripped = JSON.parse(JSON.stringify(thrown)); + expect(roundTripped.status).toBe(403); + expect(roundTripped.data).toEqual({ error: "You are not allowed to do that, and here is exactly why." }); + }); }); }); \ No newline at end of file From e0abe24bfcb1aceda6ca642d45c50f25bae26419 Mon Sep 17 00:00:00 2001 From: "Juan V." Date: Wed, 24 Jun 2026 15:36:50 +0200 Subject: [PATCH 2/7] fix(http): surface the server's error body in streamable_http + sse discovery The same body-swallowing pattern existed in the fetch-based streamable_http and sse protocols: on a non-2xx during manual discovery (registerManual) they threw `HTTP ${status}: ${statusText}` without ever reading the response body, so a server that refuses discovery with a descriptive 403/400 surfaced only as "HTTP 403: Forbidden" in the returned errors[]. Both now read the body before throwing and fold it into the message (falling back to statusText when the body is empty). Their callTool paths are stubs (no HTTP call yet), so discovery is the only real failure surface today. Adds a GET /forbidden-discovery route (403 + text body) and tests asserting both protocols' registerManual surfaces the body, not just the status code. --- .../http/src/sse_communication_protocol.ts | 7 ++- .../streamable_http_communication_protocol.ts | 7 ++- .../tests/http_communication_protocol.test.ts | 46 ++++++++++++++++++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/packages/http/src/sse_communication_protocol.ts b/packages/http/src/sse_communication_protocol.ts index 9474279..b4bdc41 100644 --- a/packages/http/src/sse_communication_protocol.ts +++ b/packages/http/src/sse_communication_protocol.ts @@ -215,7 +215,12 @@ export class SseCommunicationProtocol implements CommunicationProtocol { }); if (!response.ok) { - throw new Error(`HTTP ${response.status}: ${response.statusText}`); + // Read the body before throwing — servers put the real reason there + // (e.g. { "error": "..." }); discarding it leaves callers with only a + // status code. Fall back to statusText when the body is empty. + const body = await response.text().catch(() => ''); + const detail = body.trim() || response.statusText; + throw new Error(`HTTP ${response.status}: ${detail}`); } // Discovery returns the manual as a plain JSON document. diff --git a/packages/http/src/streamable_http_communication_protocol.ts b/packages/http/src/streamable_http_communication_protocol.ts index fc1bcbe..7038e0d 100644 --- a/packages/http/src/streamable_http_communication_protocol.ts +++ b/packages/http/src/streamable_http_communication_protocol.ts @@ -168,7 +168,12 @@ export class StreamableHttpCommunicationProtocol implements CommunicationProtoco }); if (!response.ok) { - throw new Error(`HTTP ${response.status}: ${response.statusText}`); + // Read the body before throwing: servers put the real reason there + // (e.g. { "error": "..." }); discarding it leaves callers with only a + // status code. Fall back to statusText when the body is empty. + const body = await response.text().catch(() => ''); + const detail = body.trim() || response.statusText; + throw new Error(`HTTP ${response.status}: ${detail}`); } const responseText = await response.text(); diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index 37a2733..0835adc 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -3,7 +3,12 @@ import { test, expect, describe, beforeAll, afterAll } from "bun:test"; import express, { Express } from 'express'; import { Server } from 'http'; // Import from package index to trigger auto-registration -import { HttpCommunicationProtocol, HttpCallTemplate } from "@utcp/http"; +import { + HttpCommunicationProtocol, + HttpCallTemplate, + StreamableHttpCommunicationProtocol, + SseCommunicationProtocol, +} from "@utcp/http"; import { ApiKeyAuth, BasicAuth, OAuth2Auth } from "@utcp/sdk"; import { IUtcpClient } from "@utcp/sdk"; @@ -68,6 +73,11 @@ beforeAll(async () => { res.status(403).json({ error: "You are not allowed to do that, and here is exactly why." }); }); + // GET variant for the streamable/sse discovery (registerManual) error-body test. + app.get("/forbidden-discovery", (req, res) => { + res.status(403).send("discovery refused: tenant is not provisioned for streaming"); + }); + await new Promise((resolve) => { server = app.listen(0, () => { serverPort = (server.address() as any).port; @@ -222,4 +232,38 @@ describe("HttpCommunicationProtocol", () => { expect(roundTripped.data).toEqual({ error: "You are not allowed to do that, and here is exactly why." }); }); }); +}); + +// The streamable/sse protocols' callTool paths are stubs; the only real fetch +// that can fail with a body is in registerManual (discovery). These prove that +// failure surfaces the server's body, not just "HTTP 403: Forbidden". +describe("discovery error body (streamable_http + sse)", () => { + test("StreamableHttpCommunicationProtocol.registerManual surfaces the server body", async () => { + const protocol = new StreamableHttpCommunicationProtocol(); + const result = await protocol.registerManual(mockClient, { + name: "forbidden_stream", + call_template_type: "streamable_http", + url: `http://localhost:${serverPort}/forbidden-discovery`, + http_method: "GET", + } as any); + + expect(result.success).toBe(false); + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0]).toContain("discovery refused: tenant is not provisioned for streaming"); + expect(result.errors[0]).toContain("403"); + }); + + test("SseCommunicationProtocol.registerManual surfaces the server body", async () => { + const protocol = new SseCommunicationProtocol(); + const result = await protocol.registerManual(mockClient, { + name: "forbidden_sse", + call_template_type: "sse", + url: `http://localhost:${serverPort}/forbidden-discovery`, + } as any); + + expect(result.success).toBe(false); + expect(result.errors.length).toBeGreaterThan(0); + expect(result.errors[0]).toContain("discovery refused: tenant is not provisioned for streaming"); + expect(result.errors[0]).toContain("403"); + }); }); \ No newline at end of file From 15e978b983670e8f2d21c2b53656aedbd77997e6 Mon Sep 17 00:00:00 2001 From: "Juan V." Date: Wed, 24 Jun 2026 15:41:59 +0200 Subject: [PATCH 3/7] fix(http): JSON-stringify object-valued error fields (cubic P1) `_normalizeToolError` preferred `data.error` / `data.message` / `data.detail` unconditionally, but some APIs nest an OBJECT there (e.g. { error: { code, reason } }). Using it directly interpolated as "[object Object]", hiding the real detail. Now only a STRING candidate is used; a non-string falls through to JSON.stringify(data) so the structure shows. The raw object is still preserved on the error's `data` field. Adds a /forbidden-object route (422 + nested object body) and a callTool test asserting the message contains the structured detail (not "[object Object]") and `data` holds the original object. Issue identified by cubic. --- .../http/src/http_communication_protocol.ts | 8 ++++- .../tests/http_communication_protocol.test.ts | 31 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index 0503a88..d0af090 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -247,12 +247,18 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { } const status = error.response.status; const data = error.response.data; + // Prefer a string `error` / `message` / `detail` field from the body; some + // APIs nest an OBJECT there (e.g. { error: { code, reason } }), so only use + // the candidate when it's actually a string — otherwise fall through to the + // full JSON so the real structure shows instead of "[object Object]". + const stringField = (...candidates: unknown[]): string | undefined => + candidates.find((c): c is string => typeof c === 'string'); const detail = typeof data === 'string' ? data : data == null ? error.message - : (data.error ?? data.message ?? data.detail ?? JSON.stringify(data)); + : (stringField(data.error, data.message, data.detail) ?? JSON.stringify(data)); const normalized = new Error( `HTTP ${status} calling tool '${toolName}': ${detail}`, ) as Error & { status: number; data: unknown }; diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index 0835adc..e1fbe46 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -78,6 +78,12 @@ beforeAll(async () => { res.status(403).send("discovery refused: tenant is not provisioned for streaming"); }); + // Some APIs nest an OBJECT under `error` — the message must show its JSON, + // not "[object Object]". + app.post("/forbidden-object", (req, res) => { + res.status(422).json({ error: { code: "INVALID_FIELD", reason: "value out of range" } }); + }); + await new Promise((resolve) => { server = app.listen(0, () => { serverPort = (server.address() as any).port; @@ -231,6 +237,31 @@ describe("HttpCommunicationProtocol", () => { expect(roundTripped.status).toBe(403); expect(roundTripped.data).toEqual({ error: "You are not allowed to do that, and here is exactly why." }); }); + + test("should JSON-stringify an object-valued error field, not '[object Object]'", async () => { + const callTemplate: HttpCallTemplate = { + name: "forbidden_object_server", + call_template_type: "http", + url: `http://localhost:${serverPort}/forbidden-object`, + http_method: "POST", + }; + + let thrown: any; + try { + await protocol.callTool(mockClient, "test.forbidden_object", {}, callTemplate); + } catch (e) { + thrown = e; + } + + expect(thrown).toBeInstanceOf(Error); + expect(thrown.message).not.toContain("[object Object]"); + // The structured detail survives in the message... + expect(thrown.message).toContain("INVALID_FIELD"); + expect(thrown.message).toContain("value out of range"); + // ...and the raw object is preserved on `data`. + expect(thrown.status).toBe(422); + expect(thrown.data).toEqual({ error: { code: "INVALID_FIELD", reason: "value out of range" } }); + }); }); }); From 35395257424c10949b34bb0f04067918721cdba0 Mon Sep 17 00:00:00 2001 From: Razvan Radulescu <43811028+h3xxit@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:03:54 +0200 Subject: [PATCH 4/7] http: fall back to status text on a blank error body; bound error detail length Addresses cubic review on #44: - _normalizeToolError used an empty body string as the detail, producing a message ending in a bare colon that was less informative than the raw axios message. A blank body now falls back to the status text, then axios's message. The detail is capped at 2000 characters (the raw body stays intact on `data`). - SSE and Streamable HTTP discovery truncate the error body to 200 characters, matching their streaming paths, so a huge error page does not land in errors[] and logs in full. Tests for the blank body, the truncated call message, and the truncated discovery error. Co-Authored-By: Claude Fable 5.1 --- .../http/src/http_communication_protocol.ts | 10 +++- .../http/src/sse_communication_protocol.ts | 4 +- .../streamable_http_communication_protocol.ts | 4 +- .../tests/http_communication_protocol.test.ts | 59 +++++++++++++++++++ 4 files changed, 73 insertions(+), 4 deletions(-) diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index d0af090..0049b5e 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -253,12 +253,18 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { // full JSON so the real structure shows instead of "[object Object]". const stringField = (...candidates: unknown[]): string | undefined => candidates.find((c): c is string => typeof c === 'string'); - const detail = + // A blank body carries no reason: fall back to the status text (then + // axios's own message) rather than emitting a message that ends in a + // bare colon. Bound the detail so a multi-megabyte error page (an HTML + // stack trace, say) does not end up in messages and logs in full. + const MAX_DETAIL_CHARS = 2000; + const rawDetail = typeof data === 'string' - ? data + ? data.trim() || error.response.statusText || error.message : data == null ? error.message : (stringField(data.error, data.message, data.detail) ?? JSON.stringify(data)); + const detail = rawDetail.length > MAX_DETAIL_CHARS ? `${rawDetail.slice(0, MAX_DETAIL_CHARS)}…` : rawDetail; const normalized = new Error( `HTTP ${status} calling tool '${toolName}': ${detail}`, ) as Error & { status: number; data: unknown }; diff --git a/packages/http/src/sse_communication_protocol.ts b/packages/http/src/sse_communication_protocol.ts index b4bdc41..e885cb4 100644 --- a/packages/http/src/sse_communication_protocol.ts +++ b/packages/http/src/sse_communication_protocol.ts @@ -219,7 +219,9 @@ export class SseCommunicationProtocol implements CommunicationProtocol { // (e.g. { "error": "..." }); discarding it leaves callers with only a // status code. Fall back to statusText when the body is empty. const body = await response.text().catch(() => ''); - const detail = body.trim() || response.statusText; + // Truncate like the streaming path does, so a huge error page does + // not land in errors[] and logs in full. + const detail = body.trim().slice(0, 200) || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); } diff --git a/packages/http/src/streamable_http_communication_protocol.ts b/packages/http/src/streamable_http_communication_protocol.ts index 7038e0d..a7d687f 100644 --- a/packages/http/src/streamable_http_communication_protocol.ts +++ b/packages/http/src/streamable_http_communication_protocol.ts @@ -172,7 +172,9 @@ export class StreamableHttpCommunicationProtocol implements CommunicationProtoco // (e.g. { "error": "..." }); discarding it leaves callers with only a // status code. Fall back to statusText when the body is empty. const body = await response.text().catch(() => ''); - const detail = body.trim() || response.statusText; + // Truncate like the streaming path does, so a huge error page does + // not land in errors[] and logs in full. + const detail = body.trim().slice(0, 200) || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); } diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index e1fbe46..f2a8ebb 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -84,6 +84,19 @@ beforeAll(async () => { res.status(422).json({ error: { code: "INVALID_FIELD", reason: "value out of range" } }); }); + // A refusal with no body at all: the message must still carry a reason. + app.post("/forbidden-empty", (req, res) => { + res.status(403).end(); + }); + + // A refusal with a huge body (think an HTML stack trace): messages stay bounded. + app.post("/forbidden-huge", (req, res) => { + res.status(403).type("text/plain").send("x".repeat(5000)); + }); + app.get("/forbidden-huge", (req, res) => { + res.status(403).type("text/plain").send("x".repeat(5000)); + }); + await new Promise((resolve) => { server = app.listen(0, () => { serverPort = (server.address() as any).port; @@ -297,4 +310,50 @@ describe("discovery error body (streamable_http + sse)", () => { expect(result.errors[0]).toContain("discovery refused: tenant is not provisioned for streaming"); expect(result.errors[0]).toContain("403"); }); +}); + +describe("error body edge cases", () => { + const callForbidden = async (path: string) => { + const protocol = new HttpCommunicationProtocol(); + try { + await protocol.callTool(mockClient, "test.forbidden", {}, { + name: "forbidden_server", + call_template_type: "http", + url: `http://localhost:${serverPort}${path}`, + http_method: "POST", + } as any); + } catch (e) { + return e as any; + } + throw new Error("expected the call to fail"); + }; + + test("a blank error body falls back to the status text instead of a bare colon", async () => { + const thrown = await callForbidden("/forbidden-empty"); + expect(thrown).toBeInstanceOf(Error); + expect(thrown.status).toBe(403); + expect(thrown.message).toContain("403"); + expect(thrown.message).toContain("Forbidden"); + expect(thrown.message.trimEnd().endsWith(":")).toBe(false); + }); + + test("a huge error body is truncated in the message but kept in full on data", async () => { + const thrown = await callForbidden("/forbidden-huge"); + expect(thrown.status).toBe(403); + expect(thrown.message.length).toBeLessThan(2500); + expect(thrown.data).toHaveLength(5000); + }); + + test("a huge discovery error body is truncated in errors[]", async () => { + const protocol = new StreamableHttpCommunicationProtocol(); + const result = await protocol.registerManual(mockClient, { + name: "huge_stream", + call_template_type: "streamable_http", + url: `http://localhost:${serverPort}/forbidden-huge`, + http_method: "GET", + } as any); + expect(result.success).toBe(false); + expect(result.errors[0]).toContain("403"); + expect(result.errors[0].length).toBeLessThan(400); + }); }); \ No newline at end of file From a7fb891f2b5057c22e444bba9c0a5de39fc583a2 Mon Sep 17 00:00:00 2001 From: Razvan Radulescu <43811028+h3xxit@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:09:34 +0200 Subject: [PATCH 5/7] http: a structured error field wins over a lower-priority generic string Mirrors the fix on python-utcp#102: the first of error / message / detail that is present decides. A non-blank string is the reason; an object falls through to the full JSON so its structure stays visible instead of being skipped in favour of a later generic message field. Null and blank strings are still skipped. Test added. Co-Authored-By: Claude Fable 5.1 --- .../http/src/http_communication_protocol.ts | 17 +++++++++++++++-- .../tests/http_communication_protocol.test.ts | 13 +++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index 0049b5e..bbb713e 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -251,8 +251,21 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { // APIs nest an OBJECT there (e.g. { error: { code, reason } }), so only use // the candidate when it's actually a string — otherwise fall through to the // full JSON so the real structure shows instead of "[object Object]". - const stringField = (...candidates: unknown[]): string | undefined => - candidates.find((c): c is string => typeof c === 'string'); + // The first of `error` / `message` / `detail` that is present decides: a + // non-blank string is the reason; anything else (an object, say) is a + // structured error, so return undefined to fall through to the full JSON + // rather than skipping ahead to a lower-priority generic string. + const stringField = (...candidates: unknown[]): string | undefined => { + for (const c of candidates) { + if (c === undefined || c === null) continue; + if (typeof c === 'string') { + if (c.trim()) return c; + continue; + } + return undefined; + } + return undefined; + }; // A blank body carries no reason: fall back to the status text (then // axios's own message) rather than emitting a message that ends in a // bare colon. Bound the detail so a multi-megabyte error page (an HTML diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index f2a8ebb..c59d62b 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -97,6 +97,11 @@ beforeAll(async () => { res.status(403).type("text/plain").send("x".repeat(5000)); }); + // A structured `error` next to a generic `message`: the structure must win. + app.post("/forbidden-object-then-message", (req, res) => { + res.status(422).json({ error: { code: "INVALID_FIELD", reason: "value out of range" }, message: "Request failed" }); + }); + await new Promise((resolve) => { server = app.listen(0, () => { serverPort = (server.address() as any).port; @@ -337,6 +342,14 @@ describe("error body edge cases", () => { expect(thrown.message.trimEnd().endsWith(":")).toBe(false); }); + test("a structured error field wins over a lower-priority generic message", async () => { + const thrown = await callForbidden("/forbidden-object-then-message"); + expect(thrown.status).toBe(422); + expect(thrown.message).toContain("INVALID_FIELD"); + expect(thrown.message).toContain("value out of range"); + expect(thrown.message).not.toContain("[object Object]"); + }); + test("a huge error body is truncated in the message but kept in full on data", async () => { const thrown = await callForbidden("/forbidden-huge"); expect(thrown.status).toBe(403); From efed66589c08e4afa792ca6853bf06cf34a849fb Mon Sep 17 00:00:00 2001 From: Razvan Radulescu <43811028+h3xxit@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:29:26 +0200 Subject: [PATCH 6/7] http: truncate error details by code point, never through a surrogate pair Addresses cubic review on #44. Slicing the detail by UTF-16 unit could split a surrogate pair at the boundary and leave a lone surrogate in the message. All three truncation sites (tool-call error detail, SSE and Streamable HTTP discovery) now truncate by code point. Test with an emoji sitting exactly on the boundary. Co-Authored-By: Claude Fable 5.1 --- packages/http/src/http_communication_protocol.ts | 5 ++++- packages/http/src/sse_communication_protocol.ts | 5 +++-- .../src/streamable_http_communication_protocol.ts | 5 +++-- .../http/tests/http_communication_protocol.test.ts | 14 ++++++++++++++ 4 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index bbb713e..624b53f 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -277,7 +277,10 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { : data == null ? error.message : (stringField(data.error, data.message, data.detail) ?? JSON.stringify(data)); - const detail = rawDetail.length > MAX_DETAIL_CHARS ? `${rawDetail.slice(0, MAX_DETAIL_CHARS)}…` : rawDetail; + // Truncate by code point, not UTF-16 unit, so a multi-byte character on + // the boundary is dropped whole rather than leaving a lone surrogate. + const codePoints = Array.from(rawDetail); + const detail = codePoints.length > MAX_DETAIL_CHARS ? `${codePoints.slice(0, MAX_DETAIL_CHARS).join('')}…` : rawDetail; const normalized = new Error( `HTTP ${status} calling tool '${toolName}': ${detail}`, ) as Error & { status: number; data: unknown }; diff --git a/packages/http/src/sse_communication_protocol.ts b/packages/http/src/sse_communication_protocol.ts index e885cb4..65ea9db 100644 --- a/packages/http/src/sse_communication_protocol.ts +++ b/packages/http/src/sse_communication_protocol.ts @@ -220,8 +220,9 @@ export class SseCommunicationProtocol implements CommunicationProtocol { // status code. Fall back to statusText when the body is empty. const body = await response.text().catch(() => ''); // Truncate like the streaming path does, so a huge error page does - // not land in errors[] and logs in full. - const detail = body.trim().slice(0, 200) || response.statusText; + // not land in errors[] and logs in full. By code point, so a + // multi-byte character on the boundary cannot leave a lone surrogate. + const detail = Array.from(body.trim()).slice(0, 200).join('') || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); } diff --git a/packages/http/src/streamable_http_communication_protocol.ts b/packages/http/src/streamable_http_communication_protocol.ts index a7d687f..b8cb86f 100644 --- a/packages/http/src/streamable_http_communication_protocol.ts +++ b/packages/http/src/streamable_http_communication_protocol.ts @@ -173,8 +173,9 @@ export class StreamableHttpCommunicationProtocol implements CommunicationProtoco // status code. Fall back to statusText when the body is empty. const body = await response.text().catch(() => ''); // Truncate like the streaming path does, so a huge error page does - // not land in errors[] and logs in full. - const detail = body.trim().slice(0, 200) || response.statusText; + // not land in errors[] and logs in full. By code point, so a + // multi-byte character on the boundary cannot leave a lone surrogate. + const detail = Array.from(body.trim()).slice(0, 200).join('') || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); } diff --git a/packages/http/tests/http_communication_protocol.test.ts b/packages/http/tests/http_communication_protocol.test.ts index c59d62b..d11be38 100644 --- a/packages/http/tests/http_communication_protocol.test.ts +++ b/packages/http/tests/http_communication_protocol.test.ts @@ -97,6 +97,11 @@ beforeAll(async () => { res.status(403).type("text/plain").send("x".repeat(5000)); }); + // An emoji sitting exactly on the truncation boundary of the message. + app.post("/forbidden-emoji-boundary", (req, res) => { + res.status(403).type("text/plain").send("x".repeat(1999) + "😀" + "y".repeat(50)); + }); + // A structured `error` next to a generic `message`: the structure must win. app.post("/forbidden-object-then-message", (req, res) => { res.status(422).json({ error: { code: "INVALID_FIELD", reason: "value out of range" }, message: "Request failed" }); @@ -350,6 +355,15 @@ describe("error body edge cases", () => { expect(thrown.message).not.toContain("[object Object]"); }); + test("truncation never splits a surrogate pair", async () => { + const thrown = await callForbidden("/forbidden-emoji-boundary"); + expect(thrown.status).toBe(403); + const loneSurrogate = /[\uD800-\uDBFF](?![\uDC00-\uDFFF])|(? { const thrown = await callForbidden("/forbidden-huge"); expect(thrown.status).toBe(403); From 1bd01e5f8e7537634780cb890e95d4d511f751d3 Mon Sep 17 00:00:00 2001 From: Razvan Radulescu <43811028+h3xxit@users.noreply.github.com> Date: Fri, 4 Sep 2026 16:46:33 +0200 Subject: [PATCH 7/7] http: truncate error details with an O(cap) code-point walk Addresses cubic review on #44. Array.from(rawDetail) materialised every code point of a possibly multi-megabyte body before the cap applied. A small truncateByCodePoint helper walks the string with an iterator and stops at the cap, and short strings skip the walk entirely. Used at all three truncation sites. Co-Authored-By: Claude Fable 5.1 --- packages/http/src/_text.ts | 29 +++++++++++++++++++ .../http/src/http_communication_protocol.ts | 4 +-- .../http/src/sse_communication_protocol.ts | 3 +- .../streamable_http_communication_protocol.ts | 3 +- 4 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 packages/http/src/_text.ts diff --git a/packages/http/src/_text.ts b/packages/http/src/_text.ts new file mode 100644 index 0000000..d86f5af --- /dev/null +++ b/packages/http/src/_text.ts @@ -0,0 +1,29 @@ +// packages/http/src/_text.ts + +/** + * Truncate `text` to at most `maxCodePoints` Unicode code points, appending + * an ellipsis when anything was cut. + * + * Works by code point rather than UTF-16 unit so a surrogate pair on the + * boundary is dropped whole instead of leaving a lone surrogate. Walks the + * string with an iterator and stops at the cap, so a multi-megabyte error + * body costs O(cap) work and allocation rather than an array of every + * character in the body. + */ +export function truncateByCodePoint(text: string, maxCodePoints: number): string { + // A string of N UTF-16 units has at most N code points, so short strings + // need no walk at all. + if (text.length <= maxCodePoints) { + return text; + } + let kept = ''; + let count = 0; + for (const codePoint of text) { + if (count === maxCodePoints) { + return `${kept}…`; + } + kept += codePoint; + count += 1; + } + return kept; +} diff --git a/packages/http/src/http_communication_protocol.ts b/packages/http/src/http_communication_protocol.ts index 624b53f..5299b25 100644 --- a/packages/http/src/http_communication_protocol.ts +++ b/packages/http/src/http_communication_protocol.ts @@ -14,6 +14,7 @@ import { IUtcpClient } from '@utcp/sdk'; import { HttpCallTemplateSchema, HttpCallTemplate } from './http_call_template'; import { OpenApiConverter } from './openapi_converter'; import { ensureSecureUrl, safeRequestWithRedirects, assertNoCrlf } from './_security'; +import { truncateByCodePoint } from './_text'; /** * HTTP communication protocol implementation for UTCP client. @@ -279,8 +280,7 @@ export class HttpCommunicationProtocol implements CommunicationProtocol { : (stringField(data.error, data.message, data.detail) ?? JSON.stringify(data)); // Truncate by code point, not UTF-16 unit, so a multi-byte character on // the boundary is dropped whole rather than leaving a lone surrogate. - const codePoints = Array.from(rawDetail); - const detail = codePoints.length > MAX_DETAIL_CHARS ? `${codePoints.slice(0, MAX_DETAIL_CHARS).join('')}…` : rawDetail; + const detail = truncateByCodePoint(rawDetail, MAX_DETAIL_CHARS); const normalized = new Error( `HTTP ${status} calling tool '${toolName}': ${detail}`, ) as Error & { status: number; data: unknown }; diff --git a/packages/http/src/sse_communication_protocol.ts b/packages/http/src/sse_communication_protocol.ts index 65ea9db..aaa5071 100644 --- a/packages/http/src/sse_communication_protocol.ts +++ b/packages/http/src/sse_communication_protocol.ts @@ -15,6 +15,7 @@ import { OAuth2UserAuth } from '@utcp/sdk'; import { IUtcpClient } from '@utcp/sdk'; import { SseCallTemplate, SseCallTemplateSchema } from './sse_call_template'; import { ensureSecureUrl, assertNoCrlf } from './_security'; +import { truncateByCodePoint } from './_text'; /** * A single parsed Server-Sent Event. @@ -222,7 +223,7 @@ export class SseCommunicationProtocol implements CommunicationProtocol { // Truncate like the streaming path does, so a huge error page does // not land in errors[] and logs in full. By code point, so a // multi-byte character on the boundary cannot leave a lone surrogate. - const detail = Array.from(body.trim()).slice(0, 200).join('') || response.statusText; + const detail = truncateByCodePoint(body.trim(), 200) || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); } diff --git a/packages/http/src/streamable_http_communication_protocol.ts b/packages/http/src/streamable_http_communication_protocol.ts index b8cb86f..c163b6d 100644 --- a/packages/http/src/streamable_http_communication_protocol.ts +++ b/packages/http/src/streamable_http_communication_protocol.ts @@ -15,6 +15,7 @@ import { OAuth2UserAuth } from '@utcp/sdk'; import { IUtcpClient } from '@utcp/sdk'; import { StreamableHttpCallTemplate, StreamableHttpCallTemplateSchema } from './streamable_http_call_template'; import { ensureSecureUrl, assertNoCrlf } from './_security'; +import { truncateByCodePoint } from './_text'; /** * REQUIRED @@ -175,7 +176,7 @@ export class StreamableHttpCommunicationProtocol implements CommunicationProtoco // Truncate like the streaming path does, so a huge error page does // not land in errors[] and logs in full. By code point, so a // multi-byte character on the boundary cannot leave a lone surrogate. - const detail = Array.from(body.trim()).slice(0, 200).join('') || response.statusText; + const detail = truncateByCodePoint(body.trim(), 200) || response.statusText; throw new Error(`HTTP ${response.status}: ${detail}`); }