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 '/'