From 838c75e7a302ea6ca48b681ac08f665cc4e07891 Mon Sep 17 00:00:00 2001 From: Eric Proulx Date: Tue, 23 Jul 2024 22:53:09 +0200 Subject: [PATCH] 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 --------- Co-authored-by: Manuel Jacob --- .rubocop_todo.yml | 14 +++----------- CHANGELOG.md | 1 + lib/grape/middleware/error.rb | 2 +- spec/grape/api_spec.rb | 16 ++++++++-------- spec/grape/middleware/base_spec.rb | 2 +- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index f4482aff41..10cc2c7ed8 100644 --- 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 index 54284527e5..f6d6812439 100644 --- 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 index b798a19210..6e5304607b 100644 --- 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 index 3ac2fe7454..d12f29d76c 100644 --- 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 index 608c5012ef..a3acc39d5b 100644 --- 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