Skip to content

fix!: normalise InvalidArgumentError to server_error on the wire#451

Open
dhensby wants to merge 1 commit into
node-oauth:masterfrom
dhensby:fix/invalid-argument-error-leak
Open

fix!: normalise InvalidArgumentError to server_error on the wire#451
dhensby wants to merge 1 commit into
node-oauth:masterfrom
dhensby:fix/invalid-argument-error-leak

Conversation

@dhensby

@dhensby dhensby commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

What

InvalidArgumentError carries the non-standard invalid_argument code but extends OAuthError, so the token and authorize handler catch blocks — which only wrap non-OAuthError errors as ServerError — let it pass straight through and serialize it to the client. This normalises InvalidArgumentError to ServerError at the serialisation boundary, so genuine runtime failures surface as a standard server_error instead of leaking invalid_argument.

Resolves #67.

Why

invalid_argument is 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 ~56 InvalidArgumentError throw sites, ~32 are construction/config-time guards thrown straight back to the integrator that never reach a client — for those, invalid_argument is a useful developer signal and stays as-is. The defect is the small subset thrown during request handling, inside the handler try, which currently serialize as error=invalid_argument / HTTP 500:

  • a model returning malformed token data → TokenModel / BearerTokenType throw
  • a model returning a falsy authorization code → CodeResponseType throws

Change

Both handler catch blocks now also coerce InvalidArgumentError to ServerError (preserving the original as .inner):

if (!(e instanceof OAuthError) || e instanceof InvalidArgumentError) {
  e = new ServerError(e);
}

Setup-time guards (missing options, model does not implement X(), request/response instance checks) are thrown before the try block, so they're unaffected and still surface InvalidArgumentError to integrator code.

Tests

Added two integration tests proving the wire output for the reachable paths is now server_error/503 (token endpoint) and an error=server_error redirect (authorize endpoint), with the original InvalidArgumentError retained as .inner. Full suite green; no existing InvalidArgumentError assertion changes (all are on pre-try construction / 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 of error: invalid_argument (HTTP 500). Setup-time InvalidArgumentErrors thrown to integrator code are unchanged.

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>
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.

Replace InvalidArgumentError with ServerError

1 participant