fix!: normalise InvalidArgumentError to server_error on the wire#451
Open
dhensby wants to merge 1 commit into
Open
fix!: normalise InvalidArgumentError to server_error on the wire#451dhensby wants to merge 1 commit into
dhensby wants to merge 1 commit into
Conversation
InvalidArgumentError carries the non-standard `invalid_argument` code (defined in neither RFC 6749 §5.2 nor §4.1.2.1) but extends OAuthError, so the token and authorize handler catch blocks let it slip through their "wrap any non-OAuthError as ServerError" guard. As a result, a misbehaving model that returns malformed token data (e.g. a non-Date `accessTokenExpiresAt`) or a falsy authorization code caused the server to emit `error=invalid_argument` with HTTP 500 to the client instead of a standard `server_error`. Treat InvalidArgumentError like any other internal error at the serialisation boundary: normalise it to ServerError before it reaches the client, preserving the original as `.inner`. Construction- and request-validation guards (missing options, model-capability checks, request/response instance checks) are thrown before the try block and are unaffected — they still surface InvalidArgumentError to the integrator as a descriptive developer-facing error. BREAKING CHANGE: when a model returns malformed token data or a falsy authorization code at request time, the OAuth error response now reports `error: server_error` with HTTP status 503 instead of `error: invalid_argument` with HTTP status 500. Setup-time InvalidArgumentErrors thrown to integrator code are unchanged. Co-Authored-By: Claude Opus 4.8 (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
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.
What
InvalidArgumentErrorcarries the non-standardinvalid_argumentcode but extendsOAuthError, so the token and authorize handler catch blocks — which only wrap non-OAuthErrorerrors asServerError— let it pass straight through and serialize it to the client. This normalisesInvalidArgumentErrortoServerErrorat the serialisation boundary, so genuine runtime failures surface as a standardserver_errorinstead of leakinginvalid_argument.Resolves #67.
Why
invalid_argumentis defined in none of RFC 6749's error sets (§5.2 token endpoint; §4.1.2.1 / §4.2.2.1 authorization endpoint) nor the IANA registry. Of the ~56InvalidArgumentErrorthrow sites, ~32 are construction/config-time guards thrown straight back to the integrator that never reach a client — for those,invalid_argumentis a useful developer signal and stays as-is. The defect is the small subset thrown during request handling, inside the handlertry, which currently serialize aserror=invalid_argument/ HTTP 500:TokenModel/BearerTokenTypethrowCodeResponseTypethrowsChange
Both handler catch blocks now also coerce
InvalidArgumentErrortoServerError(preserving the original as.inner):Setup-time guards (missing options,
model does not implement X(),request/responseinstance checks) are thrown before thetryblock, so they're unaffected and still surfaceInvalidArgumentErrorto integrator code.Tests
Added two integration tests proving the wire output for the reachable paths is now
server_error/503 (token endpoint) and anerror=server_errorredirect (authorize endpoint), with the originalInvalidArgumentErrorretained as.inner. Full suite green; no existingInvalidArgumentErrorassertion changes (all are on pre-tryconstruction / request-instance paths).BREAKING CHANGE
When a model returns malformed token data or a falsy authorization code at request time, the OAuth error response now reports
error: server_error(HTTP 503) instead oferror: invalid_argument(HTTP 500). Setup-timeInvalidArgumentErrors thrown to integrator code are unchanged.