Require a formatter in error_formatter and drop the unreachable 406 throw - #2901
Merged
Conversation
ericproulx
force-pushed
the
error-formatter-nil-guard
branch
from
September 4, 2026 21:37
87971ea to
5fa824e
Compare
Danger ReportNo issues found. |
This was referenced Sep 4, 2026
ericproulx
force-pushed
the
error-formatter-nil-guard
branch
from
September 4, 2026 22:09
5fa824e to
3ee9d16
Compare
dblock
approved these changes
Sep 5, 2026
…hrow `error_formatter :json` — with no formatter, positionally or as `with:` — registered `nil`. `ErrorFormatter.formatter_for` returns a registered value verbatim through its `key?` short-circuit, so that format resolved to no formatter at all, and `Middleware::Error#format_message` fell through to a `throw :error, 406`. That throw could never deliver its 406: nothing catches `:error` around the call. `#call!` has already left its `catch` by the time `error_response` runs, and `run_rescue_handler` renders outside its own. So it raised `UncaughtThrowError`, `render_response` rescued it, and the request answered with the failsafe `500 Internal Server Error` in `text/plain` instead of the status and message the API asked for. Raise `ArgumentError` from the DSL when no formatter is given, so the mistake surfaces where it is made, and drop the dead branch — `formatter_for` always resolves to a registered formatter, `default_error_formatter` or `ErrorFormatter::Txt`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericproulx
force-pushed
the
error-formatter-nil-guard
branch
from
September 5, 2026 09:27
3ee9d16 to
e006314
Compare
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.
error_formatter :json— with no formatter, positionally or aswith:— registerednil:Grape::ErrorFormatter.formatter_forhands a registered value back verbatim through itskey?short-circuit, so that format resolved to no formatter at all andMiddleware::Error#format_messagefell through to athrow :error, 406.That throw could never deliver its 406. Nothing catches
:erroraround the call:#call!has already left itscatchby the timeerror_responseruns with the caught value, andrun_rescue_handlerre-renders outside its owncatch. So it raisedUncaughtThrowError,render_responserescued it, and the request answered with the failsafe500 Internal Server Errorintext/plain— whatever status and message the API had asked for:format :json+error_formatter :json)format_messagecallserror!('nope', 422)text/plaintext/plainBoth ends are fixed here:
DSL::RequestResponse#error_formatterraisesArgumentErrorwhen no formatter is given, so the mistake surfaces where it is made rather than on the first error the API renders.format_messagedrops the dead branch —formatter_foralways resolves to a registered formatter,default_error_formatterorErrorFormatter::Txt.An API that passes a formatter is unaffected, and a format with no
error_formatterof its own keeps falling back todefault_error_formatterand then toTxt(pinned by a new middleware spec).🤖 Generated with Claude Code