fix(rpcclient): handle non-dict error bodies from JSON-RPC 500 responses - #27
Merged
Merged
Conversation
`rpcclient.py:75` does `raise ServerError(**err["error"])` which
requires `err["error"]` to be a mapping. Well-behaved JSON-RPC servers
return a dict there, but tutorial.theseed.org (and other backends
under load) occasionally return a plain string or a list. The
`**` unpack then raises
TypeError: argument after ** must be a mapping, not str
which masks the real upstream failure and surfaces to callers as a
completely unrelated exception. modelseed-api's failure_watcher has
been auto-filing GitHub issues for this shape.
Fix: check `isinstance(error_body, dict)` before the `**` unpack. When
it's not a mapping, coerce the body into `ServerError("Unknown", 0,
str(body))` - the same shape we already produce for a non-JSON 500 or
a JSON 500 without an `error` key. Callers see a `ServerError` with
the upstream body attached to `.message` instead of a `TypeError` with
no context.
5 unit tests in tests/core/test_rpcclient.py:
- dict error body -> populated ServerError (happy path)
- string error body -> Unknown ServerError with body in message (regression)
- list error body -> same shape as string
- 500 with no `error` key -> pre-existing Unknown branch, locked in
- 500 with non-JSON content-type -> pre-existing branch, locked in
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
2 participants
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.
rpcclient.py:75doesraise ServerError(**err[\"error\"])which requireserr[\"error\"]to be a mapping. Well-behaved JSON-RPC servers return a dict there, but tutorial.theseed.org (and other backends under load) occasionally return a plain string or a list. The**unpack then raiseswhich masks the real upstream failure and surfaces to callers as a completely unrelated exception. modelseed-api's failure_watcher has been auto-filing GitHub issues for this shape periodically.
Fix
Check
isinstance(error_body, dict)before the**unpack. When it's not a mapping, coerce intoServerError(\"Unknown\", 0, str(body))- the same shape already produced for a non-JSON 500 or a JSON 500 without anerrorkey. Callers now see aServerErrorwith the upstream body attached to.messageinstead of aTypeErrorwith no context.Tests
5 unit tests in
tests/core/test_rpcclient.py:ServerError(happy path)UnknownServerErrorwith body in.message(regression)errorkey → pre-existingUnknownbranch, locked inAll 5 pass locally.
Context
Companion to the modelseed-api Phase 3 bulk-reconstruction work (which loops the ModelSEEDpy annotate-fasta primitives directly). We've been catching this via the annotator's retry logic in modelseed-api, but the rpcclient shouldn't lose the upstream context in the first place.
🤖 Generated with Claude Code