Map JetStream errors in callers, not the API client - #984
Conversation
The api/client layer is meant to be raw endpoint dispatch. Each method wrapped request_json in a try/except that re-raised a specific error type (StreamNotFoundError, ConsumerNotFoundError, ...) keyed on error_code. Move that mapping to the callers (JetStream in __init__.py, Stream in stream.py), where the call site documents which errors each operation produces. request_json still raises the generic JetStreamError, so errors stay exceptions; api/client now just builds the request and returns the response.
|
Claude finished @caspervonb's task in 4m 4s —— View job Review
The refactoring is correct. I traced all 11 error mappings that were removed from
One minor issue: the No asyncio issues, no type accuracy regressions, no security concerns. |
![synadia-claude-reviewer[bot]](https://avatars.githubusercontent.com/u/132956691?s=60&v=4){kind=small}
| @@ -757,7 +792,25 @@ async def account_info(self) -> AccountInfo: | |||
| JetStreamNotEnabledForAccountError: If JetStream is not enabled for this account | |||
| JetStreamError: For other JetStream API errors | |||
| """ | |||
There was a problem hiding this comment.
The deferred import of NoRespondersError inside the method body is non-idiomatic. Since nats.client is already a module-level dependency (e.g. from nats.client.message import Headers at the top of the file), there is no circular import risk here. This should sit with the other nats.client imports at the top of the file.
|
Review comments addressed, PTAL @claude |
|
Claude finished @caspervonb's task in 31s —— View job Review
The review comment has been addressed. from nats.client.errors import NoRespondersError
from nats.client.message import Headers
from nats.client.protocol.message import parse_headersNo other issues. LGTM. |
1 participant
The
api/clientlayer is meant to be raw endpoint dispatch, but each method wrappedrequest_jsonin atry/exceptthat re-raised a specific type (StreamNotFoundError,ConsumerNotFoundError, …) keyed onerror_code.Move that mapping to the callers —
JetStream(__init__.py) andStream(stream.py) — so the call site documents which errors each operation produces (inline per caller).request_jsonstill raises the genericJetStreamError, so errors stay exceptions and can't be silently dropped;api/clientnow just builds the request and returns the response.The 14 existing tests that assert specific typed errors (
StreamNotFoundError,ConsumerNotFoundError,MessageNotFoundError,StreamNameAlreadyInUseError,ConsumerInvalidResetError,JetStreamNotEnabledError) all still pass; full suite is 308 green.