fix(api): keep every 500 on the ADR 0031 envelope; narrow the fail-closed handler (#101, #102)#116
Merged
Merged
Conversation
…osed handler (#101, #102) #101 (two gaps in the error envelope): - An unmapped TollgateError fell to the 500 fallback but still echoed str(exc), so an internal invariant string (e.g. "idempotency replay is missing its stored response") surfaced in the body. Unmapped errors now use the generic fallback message; mapped errors keep their controlled domain message. - A non-TollgateError/non-datastore exception (a genuine bug) was handled by Starlette's default middleware as plain-text "Internal Server Error", off the documented envelope. A catch-all Exception handler now returns the enveloped generic 500 and logs the traceback so the defect is not lost. #102: the fail-closed handler was registered for bare OSError, a very broad base — any incidental non-datastore OSError/TimeoutError on the request path became a retryable 503 enforcement_unavailable, telling the SDK the datastore is down and masking a real defect. Narrowed to ConnectionError + the builtin TimeoutError plus the SQLAlchemy connectivity types (OperationalError, InterfaceError, TimeoutError); driver-wrapped connect/DNS failures still fail closed, while a bare OSError now surfaces as a 500 via the catch-all. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BjJ4QScUKbrMzF1jujL5Bg
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
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.
#101 — 500s not consistently enveloped / internal messages leaking
Two gaps:
TollgateErrorechoedstr(exc). ATollgateErrorsubtype not in the mapping fell to the 500 fallback but still surfacedstr(exc), so internal invariant strings (e.g."idempotency replay is missing its stored response") leaked into the 500 body. Unmapped errors now use the generic fallback message; mapped errors keep echoing their controlled domain message (e.g.insufficient budget at user:u1), unchanged.TollgateError/non-datastore exception (a genuine bug) was handled by Starlette's default middleware as plain-textInternal Server Error, off the{"error": {...}}contract. A catch-allExceptionhandler now returns the enveloped generic 500 and logs the traceback so the defect isn't lost.#102 — fail-closed handler on bare
OSErrorwas too broadhandle_unavailablewas registered forOSError— a very broad base, so any incidental non-datastoreOSError/TimeoutErroron the request path became503 enforcement_unavailable, telling the SDK the datastore is down (inviting fail-closed-with-grace) while masking a real defect. Narrowed toConnectionError+ the builtinTimeoutErrorplus the SQLAlchemy connectivity types (OperationalError,InterfaceError, SQLAlchemyTimeoutError). Driver-wrapped connect/DNS/timeout failures still fail closed to 503; a bareOSErrornow surfaces as a 500 via the catch-all.Handler resolution is by MRO, so
ConnectionRefusedError→ConnectionErrorhandler (503), while a bareOSError→ theExceptioncatch-all (500).Tests
TollgateError("<internal string>")→ 500 with the generic message (no leak); aValueError→ enveloped 500 (not plain text); a bareOSError→ 500internal_error(not 503).ConnectionRefusedError,TimeoutError,OperationalError,InterfaceError, SQLAlchemyTimeoutError) and the full domain-error mapping.Local gate: ruff, mypy --strict, import-linter, full unit suite (492), API-http integration — all green.
Closes #101, #102.