Bubble up to the error_formatter the original exception and the backtrace (#1652)

Author: dcsg

.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:

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
 
-* Your contribution here.
+* [#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)
 

lib/grape/error_formatter/json.rb

@@ -4,12 +4,16 @@ module Json
       extend Base
 
       class << self
-        def call(message, backtrace, options = {}, env = nil)
+        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 rescue_options[:original_exception] && original_exception
+            result = result.merge(original_exception: original_exception.inspect)
+          end
           ::Grape::Json.dump(result)
         end
 

lib/grape/error_formatter/txt.rb

@@ -4,14 +4,19 @@ module Txt
       extend Base
 
       class << self
-        def call(message, backtrace, options = {}, env = nil)
+        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 rescue_options[:original_exception] && original_exception
+            result += "\r\n original exception:"
+            result += "\r\n #{original_exception.inspect}"
+          end
           result
         end
       end

lib/grape/error_formatter/xml.rb

@@ -4,13 +4,17 @@ module Xml
       extend Base
 
       class << self
-        def call(message, backtrace, options = {}, env = nil)
+        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 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

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
+            original_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 = [], original_exception = nil)
         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, original_exception), status, headers)
       end
 
       def handle_error(e)
-        error_response(message: e.message, backtrace: e.backtrace)
+        error_response(message: e.message, backtrace: e.backtrace, original_exception: e)
       end
 
       # TODO: This method is deprecated. Refactor out.
@@ -92,19 +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] || []
-        rack_response(format_message(message, backtrace), 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)
+      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." unless formatter
-        formatter.call(message, backtrace, options, env)
+        throw :error,
+              status: 406,
+              message: "The requested format '#{format}' is not supported.",
+              backtrace: backtrace,
+              original_exception: original_exception unless formatter
+        formatter.call(message, backtrace, options, env, original_exception)
       end
 
       private

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, 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
+            throw :error, status: 400, message: e.message, backtrace: e.backtrace, original_exception: e
           end
         else
           env[Grape::Env::API_REQUEST_BODY] = body

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, _original_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, _original_exception)
                 "message: #{message} @backtrace"
               end
             end

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, _original_exception|
+                { custom_formatter: message }.inspect
+              end
+            }
         run ExceptionSpec::ExceptionApp
       end
       get '/'
@@ -192,5 +193,46 @@ 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 the original 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, 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 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

spec/grape/middleware/formatter_spec.rb

@@ -324,4 +324,28 @@ 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
+    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' } }
+      )
+      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