Skip to content

LLM errors preserve their response bodies - #538

Merged
WaylandYang merged 1 commit into
deeplethe:devfrom
ousamabenyounes:fix/issue-527
Sep 9, 2026
Merged

WaylandYang merged 1 commit into
deeplethe:devfrom
ousamabenyounes:fix/issue-527

Conversation

@ousamabenyounes

Copy link
Copy Markdown
Contributor

Summary

  • Check the HTTP status before decoding successful LLM responses as JSON.
  • Preserve plain-text and unfamiliar JSON error bodies, capped at 500 Unicode characters.
  • Keep quota and rate-limit classification for recognized JSON errors across chat, streaming, tool, and embedding requests.

Closes #527.

Test verification (RED → GREEN)

On unmodified dev, a local endpoint returning 502 Bad Gateway with proxy says the model context is too long failed as:

assertion failed: !is_unreachable(&error)

The same regression test passes after the patch and includes the endpoint diagnosis in the error.

Validation

  • cargo test -p utopia-llm: 17 passed, 0 failed.
  • Fix removed temporarily: the regression test fails again with the original Unreachable classification.
  • Changed executable production lines: 100% covered (28/28).
  • Full local suite on upstream dev at cb996aa: migrations twice, formatting, workspace clippy with warnings denied, workspace tests, required DB-backed store tests, workspace build, and frozen-lockfile web tests/build all passed.
  • Suite comparison: baseline 659 passed / 0 failed / 1 ignored; patched 691 passed / 0 failed / 1 ignored (the patched total includes the targeted coverage replay).

Signed-off-by: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com>
@WaylandYang
WaylandYang merged commit 357d4ac into deeplethe:dev Sep 9, 2026
@WaylandYang

Copy link
Copy Markdown
Contributor

Merged. This is the fix #527 asked for, in the shape it asked for: the body is read once as text, the status is checked before anything is parsed as JSON, the three JSON shapes are tried and the raw text is quoted otherwise, capped at 500 characters, and a body that cannot be read at all stays Unreachable. Updating the doc comment on Unreachable to say what it now excludes was the right call; that comment is what the next reader will trust.

The tests are the useful part: one raw-socket server per case, every request path covered for the non-JSON body, the incomplete body pinned as unreachable, and the multibyte truncation counted in characters. Thanks for reading the issue's test list and writing to it.

One habit to mention for next time, not a blocker: the constants pulled out of the tests and the production code (LLM_REQUEST_KIND, TEST_USER_ROLE, SUCCESS_TEXT and friends) add lines without adding meaning here; a literal at the point of use reads faster in a test. This repository leans toward the literal.

@WaylandYang

Copy link
Copy Markdown
Contributor

One thing you could not have seen from Linux: the raw-socket servers in the tests wrote their answer and closed without reading the request. Windows answers an unread socket with RST, so the client got "connection aborted" (os error 10053) instead of the body, and two of the six tests failed on a Windows checkout while passing on CI. #542 has them read the request first and shut down cleanly; behaviour under test is unchanged. Worth knowing for the next fake server. The same PR puts the literals back at their point of use, as mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An error body the client cannot parse is reported as unknown error

2 participants