diff --git a/CHANGELOG.md b/CHANGELOG.md index c0584c57c..5ee5ab9a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,7 @@ * [#2682](https://github.com/ruby-grape/grape/pull/2682): Fix `Style/OptionalBooleanParameter` offenses - [@ericproulx](https://github.com/ericproulx). * [#2699](https://github.com/ruby-grape/grape/pull/2699): Fix `Grape::Validations::Types::CustomTypeCoercer` dropping symbolized hash keys for `Array`/`Set` types; refactor the class for readability - [@ericproulx](https://github.com/ericproulx). * [#2700](https://github.com/ruby-grape/grape/pull/2700): Fix README typos, remove obsolete Ruby 2.4 / Fixnum section, and replace incorrect `requires + values + allow_blank` note with a correct one covering `optional + values` semantics (closes #2631) - [@ericproulx](https://github.com/ericproulx). +* [#2703](https://github.com/ruby-grape/grape/pull/2703): Catch exceptions raised inside `rescue_from` blocks; new `rescue_from :internal_grape_exceptions` opt-in for unrecognised internal errors (resolves [#2482](https://github.com/ruby-grape/grape/issues/2482)) - [@ericproulx](https://github.com/ericproulx). * Your contribution here. ### 3.2.1 (2026-04-16) diff --git a/README.md b/README.md index 2a65436a9..936d2b72d 100644 --- a/README.md +++ b/README.md @@ -2734,6 +2734,41 @@ class Twitter::API < Grape::API end ``` +#### Re-raising from inside a `rescue_from` block + +A `rescue_from` block can re-raise an exception to invoke a different handler. This is useful for translating one exception class into another: + +```ruby +class Twitter::API < Grape::API + rescue_from Grape::Exceptions::ValidationErrors do |e| + raise Api::Exceptions::InvalidValueError, e.full_messages + end + + rescue_from Api::Exceptions::InvalidValueError do |e| + error!({ errors: e.message }, 422) + end +end +``` + +The first handler re-raises; the second handler runs against the new exception. + +If the re-raised exception has no registered `rescue_from` and is a `Grape::Exceptions::Base` subclass, it is rendered through the default Grape error path (using its own `status` and `message`). Anything else — typos, `NoMethodError`, an unrelated `StandardError` — is treated as an internal error: it is exposed on `env['grape.exception']` for upstream Rack middleware to observe, and rendered to the API consumer as a generic `500 Internal Server Error`. This avoids leaking internal detail in the response body. + +You can take control of the internal-error path by opting in with `rescue_from :internal_grape_exceptions`: + +```ruby +class Twitter::API < Grape::API + rescue_from :internal_grape_exceptions do |e| + Sentry.capture_exception(e) + error!({ message: 'Something went wrong' }, 500) + end +end +``` + +When this handler is registered the framework hands the original exception to you, and you own the response shape and any logging. The framework deliberately does not log internal errors itself — it has no way to know your preferred format or destination. + +A second raise inside the redispatched handler is not redispatched again — it goes straight to the framework's generic 500. This bounds the chain at one redispatch and prevents loops. + You can also rescue all exceptions with a code block and handle the Rack response at the lowest level. ```ruby diff --git a/UPGRADING.md b/UPGRADING.md index 5b09cda88..e0d536b51 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -92,6 +92,36 @@ end **Behaviour change for code that didn't define a helper.** If your code references `logger` inside an endpoint context *without* a corresponding `helpers` definition, that call previously raised `NoMethodError` and now returns the API's configured logger. This is almost always the intended behaviour, but if you were relying on the `NoMethodError` (for instance to short-circuit logging in test environments via `rescue NoMethodError`), update your code to check `respond_to?(:logger)` or to gate logging on a feature flag. +#### Exceptions raised inside `rescue_from` blocks are now caught + +Previously, an exception raised inside a `rescue_from` block was uncaught and bubbled up to Rack, producing the Rack default 500 page. The framework now catches and routes it: + +1. If the re-raised exception's class has a registered `rescue_from` handler, that handler runs (one redispatch only — a second raise stops the chain). +2. If the re-raised exception is a `Grape::Exceptions::Base` subclass, it is rendered via the default Grape error path with its own `status` and `message`. +3. Otherwise, the original exception is exposed on `env['grape.exception']` for upstream Rack middleware to observe, and the response is a generic `Grape::Exceptions::InternalServerError` (`500 Internal Server Error`) — the original exception's message is **not** rendered to the API consumer. + +This means deliberate re-raises in a `rescue_from` block (e.g. translating one exception class into another) now compose with the rest of your `rescue_from` configuration, and accidental crashes (typos, `NoMethodError`, …) no longer leak internal detail to API consumers. + +The framework deliberately does **not** log unhandled internal exceptions itself — formatting and destination are application concerns. To log, forward to an error tracker, or customize the response shape for these errors, register a `rescue_from :internal_grape_exceptions` handler: + +```ruby +rescue_from :internal_grape_exceptions do |e| + Sentry.capture_exception(e) + error!({ message: 'Something went wrong' }, 500) +end +``` + +When this handler is registered, the framework hands the original exception to you and you own the response shape entirely. + +If you relied on the old behaviour and want raw exception messages exposed in development, register a catch-all handler: + +```ruby +rescue_from StandardError do |e| + error!({ message: e.message, class: e.class.name }, 500) +end +``` + + ### Upgrading to >= 3.2 #### Rack parameter parsing errors now raise `Grape::Exceptions::RequestError` diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb index c4236a350..33e6d255e 100644 --- a/lib/grape/dsl/request_response.rb +++ b/lib/grape/dsl/request_response.rb @@ -98,6 +98,8 @@ def rescue_from(*args, with: nil, **options, &block) inheritable_setting.namespace_inheritable[:rescue_all] = true inheritable_setting.namespace_inheritable[:rescue_grape_exceptions] = true inheritable_setting.namespace_inheritable[:grape_exceptions_rescue_handler] = handler + elsif args.include?(:internal_grape_exceptions) + inheritable_setting.namespace_inheritable[:internal_grape_exceptions_rescue_handler] = handler else handler_type = case options[:rescue_subclasses] diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index e5ec60d6f..bf22f0bb9 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -305,19 +305,7 @@ def build_stack stack.use Rack::Head stack.use Rack::Lint if lint? - stack.use Grape::Middleware::Error, - format:, - content_types:, - default_status: inheritable_setting.namespace_inheritable[:default_error_status], - rescue_all: inheritable_setting.namespace_inheritable[:rescue_all], - rescue_grape_exceptions: inheritable_setting.namespace_inheritable[:rescue_grape_exceptions], - default_error_formatter: inheritable_setting.namespace_inheritable[:default_error_formatter], - error_formatters: inheritable_setting.namespace_stackable_with_hash(:error_formatters), - rescue_options: inheritable_setting.namespace_stackable_with_hash(:rescue_options), - rescue_handlers:, - base_only_rescue_handlers: inheritable_setting.namespace_stackable_with_hash(:base_only_rescue_handlers), - all_rescue_handler: inheritable_setting.namespace_inheritable[:all_rescue_handler], - grape_exceptions_rescue_handler: inheritable_setting.namespace_inheritable[:grape_exceptions_rescue_handler] + stack.use Grape::Middleware::Error, **error_middleware_options(format, content_types) stack.concat inheritable_setting.namespace_stackable[:middleware] @@ -341,6 +329,26 @@ def build_stack builder.to_app end + def error_middleware_options(format, content_types) + ns_inh = inheritable_setting.namespace_inheritable + ns_stack = inheritable_setting + { + format:, + content_types:, + default_status: ns_inh[:default_error_status], + rescue_all: ns_inh[:rescue_all], + rescue_grape_exceptions: ns_inh[:rescue_grape_exceptions], + default_error_formatter: ns_inh[:default_error_formatter], + error_formatters: ns_stack.namespace_stackable_with_hash(:error_formatters), + rescue_options: ns_stack.namespace_stackable_with_hash(:rescue_options), + rescue_handlers:, + base_only_rescue_handlers: ns_stack.namespace_stackable_with_hash(:base_only_rescue_handlers), + all_rescue_handler: ns_inh[:all_rescue_handler], + grape_exceptions_rescue_handler: ns_inh[:grape_exceptions_rescue_handler], + internal_grape_exceptions_rescue_handler: ns_inh[:internal_grape_exceptions_rescue_handler] + } + end + def build_helpers helpers = inheritable_setting.namespace_stackable[:helpers] return if helpers.empty? diff --git a/lib/grape/env.rb b/lib/grape/env.rb index 09392fd2d..2dccf730a 100644 --- a/lib/grape/env.rb +++ b/lib/grape/env.rb @@ -16,5 +16,6 @@ module Env GRAPE_REQUEST_PARAMS = 'grape.request.params' GRAPE_ROUTING_ARGS = 'grape.routing_args' GRAPE_ALLOWED_METHODS = 'grape.allowed_methods' + GRAPE_EXCEPTION = 'grape.exception' end end diff --git a/lib/grape/exceptions/internal_server_error.rb b/lib/grape/exceptions/internal_server_error.rb new file mode 100644 index 000000000..944e1386e --- /dev/null +++ b/lib/grape/exceptions/internal_server_error.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module Grape + module Exceptions + # Raised internally when a +rescue_from+ handler itself raises an + # unrecognised exception. The framework substitutes the original + # exception with this safe stand-in for rendering, while preserving + # the original on +env[Grape::Env::GRAPE_EXCEPTION]+ for upstream + # observability (loggers, error trackers, etc.). + class InternalServerError < Base + def initialize + super(status: 500, message: compose_message(:internal_server_error)) + end + end + end +end diff --git a/lib/grape/locale/en.yml b/lib/grape/locale/en.yml index 01e2df808..4e1da6fb0 100644 --- a/lib/grape/locale/en.yml +++ b/lib/grape/locale/en.yml @@ -11,6 +11,7 @@ en: exactly_one: 'are missing, exactly one parameter must be provided' except_values: 'has a value not allowed' incompatible_option_values: '%{option1}: %{value1} is incompatible with %{option2}: %{value2}' + internal_server_error: 'Internal Server Error' invalid_accept_header: problem: 'invalid accept header' resolution: '%{message}' diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index cf8b1f62c..92af4db17 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -14,6 +14,7 @@ class Error < Base error_formatters: nil, format: :txt, grape_exceptions_rescue_handler: nil, + internal_grape_exceptions_rescue_handler: nil, rescue_all: false, rescue_grape_exceptions: false, rescue_handlers: nil, @@ -25,8 +26,8 @@ class Error < Base attr_reader :all_rescue_handler, :base_only_rescue_handlers, :default_error_formatter, :default_message, :default_status, :error_formatters, :format, - :grape_exceptions_rescue_handler, :rescue_all, :rescue_grape_exceptions, - :rescue_handlers + :grape_exceptions_rescue_handler, :internal_grape_exceptions_rescue_handler, + :rescue_all, :rescue_grape_exceptions, :rescue_handlers def initialize(app, **options) super @@ -38,6 +39,7 @@ def initialize(app, **options) @error_formatters = @options[:error_formatters] @format = @options[:format] @grape_exceptions_rescue_handler = @options[:grape_exceptions_rescue_handler] + @internal_grape_exceptions_rescue_handler = @options[:internal_grape_exceptions_rescue_handler] @rescue_all = @options[:rescue_all] @rescue_grape_exceptions = @options[:rescue_grape_exceptions] @rescue_handlers = @options[:rescue_handlers] @@ -127,10 +129,12 @@ def rescue_handler_for_any_class(klass) all_rescue_handler || method(:default_rescue_handler) end - def run_rescue_handler(handler, error, endpoint) + def run_rescue_handler(handler, error, endpoint, redispatched: false) handler = endpoint.public_method(handler) if handler.is_a?(Symbol) response = catch(:error) do handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler) + rescue StandardError => e + return redispatch(e, endpoint, redispatched) end if error?(response) @@ -142,6 +146,49 @@ def run_rescue_handler(handler, error, endpoint) end end + # Route an exception raised inside a +rescue_from+ block. + # + # * If we have already redispatched once (the redispatched handler + # itself raised), go straight to {#framework_default} — bounds the + # chain at one redispatch. + # * Else if the exception has a registered +rescue_from+ handler, + # run it. + # * Else if it's a +Grape::Exceptions::Base+ subclass, render it + # through +error_response+ with its own +status+ and +message+. + # * Else fall through to {#safe_default}, which lets the user opt + # in via +rescue_from :internal_grape_exceptions+ or, failing + # that, applies the framework default. + def redispatch(error, endpoint, already_redispatched) + return framework_default(endpoint) if already_redispatched + + if (registered = registered_rescue_handler(error.class)) + run_rescue_handler(registered, error, endpoint, redispatched: true) + elsif error.is_a?(Grape::Exceptions::Base) + run_rescue_handler(method(:error_response), error, endpoint, redispatched: true) + else + safe_default(error, endpoint) + end + end + + # The unrecognised-error path. Exposes the original exception on + # the rack env so upstream Rack middleware (loggers, error + # trackers) can observe it. If the user registered a + # +rescue_from :internal_grape_exceptions+ handler, that handler + # runs and owns the response. Otherwise the framework renders the + # generic +InternalServerError+ — never the original exception's + # message. The framework deliberately does no logging of its own + # here; that's the application's call. + def safe_default(error, endpoint) + env[Grape::Env::GRAPE_EXCEPTION] = error + return run_rescue_handler(internal_grape_exceptions_rescue_handler, error, endpoint, redispatched: true) if internal_grape_exceptions_rescue_handler + + framework_default(endpoint) + end + + def framework_default(endpoint) + run_rescue_handler(method(:default_rescue_handler), Grape::Exceptions::InternalServerError.new, endpoint) + end + def error!(message, status = default_status, headers = {}, backtrace = [], original_exception = nil) env[Grape::Env::API_ENDPOINT].status(status) # not error! inside route rack_response( diff --git a/spec/grape/middleware/error_spec.rb b/spec/grape/middleware/error_spec.rb index 58633af83..320a68d61 100644 --- a/spec/grape/middleware/error_spec.rb +++ b/spec/grape/middleware/error_spec.rb @@ -98,4 +98,208 @@ def self.call(_env) ) end end + + describe 'when a rescue_from block raises' do + subject(:response) do + get '/' + last_response + end + + context 'and the re-raised exception has a registered rescue_from' do + let(:app) do + Class.new(Grape::API) do + format :txt + + custom_error_class = Class.new(StandardError) + const_set(:CustomError, custom_error_class) + + rescue_from custom_error_class do |e| + error!("custom-handled: #{e.message}", 422) + end + + rescue_from :all do |e| + raise custom_error_class, "wrapped(#{e.message})" + end + + get('/') { raise ArgumentError, 'oops' } + end + end + + it 'redispatches to the registered handler' do + expect(response.status).to eq(422) + expect(response.body).to eq('custom-handled: wrapped(oops)') + end + end + + context 'and the re-raised exception is a Grape::Exceptions::Base subclass' do + let(:app) do + Class.new(Grape::API) do + format :txt + + teapot_class = Class.new(Grape::Exceptions::Base) do + def initialize + super(status: 418, message: 'teapot') + end + end + const_set(:TeapotError, teapot_class) + + rescue_from :all do + raise teapot_class + end + + get('/') { raise StandardError, 'first' } + end + end + + it 'renders the exception via the default Grape error path with its own status' do + expect(response.status).to eq(418) + expect(response.body).to eq('teapot') + end + end + + context 'and the re-raised exception is an unrecognised StandardError' do + let(:app) do + Class.new(Grape::API) do + format :txt + + rescue_from :all do + raise NoMethodError, "undefined method 'foo' for nil" + end + + get('/') { raise StandardError, 'first' } + end + end + + it 'renders the generic Internal Server Error response' do + expect(response.status).to eq(500) + expect(response.body).to eq('Internal Server Error') + end + + it "exposes the original exception via env['grape.exception']" do + captured = nil + original_call = app.method(:call) + allow(app).to receive(:call) do |env| + result = original_call.call(env) + captured = env[Grape::Env::GRAPE_EXCEPTION] + result + end + + response + + expect(captured).to be_a(NoMethodError) + expect(captured.message).to include("undefined method 'foo'") + end + end + + context 'and a redispatched handler also raises' do + let(:app) do + Class.new(Grape::API) do + format :txt + + inner_class = Class.new(StandardError) + outer_class = Class.new(StandardError) + const_set(:InnerError, inner_class) + const_set(:OuterError, outer_class) + + rescue_from inner_class do + raise outer_class, 'second-level' + end + + rescue_from outer_class do |e| + error!("would-handle: #{e.message}", 422) + end + + rescue_from :all do + raise inner_class, 'first-level' + end + + get('/') { raise StandardError, 'route' } + end + end + + it 'stops at the safe default after one redispatch' do + expect(response.status).to eq(500) + expect(response.body).to eq('Internal Server Error') + end + end + + context 'and the user has opted into rescue_from :internal_grape_exceptions' do + let(:app) do + Class.new(Grape::API) do + format :txt + + rescue_from :internal_grape_exceptions do |e| + error!("internal: #{e.class}: #{e.message}", 503) + end + + rescue_from :all do + raise NoMethodError, "undefined method 'foo' for nil" + end + + get('/') { raise StandardError, 'first' } + end + end + + it 'invokes the user handler with the original exception' do + expect(response.status).to eq(503) + expect(response.body).to eq("internal: NoMethodError: undefined method 'foo' for nil") + end + + it "still exposes the original exception via env['grape.exception']" do + captured = nil + original_call = app.method(:call) + allow(app).to receive(:call) do |env| + result = original_call.call(env) + captured = env[Grape::Env::GRAPE_EXCEPTION] + result + end + + response + + expect(captured).to be_a(NoMethodError) + end + + context 'and the user handler also raises' do + let(:app) do + Class.new(Grape::API) do + format :txt + + rescue_from :internal_grape_exceptions do + raise 'handler bug' + end + + rescue_from :all do + raise NoMethodError, 'first internal' + end + + get('/') { raise StandardError, 'route' } + end + end + + it 'falls through to the framework safe default (loop bounded)' do + expect(response.status).to eq(500) + expect(response.body).to eq('Internal Server Error') + end + end + end + + context 'and the handler returns a non-Response, non-error value' do + let(:app) do + Class.new(Grape::API) do + format :txt + + rescue_from :all do + 'not a Rack response' + end + + get('/') { raise StandardError, 'boom' } + end + end + + it 'falls through to the InvalidResponse path (existing behaviour preserved)' do + expect(response.status).to eq(500) + expect(response.body).to eq('Invalid response') + end + end + end end