From bbb20ca8ac848932e3bdb4faa6a670bef9cbdcb5 Mon Sep 17 00:00:00 2001 From: Darren Chang Date: Tue, 7 Aug 2018 14:13:41 +0800 Subject: [PATCH] Use default_rescue_handler if reponse is invalid --- CHANGELOG.md | 2 +- lib/grape/middleware/error.rb | 2 +- spec/grape/api_spec.rb | 36 ++++++++++++++++++++--------------- 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c979a78ec..93ce601275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ #### Fixes * Your contribution here. +* [#1776](https://github.com/ruby-grape/grape/pull/1776): Validates response processed by exception handler - [@darren987469](https://github.com/darren987469). ### 1.1.0 (8/4/2018) @@ -21,7 +22,6 @@ * [#1758](https://github.com/ruby-grape/grape/pull/1758): Fix expanding load_path in gemspec - [@2maz](https://github.com/2maz). * [#1765](https://github.com/ruby-grape/grape/pull/1765): Use 415 when request body is of an unsupported media type - [@jdmurphy](https://github.com/jdmurphy). * [#1771](https://github.com/ruby-grape/grape/pull/1771): Fix param aliases with 'given' blocks - [@jereynolds](https://github.com/jereynolds). -* [#1776](https://github.com/ruby-grape/grape/pull/1776): Validates response processed by exception handler - [@darren987469](https://github.com/darren987469). ### 1.0.3 (4/23/2018) diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index d681f4a962..469c350d84 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -128,7 +128,7 @@ def run_rescue_handler(handler, error) end response = handler.arity.zero? ? instance_exec(&handler) : instance_exec(error, &handler) - valid_response?(response) ? response : error!('Internal Server Error(Invalid Response)') + valid_response?(response) ? response : run_rescue_handler(:default_rescue_handler, error) end def valid_response?(response) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 5e0edeaa87..d1fae42654 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1724,24 +1724,30 @@ class CustomError < Grape::Exceptions::Base; end expect(last_response.body).to eq('Formatter Error') end - it 'validates response processed by exception handler' do - subject.rescue_from ArgumentError do - error!('rain!') - nil # invalid response caused by return nil - end - subject.rescue_from :all do - error!('rain!') + context 'validates response processed by exception handler' do + it 'calls default_rescue_handler when response is invalid' do + subject.rescue_from :all do + error!('Internal Server Error') + nil # invalid response caused by return nil + end + subject.get('/invalid_response') { raise 'rain!' } + + expect_any_instance_of(Grape::Middleware::Error).to receive(:default_rescue_handler).and_call_original + get '/invalid_response' + expect(last_response.status).to eql 500 + expect(last_response.body).to eq('rain!') end - subject.get('/invalid_response') { raise ArgumentError } - subject.get('/valid_response') { raise 'rain!' } - get '/invalid_response' - expect(last_response.status).to eql 500 - expect(last_response.body).to eq('Internal Server Error(Invalid Response)') + it 'calls custom handler when response is valid' do + subject.rescue_from :all do + error!('Internal Server Error') + end + subject.get('/valid_response') { raise 'rain!' } - get '/valid_response' - expect(last_response.status).to eql 500 - expect(last_response.body).to eq('rain!') + get '/valid_response' + expect(last_response.status).to eql 500 + expect(last_response.body).to eq('Internal Server Error') + end end end