From 05bff0b2c130547353e4d5ca699211cbe3ba1cd8 Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Tue, 4 Jul 2017 00:53:41 +0100 Subject: [PATCH 1/8] Bubbles up to the error_formatter the root exception when a custom parser is not able to parse a json input body. --- lib/grape/middleware/formatter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 1e21619b0..eadbe2cbf 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -110,7 +110,7 @@ def read_rack_input(body) rescue Grape::Exceptions::Base => e raise e rescue StandardError => e - throw :error, status: 400, message: e.message + throw :error, status: 400, message: e.message, backtrace: e end else env[Grape::Env::API_REQUEST_BODY] = body From 25b1dcd82e45de1dc853fb02bc2208e620d0baf3 Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Tue, 4 Jul 2017 01:23:36 +0100 Subject: [PATCH 2/8] update the changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cbb0ffa5..bd97427f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ #### Fixes -* Your contribution here. +* [#1652](https://github.com/ruby-grape/grape/pull/1652): Bubble up to the error_formatter the root exception - [@dcsg](https://github.com/dcsg). ### 1.0.0 (7/3/2017) From 93670da0aa5fc8db906a8af3895e05d776f6b24b Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Thu, 6 Jul 2017 00:50:08 +0100 Subject: [PATCH 3/8] Add exception to the `formatter.call` contract Add backtrace in missing `throw :error` Add tests --- lib/grape/error_formatter/json.rb | 3 ++- lib/grape/error_formatter/txt.rb | 3 ++- lib/grape/error_formatter/xml.rb | 3 ++- lib/grape/middleware/error.rb | 24 ++++++++++++++-------- lib/grape/middleware/formatter.rb | 4 ++-- spec/grape/api_spec.rb | 4 ++-- spec/grape/middleware/exception_spec.rb | 27 ++++++++++++++++++------- 7 files changed, 46 insertions(+), 22 deletions(-) diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index e11210495..9e012bc65 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -4,12 +4,13 @@ module Json extend Base class << self - def call(message, backtrace, options = {}, env = nil) + def call(message, backtrace, options = {}, env = nil, exception = '') result = wrap_message(present(message, env)) if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end + result = result.merge(exception: exception) if (options[:rescue_options] || {})[:exception] && !exception.empty? ::Grape::Json.dump(result) end diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index db8762f95..59a09d1a5 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -4,7 +4,7 @@ module Txt extend Base class << self - def call(message, backtrace, options = {}, env = nil) + def call(message, backtrace, options = {}, env = nil, exception = '') message = present(message, env) result = message.is_a?(Hash) ? ::Grape::Json.dump(message) : message @@ -12,6 +12,7 @@ def call(message, backtrace, options = {}, env = nil) result += "\r\n " result += backtrace.join("\r\n ") end + result += "\r\n #{exception}" if (options[:rescue_options] || {})[:exception] && !exception.empty? result end end diff --git a/lib/grape/error_formatter/xml.rb b/lib/grape/error_formatter/xml.rb index ea2eb3951..e29037ee3 100644 --- a/lib/grape/error_formatter/xml.rb +++ b/lib/grape/error_formatter/xml.rb @@ -4,13 +4,14 @@ module Xml extend Base class << self - def call(message, backtrace, options = {}, env = nil) + def call(message, backtrace, options = {}, env = nil, exception = '') message = present(message, env) result = message.is_a?(Hash) ? message : { message: message } if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end + result = result.merge(exception: exception) if (options[:rescue_options] || {})[:exception] && !exception.empty? result.respond_to?(:to_xml) ? result.to_xml(root: :error) : result.to_s end end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index dcf6e0144..4b6c3184a 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -14,7 +14,10 @@ def default_options rescue_all: false, # true to rescue all exceptions rescue_grape_exceptions: false, rescue_subclasses: true, # rescue subclasses of exceptions listed - rescue_options: { backtrace: false }, # true to display backtrace, true to let Grape handle Grape::Exceptions + rescue_options: { + backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions + exception: false, # true to display exception + }, rescue_handlers: {}, # rescue handler blocks base_only_rescue_handlers: {}, # rescue handler blocks rescuing only the base class all_rescue_handler: nil # rescue handler block to rescue from all exceptions @@ -77,13 +80,13 @@ def exec_handler(e, &handler) end end - def error!(message, status = options[:default_status], headers = {}, backtrace = []) + def error!(message, status = options[:default_status], headers = {}, backtrace = [], exception = '') headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type) - rack_response(format_message(message, backtrace), status, headers) + rack_response(format_message(message, backtrace, exception), status, headers) end def handle_error(e) - error_response(message: e.message, backtrace: e.backtrace) + error_response(message: e.message, backtrace: e.backtrace, exception: e.class.name) end # TODO: This method is deprecated. Refactor out. @@ -93,18 +96,23 @@ def error_response(error = {}) headers = { Grape::Http::Headers::CONTENT_TYPE => content_type } headers.merge!(error[:headers]) if error[:headers].is_a?(Hash) backtrace = error[:backtrace] || [] - rack_response(format_message(message, backtrace), status, headers) + exception = error.is_a?(Exception) ? error.class.name : error[:exception] || '' + rack_response(format_message(message, backtrace, exception), status, headers) end def rack_response(message, status = options[:default_status], headers = { Grape::Http::Headers::CONTENT_TYPE => content_type }) Rack::Response.new([message], status, headers).finish end - def format_message(message, backtrace) + def format_message(message, backtrace, exception) format = env[Grape::Env::API_FORMAT] || options[:format] formatter = Grape::ErrorFormatter.formatter_for(format, options) - throw :error, status: 406, message: "The requested format '#{format}' is not supported." unless formatter - formatter.call(message, backtrace, options, env) + throw :error, + status: 406, + message: "The requested format '#{format}' is not supported.", + backtrace: backtrace, + exception: exception unless formatter + formatter.call(message, backtrace, options, env, exception) end private diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index eadbe2cbf..2970f83dd 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -45,7 +45,7 @@ def build_formatted_response(status, headers, bodies) Rack::Response.new(bodymap, status, headers) end rescue Grape::Exceptions::InvalidFormatter => e - throw :error, status: 500, message: e.message + throw :error, status: 500, message: e.message, backtrace: e.backtrace, exception: e end def fetch_formatter(headers, options) @@ -110,7 +110,7 @@ def read_rack_input(body) rescue Grape::Exceptions::Base => e raise e rescue StandardError => e - throw :error, status: 400, message: e.message, backtrace: e + throw :error, status: 400, message: e.message, backtrace: e.backtrace, exception: e end else env[Grape::Env::API_REQUEST_BODY] = body diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 779893f8e..9cda20500 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1996,7 +1996,7 @@ class ChildError < ParentError; end before :each do module ApiSpec class CustomErrorFormatter - def self.call(message, _backtrace, _options, _env) + def self.call(message, _backtrace, _options, _env, _exception) "message: #{message} @backtrace" end end @@ -2018,7 +2018,7 @@ def self.call(message, _backtrace, _options, _env) before :each do module ApiSpec class CustomErrorFormatter - def self.call(message, _backtrace, _option, _env) + def self.call(message, _backtrace, _option, _env, _exception) "message: #{message} @backtrace" end end diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index 362cc3eba..fc933558c 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -158,13 +158,14 @@ def call(_env) it 'is possible to specify a custom formatter' do @app ||= Rack::Builder.app do use Spec::Support::EndpointFaker - use Grape::Middleware::Error, rescue_all: true, - format: :custom, - error_formatters: { - custom: lambda do |message, _backtrace, _options, _env| - { custom_formatter: message }.inspect - end - } + use Grape::Middleware::Error, + rescue_all: true, + format: :custom, + error_formatters: { + custom: lambda do |message, _backtrace, _options, _env, _exception| + { custom_formatter: message }.inspect + end + } run ExceptionSpec::ExceptionApp end get '/' @@ -192,5 +193,17 @@ def call(_env) expect(last_response.status).to eq(400) expect(last_response.body).to eq('failed validation') end + + context 'with rescue_options :backtrace and :exception set to true' do + it 'is possible to return the backtrace and exception in json format' do + @app ||= Rack::Builder.app do + use Spec::Support::EndpointFaker + use Grape::Middleware::Error, rescue_all: true, format: :json, rescue_options: { backtrace: true, exception: true } + run ExceptionSpec::ExceptionApp + end + get '/' + expect(last_response.body).to include('error', 'rain!', 'backtrace', 'exception', 'RuntimeError') + end + end end end From 2c90e6f081c3917c1648edf47baf8afc83a11e33 Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Thu, 6 Jul 2017 22:37:31 +0100 Subject: [PATCH 4/8] Refactor exception to original_exception. --- lib/grape/error_formatter/json.rb | 6 ++++-- lib/grape/error_formatter/txt.rb | 6 ++++-- lib/grape/error_formatter/xml.rb | 6 ++++-- lib/grape/middleware/error.rb | 20 ++++++++++---------- spec/grape/middleware/exception_spec.rb | 9 ++++++--- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index 9e012bc65..7525eda06 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -4,13 +4,15 @@ module Json extend Base class << self - def call(message, backtrace, options = {}, env = nil, exception = '') + def call(message, backtrace, options = {}, env = nil, original_exception = nil) result = wrap_message(present(message, env)) if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end - result = result.merge(exception: exception) if (options[:rescue_options] || {})[:exception] && !exception.empty? + if (options[:rescue_options] || {})[:original_exception] && original_exception + result = result.merge(original_exception: original_exception.inspect) + end ::Grape::Json.dump(result) end diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index 59a09d1a5..210115e26 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -4,7 +4,7 @@ module Txt extend Base class << self - def call(message, backtrace, options = {}, env = nil, exception = '') + def call(message, backtrace, options = {}, env = nil, original_exception = nil) message = present(message, env) result = message.is_a?(Hash) ? ::Grape::Json.dump(message) : message @@ -12,7 +12,9 @@ def call(message, backtrace, options = {}, env = nil, exception = '') result += "\r\n " result += backtrace.join("\r\n ") end - result += "\r\n #{exception}" if (options[:rescue_options] || {})[:exception] && !exception.empty? + if (options[:rescue_options] || {})[:original_exception] && original_exception + result += "\r\n #{original_exception.inspect}" + end result end end diff --git a/lib/grape/error_formatter/xml.rb b/lib/grape/error_formatter/xml.rb index e29037ee3..975644c26 100644 --- a/lib/grape/error_formatter/xml.rb +++ b/lib/grape/error_formatter/xml.rb @@ -4,14 +4,16 @@ module Xml extend Base class << self - def call(message, backtrace, options = {}, env = nil, exception = '') + def call(message, backtrace, options = {}, env = nil, original_exception = nil) message = present(message, env) result = message.is_a?(Hash) ? message : { message: message } if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end - result = result.merge(exception: exception) if (options[:rescue_options] || {})[:exception] && !exception.empty? + if (options[:rescue_options] || {})[:original_exception] && original_exception + result = result.merge(original_exception: original_exception.inspect) + end result.respond_to?(:to_xml) ? result.to_xml(root: :error) : result.to_s end end diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 4b6c3184a..dee065dd3 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -16,7 +16,7 @@ def default_options rescue_subclasses: true, # rescue subclasses of exceptions listed rescue_options: { backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions - exception: false, # true to display exception + original_exception: false, # true to display exception }, rescue_handlers: {}, # rescue handler blocks base_only_rescue_handlers: {}, # rescue handler blocks rescuing only the base class @@ -80,13 +80,13 @@ def exec_handler(e, &handler) end end - def error!(message, status = options[:default_status], headers = {}, backtrace = [], exception = '') + def error!(message, status = options[:default_status], headers = {}, backtrace = [], original_exception = '') headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type) - rack_response(format_message(message, backtrace, exception), status, headers) + rack_response(format_message(message, backtrace, original_exception), status, headers) end def handle_error(e) - error_response(message: e.message, backtrace: e.backtrace, exception: e.class.name) + error_response(message: e.message, backtrace: e.backtrace, original_exception: e) end # TODO: This method is deprecated. Refactor out. @@ -95,24 +95,24 @@ def error_response(error = {}) message = error[:message] || options[:default_message] headers = { Grape::Http::Headers::CONTENT_TYPE => content_type } headers.merge!(error[:headers]) if error[:headers].is_a?(Hash) - backtrace = error[:backtrace] || [] - exception = error.is_a?(Exception) ? error.class.name : error[:exception] || '' - rack_response(format_message(message, backtrace, exception), status, headers) + backtrace = error[:backtrace] || error[:original_exception] && error[:original_exception].backtrace || [] + original_exception = error.is_a?(Exception) ? error : error[:original_exception] || nil + rack_response(format_message(message, backtrace, original_exception), status, headers) end def rack_response(message, status = options[:default_status], headers = { Grape::Http::Headers::CONTENT_TYPE => content_type }) Rack::Response.new([message], status, headers).finish end - def format_message(message, backtrace, exception) + def format_message(message, backtrace, original_exception = nil) format = env[Grape::Env::API_FORMAT] || options[:format] formatter = Grape::ErrorFormatter.formatter_for(format, options) throw :error, status: 406, message: "The requested format '#{format}' is not supported.", backtrace: backtrace, - exception: exception unless formatter - formatter.call(message, backtrace, options, env, exception) + original_exception: original_exception unless formatter + formatter.call(message, backtrace, options, env, original_exception) end private diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index fc933558c..5ab8eacc2 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -162,7 +162,7 @@ def call(_env) rescue_all: true, format: :custom, error_formatters: { - custom: lambda do |message, _backtrace, _options, _env, _exception| + custom: lambda do |message, _backtrace, _options, _env, _original_exception| { custom_formatter: message }.inspect end } @@ -198,11 +198,14 @@ def call(_env) it 'is possible to return the backtrace and exception in json format' do @app ||= Rack::Builder.app do use Spec::Support::EndpointFaker - use Grape::Middleware::Error, rescue_all: true, format: :json, rescue_options: { backtrace: true, exception: true } + use Grape::Middleware::Error, + rescue_all: true, + format: :json, + rescue_options: { backtrace: true, original_exception: true } run ExceptionSpec::ExceptionApp end get '/' - expect(last_response.body).to include('error', 'rain!', 'backtrace', 'exception', 'RuntimeError') + expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original_exception', 'RuntimeError') end end end From 35fcf6f5a7448bb417b47412755e46a90bb446ea Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Fri, 7 Jul 2017 16:42:35 +0100 Subject: [PATCH 5/8] Code review changes --- CHANGELOG.md | 3 ++- lib/grape/error_formatter/json.rb | 5 +++-- lib/grape/error_formatter/txt.rb | 8 ++++--- lib/grape/error_formatter/xml.rb | 5 +++-- lib/grape/middleware/error.rb | 2 +- lib/grape/middleware/formatter.rb | 4 ++-- spec/grape/api_spec.rb | 4 ++-- spec/grape/middleware/exception_spec.rb | 28 ++++++++++++++++++++++++- 8 files changed, 45 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd97427f7..4af97c7fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,10 +3,11 @@ #### Features * Your contribution here. +* [#1652](https://github.com/ruby-grape/grape/pull/1652): Add the original exception to the error_formatter the original exception - [@dcsg](https://github.com/dcsg). #### Fixes -* [#1652](https://github.com/ruby-grape/grape/pull/1652): Bubble up to the error_formatter the root exception - [@dcsg](https://github.com/dcsg). +* [#1652](https://github.com/ruby-grape/grape/pull/1652): Fix missing backtrace that was not being bubble up to the error_formatter - [@dcsg](https://github.com/dcsg). ### 1.0.0 (7/3/2017) diff --git a/lib/grape/error_formatter/json.rb b/lib/grape/error_formatter/json.rb index 7525eda06..d3ebff765 100644 --- a/lib/grape/error_formatter/json.rb +++ b/lib/grape/error_formatter/json.rb @@ -7,10 +7,11 @@ class << self def call(message, backtrace, options = {}, env = nil, original_exception = nil) result = wrap_message(present(message, env)) - if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? + rescue_options = options[:rescue_options] || {} + if rescue_options[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end - if (options[:rescue_options] || {})[:original_exception] && original_exception + if rescue_options[:original_exception] && original_exception result = result.merge(original_exception: original_exception.inspect) end ::Grape::Json.dump(result) diff --git a/lib/grape/error_formatter/txt.rb b/lib/grape/error_formatter/txt.rb index 210115e26..b58e03152 100644 --- a/lib/grape/error_formatter/txt.rb +++ b/lib/grape/error_formatter/txt.rb @@ -8,11 +8,13 @@ def call(message, backtrace, options = {}, env = nil, original_exception = nil) message = present(message, env) result = message.is_a?(Hash) ? ::Grape::Json.dump(message) : message - if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? - result += "\r\n " + rescue_options = options[:rescue_options] || {} + if rescue_options[:backtrace] && backtrace && !backtrace.empty? + result += "\r\n backtrace:" result += backtrace.join("\r\n ") end - if (options[:rescue_options] || {})[:original_exception] && original_exception + if rescue_options[:original_exception] && original_exception + result += "\r\n original exception:" result += "\r\n #{original_exception.inspect}" end result diff --git a/lib/grape/error_formatter/xml.rb b/lib/grape/error_formatter/xml.rb index 975644c26..73ebf09c8 100644 --- a/lib/grape/error_formatter/xml.rb +++ b/lib/grape/error_formatter/xml.rb @@ -8,10 +8,11 @@ def call(message, backtrace, options = {}, env = nil, original_exception = nil) message = present(message, env) result = message.is_a?(Hash) ? message : { message: message } - if (options[:rescue_options] || {})[:backtrace] && backtrace && !backtrace.empty? + rescue_options = options[:rescue_options] || {} + if rescue_options[:backtrace] && backtrace && !backtrace.empty? result = result.merge(backtrace: backtrace) end - if (options[:rescue_options] || {})[:original_exception] && original_exception + if rescue_options[:original_exception] && original_exception result = result.merge(original_exception: original_exception.inspect) end result.respond_to?(:to_xml) ? result.to_xml(root: :error) : result.to_s diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index dee065dd3..3c00786bf 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -80,7 +80,7 @@ def exec_handler(e, &handler) end end - def error!(message, status = options[:default_status], headers = {}, backtrace = [], original_exception = '') + def error!(message, status = options[:default_status], headers = {}, backtrace = [], original_exception = nil) headers = headers.reverse_merge(Grape::Http::Headers::CONTENT_TYPE => content_type) rack_response(format_message(message, backtrace, original_exception), status, headers) end diff --git a/lib/grape/middleware/formatter.rb b/lib/grape/middleware/formatter.rb index 2970f83dd..5f8caa50f 100644 --- a/lib/grape/middleware/formatter.rb +++ b/lib/grape/middleware/formatter.rb @@ -45,7 +45,7 @@ def build_formatted_response(status, headers, bodies) Rack::Response.new(bodymap, status, headers) end rescue Grape::Exceptions::InvalidFormatter => e - throw :error, status: 500, message: e.message, backtrace: e.backtrace, exception: e + throw :error, status: 500, message: e.message, backtrace: e.backtrace, original_exception: e end def fetch_formatter(headers, options) @@ -110,7 +110,7 @@ def read_rack_input(body) rescue Grape::Exceptions::Base => e raise e rescue StandardError => e - throw :error, status: 400, message: e.message, backtrace: e.backtrace, exception: e + throw :error, status: 400, message: e.message, backtrace: e.backtrace, original_exception: e end else env[Grape::Env::API_REQUEST_BODY] = body diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 9cda20500..9b5db5a47 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1996,7 +1996,7 @@ class ChildError < ParentError; end before :each do module ApiSpec class CustomErrorFormatter - def self.call(message, _backtrace, _options, _env, _exception) + def self.call(message, _backtrace, _options, _env, _original_exception) "message: #{message} @backtrace" end end @@ -2018,7 +2018,7 @@ def self.call(message, _backtrace, _options, _env, _exception) before :each do module ApiSpec class CustomErrorFormatter - def self.call(message, _backtrace, _option, _env, _exception) + def self.call(message, _backtrace, _option, _env, _original_exception) "message: #{message} @backtrace" end end diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index 5ab8eacc2..78494a031 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -195,7 +195,7 @@ def call(_env) end context 'with rescue_options :backtrace and :exception set to true' do - it 'is possible to return the backtrace and exception in json format' do + it 'is possible to return the backtrace and the original exception in json format' do @app ||= Rack::Builder.app do use Spec::Support::EndpointFaker use Grape::Middleware::Error, @@ -207,6 +207,32 @@ def call(_env) get '/' expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original_exception', 'RuntimeError') end + + it 'is possible to return the backtrace and the original exception in xml format' do + @app ||= Rack::Builder.app do + use Spec::Support::EndpointFaker + use Grape::Middleware::Error, + rescue_all: true, + format: :xml, + rescue_options: { backtrace: true, original_exception: true } + run ExceptionSpec::ExceptionApp + end + get '/' + expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original-exception', 'RuntimeError') + end + + it 'is possible to return the backtrace and the original exception in txt format' do + @app ||= Rack::Builder.app do + use Spec::Support::EndpointFaker + use Grape::Middleware::Error, + rescue_all: true, + format: :txt, + rescue_options: { backtrace: true, original_exception: true } + run ExceptionSpec::ExceptionApp + end + get '/' + expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original exception', 'RuntimeError') + end end end end From d9f5e5137b9b3c3b22e4ee5bc9ebd9e8b12ce165 Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Tue, 11 Jul 2017 00:59:12 +0100 Subject: [PATCH 6/8] add missing test --- spec/grape/middleware/formatter_spec.rb | 29 ++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index c90f6c1f9..87484b4c9 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -1,11 +1,12 @@ require 'spec_helper' describe Grape::Middleware::Formatter do - subject { Grape::Middleware::Formatter.new(app) } + subject { Grape::Middleware::Formatter.new(app, options) } before { allow(subject).to receive(:dup).and_return(subject) } let(:body) { { 'foo' => 'bar' } } let(:app) { ->(_env) { [200, {}, [body]] } } + let(:options) { {} } context 'serialization' do let(:body) { { 'abc' => 'def' } } @@ -324,4 +325,30 @@ def self.call(_, _) expect(bodies.body.first).to eq({ message: 'invalid' }.to_json) end end + + context 'custom parser raises exception and rescue options are enabled for backtrace and original_exception' do + let(:options) { + { + rescue_options: { backtrace: true, original_exception: true }, + parsers: { json: ->(_object, _env) { raise StandardError, 'fail' } } + } + } + + it 'adds the backtrace and original_exception to the error output' do + io = StringIO.new('{invalid}') + error = catch(:error) { + subject.call( + 'PATH_INFO' => '/info', + 'REQUEST_METHOD' => 'POST', + 'CONTENT_TYPE' => 'application/json', + 'rack.input' => io, + 'CONTENT_LENGTH' => io.length + ) + } + + expect(error[:message]).to eq 'fail' + expect(error[:backtrace].size).to be >= 1 + expect(error[:original_exception].class).to eq StandardError + end + end end From 04762315adcb2bc24c749c41281d468c5966fe67 Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Tue, 11 Jul 2017 01:17:25 +0100 Subject: [PATCH 7/8] rubocop fixes --- .rubocop_todo.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 41b50f3ee..04940c0fe 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2017-06-17 22:15:11 +0300 using RuboCop version 0.47.0. +# on 2017-07-11 00:59:31 +0100 using RuboCop version 0.47.0. # 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 @@ -44,6 +44,11 @@ Metrics/ModuleLength: Metrics/PerceivedComplexity: Max: 14 +# Offense count: 2 +Style/BlockDelimiters: + Exclude: + - 'spec/grape/middleware/formatter_spec.rb' + # Offense count: 2 Style/IdenticalConditionalBranches: Exclude: From 469b05d2b236cdaaafad902c372cb06b4f374b3a Mon Sep 17 00:00:00 2001 From: Daniel Gomes Date: Tue, 11 Jul 2017 01:40:27 +0100 Subject: [PATCH 8/8] trying to fix random test failures --- spec/grape/middleware/formatter_spec.rb | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/spec/grape/middleware/formatter_spec.rb b/spec/grape/middleware/formatter_spec.rb index 87484b4c9..aeab1c4e1 100644 --- a/spec/grape/middleware/formatter_spec.rb +++ b/spec/grape/middleware/formatter_spec.rb @@ -1,12 +1,11 @@ require 'spec_helper' describe Grape::Middleware::Formatter do - subject { Grape::Middleware::Formatter.new(app, options) } + subject { Grape::Middleware::Formatter.new(app) } before { allow(subject).to receive(:dup).and_return(subject) } let(:body) { { 'foo' => 'bar' } } let(:app) { ->(_env) { [200, {}, [body]] } } - let(:options) { {} } context 'serialization' do let(:body) { { 'abc' => 'def' } } @@ -327,14 +326,12 @@ def self.call(_, _) end context 'custom parser raises exception and rescue options are enabled for backtrace and original_exception' do - let(:options) { - { + it 'adds the backtrace and original_exception to the error output' do + subject = Grape::Middleware::Formatter.new( + app, rescue_options: { backtrace: true, original_exception: true }, parsers: { json: ->(_object, _env) { raise StandardError, 'fail' } } - } - } - - it 'adds the backtrace and original_exception to the error output' do + ) io = StringIO.new('{invalid}') error = catch(:error) { subject.call(