From 90459608c8285d32e567a08493d18552ce8b8e56 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Tue, 10 Nov 2026 02:53:30 +0100 Subject: [PATCH] Rank a nested rescue_from :all ahead of the enclosing scope's handlers A rescue_from :all declared in a namespace or a mounted API lost to any class handler of an enclosing scope that matched, although a nested class handler wins over an outer one whatever the classes are (#1405). The class handlers of every scope were scanned before the catch-all was consulted. The scope that declared rescue_from :all now ends its handlers with an :all entry, so the catch-all ranks behind its own scope's class handlers and ahead of every enclosing scope's. An exception it does not catch, one that is not a StandardError, scans past it as before, and so does one raised inside a rescue_from block, which is never handed to a catch-all. The mount-order specs from #2384 pinned the outer class handler winning; they now expect the mounted API's catch-all, for either mount order. Supersedes #1982. See #1975. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + README.md | 2 + UPGRADING.md | 19 +++++ lib/grape/middleware/error.rb | 20 ++++- lib/grape/util/inheritable_setting.rb | 25 ++++-- lib/grape/util/shadowed_rescue_handlers.rb | 2 +- .../grape/api/mount_and_helpers_order_spec.rb | 12 +-- spec/grape/api_spec.rb | 82 +++++++++++++++++++ 8 files changed, 145 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 135b7803b..879f1f918 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ * [#3070](https://github.com/ruby-grape/grape/pull/3070): Speed up auto-`OPTIONS`, 405 and 404 answers on APIs with many paths - [@ericproulx](https://github.com/ericproulx). * [#3071](https://github.com/ruby-grape/grape/pull/3071): Skip the query parse when building the params of a request with no query string - [@ericproulx](https://github.com/ericproulx). * [#3072](https://github.com/ruby-grape/grape/pull/3072): Answer the `Accept` lists axios, jQuery and HTTPie send without negotiating them on every request - [@ericproulx](https://github.com/ericproulx). +* [#3073](https://github.com/ruby-grape/grape/pull/3073): Let a `rescue_from :all` in a namespace or mounted API outrank the enclosing scope's handlers - [@ericproulx](https://github.com/ericproulx). * [#3076](https://github.com/ruby-grape/grape/pull/3076): Skip the query merge when building the params of a body sent without a query string - [@ericproulx](https://github.com/ericproulx). * Your contribution here. diff --git a/README.md b/README.md index 3302b4321..d3c3159e3 100644 --- a/README.md +++ b/README.md @@ -3221,6 +3221,8 @@ end Here `'inner'` will be result of handling occurred `ArgumentError`. +A `rescue_from :all` inside the namespace takes precedence over the root scope's handlers in the same way, including the root's `rescue_from ArgumentError`. It still ranks behind the namespace's own class handlers, and an exception it does not catch, one that is not a `StandardError`, still reaches the root's handlers. + #### Unrescuable Exceptions `Grape::Exceptions::InvalidVersionHeader`, which is raised when the version in the request header doesn't match the currently evaluated version for the endpoint, will _never_ be rescued from a `rescue_from` block (even a `rescue_from :all`) This is because Grape relies on Rack to catch that error and try the next versioned-route for cases where there exist identical Grape endpoints with different versions. diff --git a/UPGRADING.md b/UPGRADING.md index 6a5df4273..f438c4056 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -94,6 +94,25 @@ Every such request now answers the router's `404`, with `X-Cascade: pass` still * A custom validator receives such a name as a Symbol in `validate_param!(attr_name, params)`. Compare `attr_name.to_s` if it checked the name against a String. Error messages and `declared(params)` name it as before. * `given 'name'` finds a parameter declared as `:name`, and `given :name` one declared as `'name'`, where both raised `Grape::Exceptions::UnknownParameter`. +#### A `rescue_from :all` in a namespace or mounted API outranks the enclosing scope's handlers + +A `rescue_from :all` declared in a namespace or a mounted API now takes precedence over the handlers of the scopes around it, as a nested class handler already did ([#3073](https://github.com/ruby-grape/grape/pull/3073)). An enclosing scope's handler used to win whenever its class matched, so with the API below a `ZeroDivisionError` raised under `/alpacas` was answered by the root's handler, not the namespace's catch-all: + +```ruby +rescue_from ZeroDivisionError do + error!('root', 500) +end + +namespace :alpacas do + rescue_from :all do + error!('alpacas', 503) # now answers the ZeroDivisionError + end + get { 1 / 0 } +end +``` + +To keep the enclosing handler for such an exception, register its class in the nested scope as well: a scope's class handlers still rank ahead of its own `rescue_from :all`. An exception `rescue_from :all` does not catch, one that is not a `StandardError`, still reaches the enclosing scope's handlers, and so does one raised inside the catch-all's block. + ### Upgrading to >= 4.1.0 #### `Rack::Head` wraps the router instead of each endpoint's stack diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 18b1596db..65bd5ab71 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -292,22 +292,34 @@ def default_rescue_handler(exception) ) end + # An exception raised in a +rescue_from+ block is never handed to a + # catch-all, so the +:all+ entry does not end the scan: an enclosing + # scope's handler still takes it. def registered_rescue_handler(klass) - registered_rescue_handler_entry(klass)&.last + registered_rescue_handler_entry(klass, catch_all: false)&.last end # The matched entry rather than just its handler, so callers can tell # *which* class matched — see {#grape_exceptions_precedence_handler}. + # + # Nil once the scan reaches the +:all+ entry of the nearest scope that + # declared +rescue_from :all+ (see InheritableSetting#rescue_handlers): + # what it catches goes to the catch-all, after #find_handler's Grape + # exception checks, rather than to an enclosing scope's handler. # @return [Array(Class, #call), nil] - def registered_rescue_handler_entry(klass) + def registered_rescue_handler_entry(klass, catch_all: true) rescue_handler_entry_from(base_only_rescue_handlers) { |err| klass == err } || - rescue_handler_entry_from(rescue_handlers) { |err| klass <= err } + rescue_handler_entry_from(rescue_handlers) do |err| + next klass <= err unless err == :all + + catch_all && klass <= StandardError + end end def rescue_handler_entry_from(handlers) error, handler = handlers&.find { |err, _handler| yield(err) } - return unless error + return if error.nil? || error == :all [error, handler || method(:default_rescue_handler)] end diff --git a/lib/grape/util/inheritable_setting.rb b/lib/grape/util/inheritable_setting.rb index 4c30bf3c3..4e18bf3a2 100644 --- a/lib/grape/util/inheritable_setting.rb +++ b/lib/grape/util/inheritable_setting.rb @@ -389,13 +389,13 @@ def internal_grape_exceptions_rescue_handler # Rescue-handler maps registered by +rescue_from+, keyed by exception # class and merged so a nested scope's handler wins. Record them with # #add_rescue_handlers; the backing store is an internal detail. - def rescue_handlers - merged_rescue_handlers(:rescue_handlers) - end + # + # #rescue_handlers also holds an +:all+ entry where a scope declared + # +rescue_from :all+, ranking that catch-all behind the scope's own + # handlers but ahead of every enclosing scope's. + def rescue_handlers = merged_rescue_handlers(:rescue_handlers) - def base_only_rescue_handlers - merged_rescue_handlers(:base_only_rescue_handlers) - end + def base_only_rescue_handlers = merged_rescue_handlers(:base_only_rescue_handlers) # An exception class registered twice in the same scope keeps its first # handler, and keeps the position it was first registered at. @@ -682,7 +682,7 @@ def stacked_keys # when an outer scope registered a more specific class. def merged_rescue_handlers(key) inherited = parent&.merged_rescue_handlers(key) - own = @rescue_handler_maps&.[](key) + own = own_rescue_handlers(key) return inherited unless own own.merge(inherited || {}) { |_klass, nearer, _inherited| nearer } @@ -706,6 +706,17 @@ def copy_state_from(source) private + # The +:all+ entry is added on read, so it follows the scope's own + # handlers however they were interleaved with the +rescue_from :all+. + # Only #rescue_handlers carries it: Middleware::Error scans every + # scope's exact-match handlers before any of these. + def own_rescue_handlers(key) + own = @rescue_handler_maps&.[](key) + return own unless key == :rescue_handlers && @namespace_inheritable&.key?(:all_rescue_handler) + + (own || {}).merge(all: @namespace_inheritable[:all_rescue_handler]) + end + # Overrides +key+ for this scope, leaving the enclosing scopes' value # untouched. The store is allocated on first use. Returns the assigned # value, since the writers built on it are +=+ methods. diff --git a/lib/grape/util/shadowed_rescue_handlers.rb b/lib/grape/util/shadowed_rescue_handlers.rb index b5180adc5..5def3e9b0 100644 --- a/lib/grape/util/shadowed_rescue_handlers.rb +++ b/lib/grape/util/shadowed_rescue_handlers.rb @@ -12,7 +12,7 @@ module Util # rescue_from ArgumentError do ... end # never runs # # Warn rather than reorder: which should win is the author's call, and - # +rescue_from :all+ (consulted only after the registered handlers) already + # +rescue_from :all+ (consulted only after its scope's registered handlers) already # offers "broad first, specific still wins" to anyone who wants it. module ShadowedRescueHandlers module_function diff --git a/spec/grape/api/mount_and_helpers_order_spec.rb b/spec/grape/api/mount_and_helpers_order_spec.rb index 3485a820e..ba18fe4f6 100644 --- a/spec/grape/api/mount_and_helpers_order_spec.rb +++ b/spec/grape/api/mount_and_helpers_order_spec.rb @@ -31,11 +31,11 @@ def app main_rescue_from_after end - it 'is rescued by the rescue_from ZeroDivisionError handler from Main class' do + it 'is rescued by the rescue_from :all handler of the mounted API' do get '/' - expect(last_response.status).to eq(500) - expect(last_response.body).to eq({ type: 'zero' }.to_json) + expect(last_response.status).to eq(404) + expect(last_response.body).to eq({ type: 'all' }.to_json) end end @@ -67,11 +67,11 @@ def app main_rescue_from_before end - it 'is rescued by the rescue_from ZeroDivisionError handler from Main class' do + it 'is rescued by the rescue_from :all handler of the mounted API' do get '/' - expect(last_response.status).to eq(500) - expect(last_response.body).to eq({ type: 'zero' }.to_json) + expect(last_response.status).to eq(404) + expect(last_response.body).to eq({ type: 'all' }.to_json) end end end diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 21519ebd7..37dd1123a 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -4884,6 +4884,88 @@ def self.call(object, _env) get '/mounted/fail' expect(last_response.body).to eq('inner rescue') end + + it 'prefers a rescue_from :all defined by mounted over an outer class handler' do + subject.rescue_from ArgumentError do + error!('outer rescue') + end + + app = Class.new(described_class) + + subject.namespace :mounted do + rescue_from :all do + error!('inner rescue') + end + app.get('/fail') { raise ArgumentError.new } + mount app + end + + get '/mounted/fail' + expect(last_response.body).to eq('inner rescue') + end + + it 'prefers class handlers defined by mounted over its rescue_from :all' do + subject.rescue_from :all do + error!('outer rescue') + end + + app = Class.new(described_class) + + subject.namespace :mounted do + rescue_from :all do + error!('inner catch-all') + end + rescue_from ArgumentError do + error!('inner rescue') + end + app.get('/fail') { raise ArgumentError.new } + mount app + end + + get '/mounted/fail' + expect(last_response.body).to eq('inner rescue') + end + + it 'leaves what a rescue_from :all defined by mounted does not catch to the outer handlers' do + subject.rescue_from Exception do |e| + error!("outer rescue of #{e.class}") + end + + app = Class.new(described_class) + + subject.namespace :mounted do + rescue_from :all do |e| + error!("inner rescue of #{e.class}") + end + app.get('/standard') { raise ArgumentError.new } + app.get('/script') { raise NotImplementedError.new } + mount app + end + + get '/mounted/standard' + expect(last_response.body).to eq('inner rescue of ArgumentError') + get '/mounted/script' + expect(last_response.body).to eq('outer rescue of NotImplementedError') + end + + it 'hands an exception raised by a rescue_from :all defined by mounted to an outer class handler' do + subject.rescue_from ArgumentError do |e| + error!("outer rescue of #{e.message}") + end + + app = Class.new(described_class) + + subject.namespace :mounted do + rescue_from :all do + raise ArgumentError, 'translated' + end + app.get('/fail') { raise 'doh!' } + mount app + end + + get '/mounted/fail' + expect(last_response.body).to eq('outer rescue of translated') + end end it 'collects the routes of the mounted api' do