Answer 500 when an error response cannot be rendered

[ericproulx]

Summary

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 — the failure happens after the handler has already returned its payload.

The trigger is client-controlled, which is what makes this more than a cosmetic issue. A rescue_from handler echoing request-derived bytes is enough:

class API < Grape::API
  format :json
  rescue_from(Missing) { |e| error!({ error: 'not_found', detail: e.message }, 404) }
  route_param(:id) { get { raise Missing, "no such thing: #{params[:id]}" } }
end

GET /%C3%28 — an invalid UTF-8 byte in the path — makes the JSON formatter raise JSON::GeneratorError, and the request dies instead of being answered. Any API whose handler interpolates request-derived text can be made to defeat its own error handling on demand.

scenario	before	after
formatter chokes on the payload	JSON::GeneratorError escapes the stack	500 {"error":"Internal Server Error"} in the API format
formatter broken outright	RuntimeError escapes the stack	500 text/plain 500 Internal Server Error
no rescue_from matches the exception	propagates	propagates (unchanged)
format has no error formatter	falls back to the default error formatter	unchanged

Approach

Guard the rendering. On failure, first retry the API's own format with the framework's InternalServerError — its message is a static string, so it cannot be what defeated the first attempt — and if that fails too, answer without a formatter at all. Both attempts call format_message directly rather than re-entering error_response, so the fallback cannot recurse.

This is the shape Rails already has. ActionDispatch::ShowExceptions#render_exception tries the application's own error rendering and falls back to a hardcoded [500, text/plain, "500 Internal Server Error\n..."] when that rendering is itself broken — same two tiers, same status, same content type, and it calls the second tier "failsafe" too.

Rendering is split so each tier is named rather than nested in a begin block inside error_response:

render_response(payload)   # → record_rendering_failure + render_failsafe_response

The guard sits on the rendering rather than around run_rescue_handler. I tried the wider placement first; it swallowed things that must keep propagating, including the deprecation raised when a handler returns a Hash (error_spec.rb:95). Routing the retry through framework_default was also wrong — that goes back through run_rescue_handler, whose failure path redispatches into framework_default again, turning a clean RuntimeError into SystemStackError.

Swallowing an exception must not make it invisible

Answering 500 means an exception that used to propagate no longer does, and Grape is usually mounted inside a host stack that was reporting it. Setting env['grape.exception'] is not enough on its own: that key is Grape-private and nothing in the ecosystem reads it, so a rendering failure Sentry used to report as a raised exception would have quietly become an unremarkable 500.

So the exception is also published on env['rack.exception'] — the convention for an exception that was handled rather than raised, which sentry-ruby collects as env['rack.exception'] || env['sinatra.error'] — and the failure is written to rack.errors, so it reaches the server log even with no tracker installed. Measured with a tracker middleware mounted above Grape, using the repro above:

	tracker observes	client gets
master	raised: JSON::GeneratorError	500 text/html, the host's error page
this PR	rack.exception: JSON::GeneratorError	500 application/json

Rails writes to $stderr from its failsafe branch for the same reason: deferring the logging to the application is not an option there, because the application's own error rendering is precisely what broke.

Opting out

Grape.config.raise_rendering_errors keeps the old behaviour, alongside the existing lint / warn_on_helper_overrides settings:

Grape.configure do |config|
  config.raise_rendering_errors = true
end

With it on the failsafe never runs: the exception is re-raised untouched, so neither env key is set and nothing is written to rack.errors — whatever caught it before catches it again.

Backward compatibility

UPGRADING entry added, including the opt-out. Tests asserting expect { get '/' }.to raise_error on a rendering failure no longer see it raised; error trackers above Grape keep reporting it via rack.exception with no application change. Exceptions that no rescue_from matches still propagate exactly as before.

README documents the behaviour under Exception Handling, with raise_rendering_errors listed in Configuration.

Test plan

• Seven examples in error_spec.rb covering both fallback tiers, grape.exception, rack.exception, rack.errors, the opt-out, and the invariant that an unrescued exception still propagates; verified the behavioural ones fail without the lib/ change.
• End-to-end check with a tracker middleware mounted above Grape (table above), on master and on this branch, plus the documented example verified in both config states.
• Full RSpec suite passes locally (2576 examples, 0 failures).
• RuboCop clean.
• CI green.

---

Follow-up: #2855 applies the same rack.exception exposure to safe_default, the pre-existing unrecognised-error path, which had the identical blind spot. Split out of this PR at review request; based on this branch since it needs Grape::Env::RACK_EXCEPTION.

🤖 Generated with Claude Code

[github-actions]

Danger Report

No issues found.

View run

[dblock]

If Grape is mounted inside another stack like Rails or Sinatra, which it usually is, this causes a different 500 than before. This makes me a little uneasy. Is there a strong argument to change the escape behavior?

[ericproulx]

You were right to push on this, and the concrete cost is worse than "a different 500": mounted in Rails, the exception stopped being reported. I had it on env['grape.exception'], but that's our own key — nothing reads it. I measured it with a tracker middleware above Grape:

	tracker observes
master	raised: JSON::GeneratorError
this PR, as you reviewed it	nothing

So a rendering failure Sentry used to report became an unremarkable 500. That's a real regression and it's fixed now: the exception is also published on env['rack.exception'] — the convention for an exception that was handled rather than raised, which sentry-ruby collects as env['rack.exception'] || env['sinatra.error'] — plus a write to rack.errors so it lands in the log with no tracker installed. Trackers report it again with no application change.

On whether there's a strong argument for changing the escape behavior — two:

The trigger is client-controlled. The repro is an invalid UTF-8 byte in the path, echoed through e.message into a rescue_from payload. Any API whose handler interpolates request-derived text can be made to defeat its own error handling by a crafted request. That's the part I'd weigh most.

Rails already does exactly this. ActionDispatch::ShowExceptions#render_exception tries the app's own error rendering and falls back to a hardcoded [500, text/plain, "500 Internal Server Error\n..."] when that rendering is itself broken — same two tiers, same status and content type, and it calls the fallback "failsafe" too. It also writes to $stderr there, for the reason above: you can't defer the logging to the application when the application's renderer is what broke. Sinatra is the counterexample — it propagates — but it also defaults raise_errors on, so it never claimed to contain errors the way rescue_from :all does.

Worth noting the swallow policy isn't new either: safe_default already answers a framework 500 for an unrecognised exception raised inside a rescue_from block. This PR closes the last hole in it rather than opening one.

One thing to flag, since it's scope I added after your review: safe_default had the same rack.exception blind spot, so it now sets that key too. Fixing one of two identical paths seemed worse than fixing both, but it does mean this touches a shipped path. Happy to split it out if you'd rather keep the diff to the rendering failure.

[ericproulx]

Split the safe_default change out into #2855 as suggested — this PR is now strictly the rendering failsafe. #2855 is based on this branch since it needs Grape::Env::RACK_EXCEPTION, so it rebases to nothing once this merges.

[dblock]

This still makes me uneasy. Grape is just a rack middleware, by attempting to rescue StandardError we abort the rack middleware stack as it's intended. But maybe I'm overthinking it?

Is there an easy way to allow for the old behavior?

Btw, copilot suggests unwrapping this a little like so:

def render_response(payload)
  rack_response(payload.status, payload.headers, format_message(payload))
rescue StandardError => error
  record_rendering_failure(error)
  render_failsafe_response
end

def render_failsafe_response
  headers = { Rack::CONTENT_TYPE => content_type }
  payload = failsafe_payload(headers)

  rack_response(FAILSAFE_STATUS, headers, format_message(payload))
rescue StandardError
  rack_response(
    FAILSAFE_STATUS,
    { Rack::CONTENT_TYPE => FAILSAFE_CONTENT_TYPE },
    FAILSAFE_MESSAGE
  )
end

[ericproulx]

Thanks! Took all three.

The layering point. I don't think you're overthinking it — it's the right question — but Grape::Middleware::Error#call! already does exactly this, and more broadly:

rescue Exception => e # rubocop:disable Lint/RescueException

It rescues Exception, not StandardError. Intercepting before the rest of the rack stack sees it is this middleware's job — that's what rescue_from is. This PR doesn't add interception, it extends the existing rescue to cover the one statement in the same method that it accidentally didn't, because the rendering happens inside the clause body.

The scope is narrower than "we rescue StandardError" too: the failsafe only fires once Grape has already committed to handling the error — a rescue_from matched, or error! was called. An exception Grape was never handling still propagates untouched, which the 'and nothing rescues the original exception' spec pins.

Opting out. Added Grape.config.raise_rendering_errors, defaulting to false, alongside the existing lint / warn_on_helper_overrides settings:

Grape.configure do |config|
  config.raise_rendering_errors = true
end

Documented in the README and UPGRADING, with a spec asserting the JSON::GeneratorError propagates when it's on.

The refactor. Applied — it's better than what I had. Splitting record_rendering_failure out of the fallback rendering is the part I'd missed; those were two jobs in one method. error_response is now flat, and each tier has a name:

render_response(payload)   # → record_rendering_failure + render_failsafe_response

Also rebased #2855 on top, so it's back to one call site plus a spec.