Replace InvalidArgumentError with ServerError #67

Activity

1. jankapunkt commented on Nov 16, 2021

   

   jankapunkt

   on Nov 16, 2021

   Member

   More actions

   Please also add, which parts of the code are affected and which tests need to be updated.
2. jankapunkt

   added

   compliance 📜OAuth 2.0 standard complianceOAuth 2.0 standard compliance

   tests 🧪Relates to testsRelates to tests

   on Nov 16, 2021
3. Uzlopak commented on Nov 16, 2021

   

   Uzlopak

   on Nov 16, 2021

   ContributorAuthor

   More actions

   Its actually scattered all over the place. It is everywhere used where an InternalServerError should be thrown because the implementor made an implementation error.
4. Uzlopak commented on Nov 16, 2021

   

   Uzlopak

   on Nov 16, 2021

   ContributorAuthor

   More actions

   We could also just patch the InvalidArgumentError to extend ServerError or change invalid_argument to server_error.
5. jankapunkt

   added

   hacktoberfest

   on Sep 29, 2022
6. jankapunkt

   removed

   hacktoberfest

   on Feb 20, 2023
7. jankapunkt

   added

   hacktoberfest

   on Oct 1, 2023
8. jankapunkt

   removed

   hacktoberfest

   on Nov 5, 2023
9. jankapunkt

   added

   hacktoberfest

   on Sep 27, 2024
10. jankapunkt

    removed

    hacktoberfest

    on Jan 22, 2025
11. dhensby commented on Jun 17, 2026

    

    dhensby

    on Jun 17, 2026

    Contributor

    More actions

    Revisiting this with a full census + serialization trace. The premise is half right, so I've opened a targeted PR rather than a blanket replacement.

    invalid_argument is genuinely non-standard — it's 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. But of the ~56 InvalidArgumentError throw sites in lib/, ~32 are construction/config-time guards (missing model/options, model does not implement X(), request/response instance checks). Those throw synchronously out of constructors or before the handler try block, are handed straight back to the integrator, and never reach an OAuth client — there a descriptive invalid_argument is a useful developer signal, and the conformance objection simply doesn't apply.

    The real defect is narrow. Both the token and authorize handler catch blocks only re-wrap if (!(e instanceof OAuthError)) as ServerError. Because InvalidArgumentError extends OAuthError, it slips through and gets serialized verbatim — updateErrorResponse sets error: error.name, so a request-time InvalidArgumentError reaches the client as error=invalid_argument with HTTP 500. The reachable sites:

    • a model returning malformed token data → TokenModel / BearerTokenType throw (lib/models/token-model.js, lib/token-types/bearer-token-type.js)
    • a model returning a falsy authorization code → CodeResponseType throws (lib/response-types/code-response-type.js)

    Fix (#451): normalise InvalidArgumentError (like any other internal error) to ServerError at the serialisation boundary — so the wire reports a standard server_error/503, with the original preserved as .inner for server-side logs. The construction/config guards are untouched and still surface InvalidArgumentError to integrator code. This concentrates the change in the two catch blocks rather than rewriting 56 throw sites + ~14 test files.

    This is breaking (the wire error/status for those runtime failures changes from invalid_argument/500 to server_error/503), so it targets the next major.
12. jankapunkt

    closed this as completedin #451on Jul 19, 2026