Rank a nested rescue_from :all ahead of the enclosing scope's handlers

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.
 

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.

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

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

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.

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

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

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