Fix rescue_from with invalid response (#2478)

* Remove expect_any_instance_of(...) Wrap `default_rescue_handler` with `method` * Add CHANGELOG entry * Fix RSpec/AnyInstance Regenerate Rubocop's todo * Update spec wording * Update spec/grape/api_spec.rb Co-authored-by: Manuel Jacob <[email protected]> --------- Co-authored-by: Manuel Jacob <[email protected]>
ruby-grape · Jul 23, 2024 · 838c75e · 838c75e
1 parent fb67ea9
commit 838c75e
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 21 deletions.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,6 +1,6 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-06-22 17:26:10 UTC using RuboCop version 1.64.1.
+# on 2024-07-23 11:24:53 UTC using RuboCop version 1.64.1.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
@@ -37,12 +37,6 @@ Naming/VariableNumber:
     - 'spec/grape/exceptions/validation_errors_spec.rb'
     - 'spec/grape/validations_spec.rb'
 
-# Offense count: 2
-RSpec/AnyInstance:
-  Exclude:
-    - 'spec/grape/api_spec.rb'
-    - 'spec/grape/middleware/base_spec.rb'
-
 # Offense count: 1
 # Configuration parameters: IgnoredMetadata.
 RSpec/DescribeClass:
@@ -140,15 +134,14 @@ RSpec/RepeatedExampleGroupDescription:
     - 'spec/grape/util/inheritable_setting_spec.rb'
     - 'spec/grape/validations/validators/values_spec.rb'
 
-# Offense count: 5
+# Offense count: 4
 RSpec/StubbedMock:
   Exclude:
     - 'spec/grape/dsl/inside_route_spec.rb'
     - 'spec/grape/dsl/routing_spec.rb'
     - 'spec/grape/middleware/formatter_spec.rb'
-    - 'spec/grape/parser_spec.rb'
 
-# Offense count: 122
+# Offense count: 121
 RSpec/SubjectStub:
   Exclude:
     - 'spec/grape/api_spec.rb'
@@ -164,7 +157,6 @@ RSpec/SubjectStub:
     - 'spec/grape/middleware/formatter_spec.rb'
     - 'spec/grape/middleware/globals_spec.rb'
     - 'spec/grape/middleware/stack_spec.rb'
-    - 'spec/grape/parser_spec.rb'
 
 # Offense count: 23
 # Configuration parameters: IgnoreNameless, IgnoreSymbolicNames.

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@
 #### Fixes
 
 * [#2471](https://github.com/ruby-grape/grape/pull/2471): Fix absence of original_exception and/or backtrace even if passed in error! - [@numbata](https://github.com/numbata).
+* [#2478](https://github.com/ruby-grape/grape/pull/2478): Fix rescue_from with invalid response - [@ericproulx](https://github.com/ericproulx).
 * Your contribution here.
 
 ### 2.1.3 (2024-07-13)

diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb
@@ -125,7 +125,7 @@ def run_rescue_handler(handler, error, endpoint)
         elsif response.is_a?(Rack::Response)
           response
         else
-          run_rescue_handler(:default_rescue_handler, Grape::Exceptions::InvalidResponse.new, endpoint)
+          run_rescue_handler(method(:default_rescue_handler), Grape::Exceptions::InvalidResponse.new, endpoint)
         end
       end
 

diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
@@ -2193,14 +2193,14 @@ def foo
       expect(last_response.body).to eq('Formatter Error')
     end
 
-    it 'uses default_rescue_handler to handle invalid response from rescue_from' do
-      subject.rescue_from(:all) { 'error' }
-      subject.get('/') { raise }
-
-      expect_any_instance_of(Grape::Middleware::Error).to receive(:default_rescue_handler).and_call_original
-      get '/'
-      expect(last_response).to be_server_error
-      expect(last_response.body).to eql 'Invalid response'
+    context 'when rescue_from block returns an invalid response' do
+      it 'returns a formatted response' do
+        subject.rescue_from(:all) { 'error' }
+        subject.get('/') { raise }
+        get '/'
+        expect(last_response).to be_server_error
+        expect(last_response.body).to eql 'Invalid response'
+      end
     end
   end
 

diff --git a/spec/grape/middleware/base_spec.rb b/spec/grape/middleware/base_spec.rb
@@ -57,7 +57,7 @@
     context 'with patched warnings' do
       before do
         @warnings = warnings = []
-        allow_any_instance_of(described_class).to receive(:warn) { |m| warnings << m }
+        allow(subject).to receive(:warn) { |m| warnings << m }
         allow(subject).to receive(:after).and_raise(StandardError)
       end