fix(http): surface the server's error body on a failed tool call - #26
Open
edujuan wants to merge 3 commits into
Open
fix(http): surface the server's error body on a failed tool call#26edujuan wants to merge 3 commits into
edujuan wants to merge 3 commits into
Conversation
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.
…iscovery
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.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
`_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.
Member
Author
|
@cubic-dev-ai the P1 (object-valued error field → |
Contributor
@edujuan I have started the AI code review. It will take a few minutes to complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When an HTTP call fails with a non-2xx status, the http protocols discard the server's response body — where servers put the actual reason (
{ "error": "..." }) — and surface only the status code.HttpCommunicationProtocol.callTool(axios) re-threw the rawAxiosError, whose.messageis the generic"Request failed with status code 403". The body sits onerror.response.dataand never reaches.message. It's also dropped entirely when the error crosses a serialization boundary (e.g.JSON.stringifyinside anisolated-vmtool runner like@utcp/code-mode), becauseError.messageis non-enumerable andAxiosErrordoesn't serialize itsresponse. In a code-mode agent a server's 403/400 reaches the model as{}.StreamableHttpCommunicationProtocol/SseCommunicationProtocol(fetch) threwHTTP ${status}: ${statusText}during manual discovery (registerManual) without ever reading the body, so a refused discovery surfaced as"HTTP 403: Forbidden"inerrors[].Fix
callToolnormalizes a failed HTTP call into anErrorthat folds the status + server body into.messageAND attaches enumerablestatus/datafields, so the reason survives both.messagereaders and structured serialization. Non-HTTP errors (network, timeout) pass through unchanged.statusTextwhen empty). TheircallToolpaths are stubs (no HTTP call yet), so discovery is the only real failure surface today.No public signature changes.
Tests
/forbiddenroute (403 + JSON body) + acallTooltest asserting the thrown error carries the body in.message, exposesstatus/data, and round-trips throughJSON.stringify(the sandbox case).GET /forbidden-discoveryroute (403 + text body) + tests asserting both streamable and sseregisterManualsurface the body inerrors[].bun test packages/http/tests/→ 85 pass, 0 fail.build:httpclean (DTS included) oncecoreis built first.