Expose a swallowed exception on rack.exception in safe_default too - #2855
Merged
ericproulx merged 1 commit intoAug 18, 2026
Merged
Conversation
ericproulx
force-pushed
the
fix/safe-default-rack-exception
branch
from
August 17, 2026 16:13
84813ab to
846ce1d
Compare
5 tasks
Danger ReportNo issues found. |
dblock
approved these changes
Aug 17, 2026
ericproulx
force-pushed
the
fix/error-rendering-failsafe
branch
from
August 18, 2026 06:14
abad865 to
395aa2c
Compare
ericproulx
force-pushed
the
fix/safe-default-rack-exception
branch
from
August 18, 2026 06:23
846ce1d to
7ee6c41
Compare
ericproulx
force-pushed
the
fix/error-rendering-failsafe
branch
from
August 18, 2026 06:47
395aa2c to
0c743e6
Compare
safe_default is the unrecognised-error path: an exception raised inside a rescue_from block that nothing else handles. Grape answers a generic 500 there rather than letting it propagate, and records the exception on env['grape.exception']. That key is Grape's own and no error tracker reads it. Because the exception never propagates, a tracker mounted above Grape has nothing to catch either, so a bug in a rescue_from block has always been able to turn into a silent 500 — the request is answered, the log says nothing, and the tracker never fires. Publish it on env['rack.exception'] as well, the convention for an exception that was handled rather than raised, which sentry-ruby collects as env['rack.exception'] || env['sinatra.error']. Extracted as expose_exception, now shared with the rendering failsafe. The deliberate silence of this path is left alone: unlike the failsafe, a rescue_from :internal_grape_exceptions handler can still own the response here, so the logging stays the application's call. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ericproulx
force-pushed
the
fix/safe-default-rack-exception
branch
from
August 18, 2026 06:47
7ee6c41 to
cb36168
Compare
ericproulx
added a commit
that referenced
this pull request
Aug 22, 2026
…2855) safe_default is the unrecognised-error path: an exception raised inside a rescue_from block that nothing else handles. Grape answers a generic 500 there rather than letting it propagate, and records the exception on env['grape.exception']. That key is Grape's own and no error tracker reads it. Because the exception never propagates, a tracker mounted above Grape has nothing to catch either, so a bug in a rescue_from block has always been able to turn into a silent 500 — the request is answered, the log says nothing, and the tracker never fires. Publish it on env['rack.exception'] as well, the convention for an exception that was handled rather than raised, which sentry-ruby collects as env['rack.exception'] || env['sinatra.error']. Extracted as expose_exception, now shared with the rendering failsafe. The deliberate silence of this path is left alone: unlike the failsafe, a rescue_from :internal_grape_exceptions handler can still own the response here, so the logging stays the application's call. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericproulx
added a commit
that referenced
this pull request
Aug 22, 2026
…2855) safe_default is the unrecognised-error path: an exception raised inside a rescue_from block that nothing else handles. Grape answers a generic 500 there rather than letting it propagate, and records the exception on env['grape.exception']. That key is Grape's own and no error tracker reads it. Because the exception never propagates, a tracker mounted above Grape has nothing to catch either, so a bug in a rescue_from block has always been able to turn into a silent 500 — the request is answered, the log says nothing, and the tracker never fires. Publish it on env['rack.exception'] as well, the convention for an exception that was handled rather than raised, which sentry-ruby collects as env['rack.exception'] || env['sinatra.error']. Extracted as expose_exception, now shared with the rendering failsafe. The deliberate silence of this path is left alone: unlike the failsafe, a rescue_from :internal_grape_exceptions handler can still own the response here, so the logging stays the application's call. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericproulx
added a commit
that referenced
this pull request
Aug 22, 2026
* Answer 500 when an error response cannot be rendered
Grape::Middleware::Error#call! renders the error response from inside its own
rescue clause, so that clause never covered the rendering. An error formatter
that raised on the payload it was handed took the exception straight out
through every middleware above Grape and into the application server —
`rescue_from :all` did not help, because the failure happened after the
handler had already returned.
A rescue_from handler echoing request-derived bytes was enough to hit it:
rescue_from(Missing) { |e| error!({ detail: e.message }, 404) }
with an invalid UTF-8 byte in the path, the JSON formatter raised
JSON::GeneratorError and the request died rather than being answered.
Guard the rendering in error_response. On failure, first retry the API's own
format with the framework's InternalServerError, whose message is a static
string and so cannot be what defeated the first attempt; if that fails too — a
formatter broken outright rather than one payload it choked on — answer
without a formatter at all. Both attempts call format_message directly instead
of re-entering error_response, so the fallback cannot recurse. This is the
shape ActionDispatch::ShowExceptions#render_exception already has in Rails,
down to the text/plain last resort.
The guard sits on the rendering rather than around run_rescue_handler on
purpose. Wrapping the handler call too would have swallowed things that must
keep propagating, the deprecation raised when a handler returns a Hash among
them.
Exceptions that no rescue_from matches still propagate unchanged; only
rendering failures are caught.
Swallowing an exception must not make it invisible. Grape put the exception on
env['grape.exception'], but that is a Grape-private key no tracker reads, so a
rendering failure that Sentry used to report as a raised exception would have
become an unremarkable 500. Publish it on env['rack.exception'] as well — the
convention for an exception that was handled rather than raised, which
sentry-ruby collects as `env['rack.exception'] || env['sinatra.error']` — and
write the failure to rack.errors so it reaches the server log even with no
tracker installed. Rails likewise writes to $stderr from its failsafe branch:
deferring the logging to the application is not an option here, since the
application's own error rendering is precisely what broke.
Grape.config.raise_rendering_errors opts back out: an application that would
rather have the exception propagate out of the middleware stack, as it did
before, can have that. Off by default.
Rendering is split into render_response / record_rendering_failure /
render_failsafe_response so each tier is named rather than nested in a begin
block inside error_response.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* Expose a swallowed exception on rack.exception in safe_default too (#2855)
safe_default is the unrecognised-error path: an exception raised inside a
rescue_from block that nothing else handles. Grape answers a generic 500 there
rather than letting it propagate, and records the exception on
env['grape.exception'].
That key is Grape's own and no error tracker reads it. Because the exception
never propagates, a tracker mounted above Grape has nothing to catch either, so
a bug in a rescue_from block has always been able to turn into a silent 500 —
the request is answered, the log says nothing, and the tracker never fires.
Publish it on env['rack.exception'] as well, the convention for an exception
that was handled rather than raised, which sentry-ruby collects as
env['rack.exception'] || env['sinatra.error']. Extracted as expose_exception,
now shared with the rendering failsafe.
The deliberate silence of this path is left alone: unlike the failsafe, a
rescue_from :internal_grape_exceptions handler can still own the response here,
so the logging stays the application's call.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 <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
2 participants
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.
Split out of #2840 at review request — that PR is now strictly the rendering failsafe.
Depends on #2840, which introduces
Grape::Env::RACK_EXCEPTIONand the shared exposure helper. Based on its branch; the diff here is one call site, a spec, and a README correction. Merge #2840 first and this rebases cleanly.The gap
safe_defaultis the unrecognised-error path: an exception raised inside arescue_fromblock that nothing else handles. Grape answers a generic 500 there rather than letting it propagate, and records the exception onenv['grape.exception'].That key is Grape's own and no error tracker reads it. Because the exception never propagates, a tracker mounted above Grape has nothing to catch either. So a bug in a
rescue_fromblock — a typo'd method, a nil deref — has always been able to become a silent 500: the request is answered, the log says nothing, the tracker never fires.This is longstanding, not a regression. It surfaced while fixing #2840, whose failsafe had the identical blind spot.
The change
Publish the exception on
env['rack.exception']as well — the convention for an exception that was handled rather than raised, which sentry-ruby collects asenv['rack.exception'] || env['sinatra.error']. Extracted asexpose_exception, now shared with the rendering failsafe in #2840.grape.exceptionkeeps its current meaning; nothing is removed.Documentation
README described this path as exposing the exception on
env['grape.exception']"for upstream Rack middleware to observe" — the claim this PR makes actually true. Updated to namerack.exceptionand say why the key is needed at all: the exception is answered rather than raised, so nothing above Grape catches it.No UPGRADING entry: adding an env key is additive, not a contract break.
What is deliberately not changed
This path stays silent — no
rack.errorswrite, unlike the failsafe in #2840. The difference is that here arescue_from :internal_grape_exceptionshandler can still own the response, so the logging remains the application's call. In the failsafe there is no such option, because the application's own error rendering is what broke.Test plan
rack.exceptionon the unrecognised-error path; verified it fails without the change (expected nil to be a kind of NoMethodError).🤖 Generated with Claude Code