From 4b7a3b3b08052e2c7fe224504127d4809e1321db Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Wed, 29 Jul 2026 23:06:25 +0200 Subject: [PATCH] Answer 500 when an error response cannot be rendered MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. 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. The exception that defeated rendering is put on env['grape.exception'], the key the existing unrecognised-error path already uses, so upstream loggers can still observe it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 2 + UPGRADING.md | 14 ++++++ lib/grape/middleware/error.rb | 47 +++++++++++++++++++- spec/grape/middleware/error_spec.rb | 67 +++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0049465b9..85b04c2aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,8 @@ * [#2827](https://github.com/ruby-grape/grape/pull/2827): Make the `cascade` DSL getter return the configured value (`cascade false` read back as `true`) - [@ericproulx](https://github.com/ericproulx). * [#2829](https://github.com/ruby-grape/grape/pull/2829): Fix a cascading route handing over only to the last route registered for the path, making a middle version (3+ mounted versions with a catch-all) answer 406 - [@ericproulx](https://github.com/ericproulx). * [#2826](https://github.com/ruby-grape/grape/pull/2826): Fix `api.version` not being set for the root route of a path-versioned API (`GET /v1`) - [@ericproulx](https://github.com/ericproulx). +* [#2834](https://github.com/ruby-grape/grape/pull/2834): Restore the #2824 fix for cascaded routes leaking `route_info` and path captures, silently reverted by #2829 - [@ericproulx](https://github.com/ericproulx). +* [#2840](https://github.com/ruby-grape/grape/pull/2840): Answer 500 instead of letting an exception escape the middleware stack when an error response cannot be rendered (see UPGRADING) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.3.5 (2026-07-30) diff --git a/UPGRADING.md b/UPGRADING.md index ab4838229..2dd67043b 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -3,6 +3,20 @@ Upgrading Grape ### Upgrading to >= 4.0.0 +#### A failed error rendering answers 500 instead of escaping the middleware stack + +When Grape could not render an error response — an error formatter handed a payload it cannot serialize, most often — the exception escaped every middleware above Grape and reached the application server. Rendering runs inside `Grape::Middleware::Error#call!`'s own `rescue` clause, so that clause did not cover it. + +Grape now answers `500` instead: first retrying the API's format with the framework's own `Internal Server Error` message, then falling back to a bare `text/plain` body if even that cannot be rendered. + +**What can break.** Code that observed these exceptions by letting them propagate — an error tracker mounted as Rack middleware above Grape, or a test asserting `expect { get '/' }.to raise_error` — no longer sees them. The exception is exposed on the rack env instead, under the same key the existing unrecognised-error path uses: + +```ruby +env[Grape::Env::GRAPE_EXCEPTION] # => the exception that defeated rendering +``` + +Exceptions that no `rescue_from` matches still propagate exactly as before; only rendering failures changed. + #### `Array`/`Set` of an unsupported type is rejected when the API is defined Declaring a collection whose element type Grape cannot coerce — `type: Array[Foo]` or `type: Set[Foo]` where `Foo` is neither a primitive, a structure, nor a valid custom type — now raises as soon as the `params` block is evaluated, i.e. while the API class is being loaded: diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index f12fa398e..b1c569da8 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -48,6 +48,13 @@ def initialize( def_delegator :rescue_options, :backtrace, :include_backtrace def_delegator :rescue_options, :original_exception, :include_original_exception + # Emitted by {#failsafe_response} once even the framework's own message + # could not be rendered. Deliberately built without a formatter, an i18n + # lookup or anything else that could be the thing that is broken. + FAILSAFE_STATUS = 500 + FAILSAFE_MESSAGE = '500 Internal Server Error' + FAILSAFE_CONTENT_TYPE = 'text/plain' + def call!(env) @env = env error_response(catch(:error) { return @app.call(@env) }) @@ -104,7 +111,45 @@ def error_response(error = nil) backtrace: raw.backtrace || raw.original_exception&.backtrace || [] ) env[Grape::Env::API_ENDPOINT].status(payload.status) # error! may not have been called - rack_response(payload.status, payload.headers, format_message(payload)) + begin + rack_response(payload.status, payload.headers, format_message(payload)) + rescue StandardError => e + failsafe_response(e) + end + end + + # Last resort for an error response that could not be rendered — an error + # formatter handed a payload it cannot serialize, typically. Rendering runs + # inside #call!'s rescue clause, so it is not covered by that rescue and + # anything raised here would escape the entire middleware stack. Grape has + # committed to answering with an error by this point, so it answers with + # one that does not depend on the payload rather than dropping the request. + # + # 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. Should even that fail — a wholesale broken formatter, rather than + # one payload it choked on — drop the formatter entirely. Both attempts call + # {#format_message} directly rather than re-entering {#error_response}, so + # this path cannot recurse. + # + # The exception is exposed on the rack env so upstream middleware (loggers, + # error trackers) can still observe what actually went wrong. + def failsafe_response(error) + env[Grape::Env::GRAPE_EXCEPTION] = error + headers = { Rack::CONTENT_TYPE => content_type } + rack_response(FAILSAFE_STATUS, headers, format_message(failsafe_payload(headers))) + rescue StandardError + rack_response(FAILSAFE_STATUS, { Rack::CONTENT_TYPE => FAILSAFE_CONTENT_TYPE }, FAILSAFE_MESSAGE) + end + + def failsafe_payload(headers) + Grape::Exceptions::ErrorResponse.new( + status: FAILSAFE_STATUS, + message: Grape::Exceptions::InternalServerError.new.message, + headers:, + backtrace: [], + original_exception: nil + ) end def default_rescue_handler(exception) diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 320a68d61..5edf8be84 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -99,6 +99,73 @@ def self.call(_env) end end + # Rendering happens inside #call!'s rescue clause, so it is not covered by + # that rescue: without a failsafe an error formatter that raises takes the + # exception straight out through every middleware above. + describe 'when the error response cannot be rendered' do + subject(:response) do + get '/' + last_response + end + + context 'and the formatter chokes on the payload' do + let(:app) do + Class.new(Grape::API) do + format :json + + rescue_from(:all) { |e| error!({ detail: e.message }, 404) } + + # A message the JSON formatter cannot serialize. + get('/') { raise StandardError, +"bad \xC3 byte".b } + end + end + + it 'answers with the framework message in the API format' do + expect(response.status).to eq(500) + expect(response.headers[Rack::CONTENT_TYPE]).to include('application/json') + expect(JSON.parse(response.body)).to eq('error' => 'Internal Server Error') + end + + it 'exposes the rendering failure on the rack env' do + get '/' + expect(last_request.env[Grape::Env::GRAPE_EXCEPTION]).to be_a(StandardError) + end + end + + context 'and the formatter is broken outright' do + let(:app) do + Class.new(Grape::API) do + format :json + + error_formatter :json, ->(**) { raise 'formatter is broken' } + rescue_from(:all) { error!({ detail: 'nope' }, 404) } + + get('/') { raise StandardError, 'boom' } + end + end + + it 'drops the formatter rather than recursing' do + expect(response.status).to eq(500) + expect(response.headers[Rack::CONTENT_TYPE]).to include('text/plain') + expect(response.body).to eq('500 Internal Server Error') + end + end + + context 'and nothing rescues the original exception' do + let(:app) do + Class.new(Grape::API) do + format :json + + get('/') { raise ArgumentError, 'kaboom' } + end + end + + it 'keeps propagating it' do + expect { get '/' }.to raise_error(ArgumentError, 'kaboom') + end + end + end + describe 'when a rescue_from block raises' do subject(:response) do get '/'