From dd492edf4cd6716f20f12e0dbd305ad5f15e9642 Mon Sep 17 00:00:00 2001 From: Alejandro Wainzinger Date: Tue, 31 Dec 2013 03:54:09 +0900 Subject: [PATCH 1/4] Add :rescue_children option to rescue_from error middleware, to rescue children of exceptions as well as the parent given. --- lib/grape/api.rb | 10 +++---- lib/grape/endpoint.rb | 4 +-- lib/grape/middleware/error.rb | 15 ++++++++--- spec/grape/api_spec.rb | 51 +++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 12 deletions(-) diff --git a/lib/grape/api.rb b/lib/grape/api.rb index c249c5483..24adb49ed 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -199,6 +199,7 @@ def default_error_status(new_status = nil) # @param [Block] block Execution block to handle the given exception. # @param [Hash] options Options for the rescue usage. # @option options [Boolean] :backtrace Include a backtrace in the rescue response. + # @option options [Boolean] :rescue_children Also rescue children of exception classes # @param [Proc] handler Execution proc to handle the given exception as an # alternative to passing a block def rescue_from(*args, &block) @@ -211,18 +212,13 @@ def rescue_from(*args, &block) options = args.last.is_a?(Hash) ? args.pop : {} handler ||= proc { options[:with] } if options.has_key?(:with) - if handler - args.each do |arg| - imbue :rescue_handlers, arg => handler - end - end + handler_type = !!options[:rescue_children] ? :rescue_children_handlers : :rescue_handlers + imbue handler_type, Hash[args.map { |arg| [arg, handler] }] imbue(:rescue_options, options) if args.include?(:all) set(:rescue_all, true) - else - imbue(:rescued_errors, args) end end diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index c8b8d2b0b..3c3a3e46c 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -411,11 +411,11 @@ def build_middleware format: settings[:format], default_status: settings[:default_error_status] || 500, rescue_all: settings[:rescue_all], - rescued_errors: aggregate_setting(:rescued_errors), default_error_formatter: settings[:default_error_formatter], error_formatters: settings[:error_formatters], rescue_options: settings[:rescue_options], - rescue_handlers: merged_setting(:rescue_handlers) + rescue_handlers: merged_setting(:rescue_handlers), + rescue_children_handlers: merged_setting(:rescue_children_handlers) aggregate_setting(:middleware).each do |m| m = m.dup diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index d14539113..9eec5ced2 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -11,9 +11,10 @@ def default_options formatters: {}, error_formatters: {}, rescue_all: false, # true to rescue all exceptions + rescue_children: false, # rescue children of exceptions listed rescue_options: { backtrace: false }, # true to display backtrace rescue_handlers: {}, # rescue handler blocks - rescued_errors: [] + rescue_children_handlers: {} # rescue handler blocks } end @@ -30,15 +31,23 @@ def call!(env) handler = lambda { |arg| error_response(arg) } else raise unless is_rescuable - handler = options[:rescue_handlers][e.class] || options[:rescue_handlers][:all] + handler = find_handler(e.class) end handler.nil? ? handle_error(e) : exec_handler(e, &handler) end end + def find_handler(klass) + handler = options[:rescue_handlers][klass] + unless handler + handler = options[:rescue_children_handlers].find(-> {[]}) { |error, handler| klass <= error }[1] + end + handler || options[:rescue_handlers][:all] + end + def rescuable?(klass) - options[:rescue_all] || (options[:rescued_errors] || []).include?(klass) + options[:rescue_all] || (options[:rescue_handlers] || []).include?(klass) || (options[:rescue_children_handlers] || []).any? { |error, handler| klass <= error } end def exec_handler(e, &handler) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ef9178635..43f5bbc65 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1286,6 +1286,57 @@ def rescue_arg_error end end + describe '.rescue_from klass, children: method' do + it 'rescues error as well as child errors with rescue_children option set' do + class CommunicationError < RuntimeError; end + subject.rescue_from RuntimeError, rescue_children: true do |e| + rack_response("rescued from #{e.class.name}", 500) + end + subject.get '/caught_child' do + raise CommunicationError + end + subject.get '/caught_parent' do + raise RuntimeError + end + subject.get '/uncaught_parent' do + raise StandardError + end + + get '/caught_child' + last_response.status.should eql 500 + get '/caught_parent' + last_response.status.should eql 500 + lambda { get '/uncaught_parent' }.should raise_error(StandardError) + end + + it 'does not rescue child errors for other rescue_from instances' do + class CommunicationError < RuntimeError; end + class BadCommunicationError < CommunicationError; end + + class ProcessingError < RuntimeError; end + class BadProcessingError < ProcessingError; end + + subject.rescue_from CommunicationError, rescue_children: true do |e| + rack_response("rescued from #{e.class.name}", 500) + end + + subject.rescue_from ProcessingError do |e| + rack_response("rescued from #{e.class.name}", 500) + end + + subject.get '/caught_child' do + raise BadCommunicationError + end + subject.get '/uncaught_child' do + raise BadProcessingError + end + + get '/caught_child' + last_response.status.should eql 500 + lambda { get '/uncaught_child' }.should raise_error(BadProcessingError) + end + end + describe '.error_format' do it 'rescues all errors and return :txt' do subject.rescue_from :all From 1f137e8a75841845414948f921166f9a4d14a23c Mon Sep 17 00:00:00 2001 From: Alejandro Wainzinger Date: Tue, 31 Dec 2013 04:46:20 +0900 Subject: [PATCH 2/4] Fix rubocop errors. --- lib/grape/api.rb | 4 +--- lib/grape/middleware/error.rb | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 24adb49ed..0742320d5 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -217,9 +217,7 @@ def rescue_from(*args, &block) imbue(:rescue_options, options) - if args.include?(:all) - set(:rescue_all, true) - end + set(:rescue_all, true) if args.include?(:all) end # Allows you to specify a default representation entity for a diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 9eec5ced2..8731c8f40 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -41,7 +41,7 @@ def call!(env) def find_handler(klass) handler = options[:rescue_handlers][klass] unless handler - handler = options[:rescue_children_handlers].find(-> {[]}) { |error, handler| klass <= error }[1] + handler = options[:rescue_children_handlers].find(-> { [] }) { |error, _| klass <= error }[1] end handler || options[:rescue_handlers][:all] end From 94518e9f7b01dbc051add0225c272175b89579de Mon Sep 17 00:00:00 2001 From: Alejandro Wainzinger Date: Wed, 1 Jan 2014 18:11:46 +0900 Subject: [PATCH 3/4] Change rescue_from to default to rescuing subclasses of the given exception classes. Rename children -> subclasses. --- lib/grape/api.rb | 4 ++-- lib/grape/endpoint.rb | 2 +- lib/grape/middleware/error.rb | 14 ++++++-------- spec/grape/api_spec.rb | 25 ++++++++++++++++++------- 4 files changed, 27 insertions(+), 18 deletions(-) diff --git a/lib/grape/api.rb b/lib/grape/api.rb index 0742320d5..f1dc82990 100644 --- a/lib/grape/api.rb +++ b/lib/grape/api.rb @@ -199,7 +199,7 @@ def default_error_status(new_status = nil) # @param [Block] block Execution block to handle the given exception. # @param [Hash] options Options for the rescue usage. # @option options [Boolean] :backtrace Include a backtrace in the rescue response. - # @option options [Boolean] :rescue_children Also rescue children of exception classes + # @option options [Boolean] :rescue_subclasses Also rescue subclasses of exception classes # @param [Proc] handler Execution proc to handle the given exception as an # alternative to passing a block def rescue_from(*args, &block) @@ -212,7 +212,7 @@ def rescue_from(*args, &block) options = args.last.is_a?(Hash) ? args.pop : {} handler ||= proc { options[:with] } if options.has_key?(:with) - handler_type = !!options[:rescue_children] ? :rescue_children_handlers : :rescue_handlers + handler_type = !!options[:rescue_subclasses] ? :rescue_handlers : :base_only_rescue_handlers imbue handler_type, Hash[args.map { |arg| [arg, handler] }] imbue(:rescue_options, options) diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 3c3a3e46c..ccbde50fb 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -415,7 +415,7 @@ def build_middleware error_formatters: settings[:error_formatters], rescue_options: settings[:rescue_options], rescue_handlers: merged_setting(:rescue_handlers), - rescue_children_handlers: merged_setting(:rescue_children_handlers) + base_only_rescue_handlers: merged_setting(:base_only_rescue_handlers) aggregate_setting(:middleware).each do |m| m = m.dup diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 8731c8f40..d824aea42 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -11,10 +11,10 @@ def default_options formatters: {}, error_formatters: {}, rescue_all: false, # true to rescue all exceptions - rescue_children: false, # rescue children of exceptions listed + rescue_subclasses: true, # rescue subclasses of exceptions listed rescue_options: { backtrace: false }, # true to display backtrace rescue_handlers: {}, # rescue handler blocks - rescue_children_handlers: {} # rescue handler blocks + base_only_rescue_handlers: {} # rescue handler blocks rescuing only the base class } end @@ -39,15 +39,13 @@ def call!(env) end def find_handler(klass) - handler = options[:rescue_handlers][klass] - unless handler - handler = options[:rescue_children_handlers].find(-> { [] }) { |error, _| klass <= error }[1] - end - handler || options[:rescue_handlers][:all] + handler = options[:rescue_handlers].find(-> { [] }) { |error, _| klass <= error }[1] + handler = options[:base_only_rescue_handlers][klass] || options[:base_only_rescue_handlers][:all] unless handler + handler end def rescuable?(klass) - options[:rescue_all] || (options[:rescue_handlers] || []).include?(klass) || (options[:rescue_children_handlers] || []).any? { |error, handler| klass <= error } + options[:rescue_all] || (options[:rescue_handlers] || []).any? { |error, handler| klass <= error } || (options[:base_only_rescue_handlers] || []).include?(klass) end def exec_handler(e, &handler) diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 43f5bbc65..f97a07dbe 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -1236,7 +1236,7 @@ class DatabaseError < RuntimeError; end last_response.body.should == 'rescued from DatabaseError' end it 'does not rescue a different error' do - class CommunicationError < RuntimeError; end + class CommunicationError < StandardError; end subject.rescue_from RuntimeError do |e| rack_response("rescued from #{e.class.name}", 500) end @@ -1288,12 +1288,12 @@ def rescue_arg_error describe '.rescue_from klass, children: method' do it 'rescues error as well as child errors with rescue_children option set' do - class CommunicationError < RuntimeError; end - subject.rescue_from RuntimeError, rescue_children: true do |e| + class CommunicationsError < RuntimeError; end + subject.rescue_from RuntimeError, rescue_subclasses: true do |e| rack_response("rescued from #{e.class.name}", 500) end subject.get '/caught_child' do - raise CommunicationError + raise CommunicationsError end subject.get '/caught_parent' do raise RuntimeError @@ -1310,17 +1310,17 @@ class CommunicationError < RuntimeError; end end it 'does not rescue child errors for other rescue_from instances' do - class CommunicationError < RuntimeError; end + class CommunicationsError < RuntimeError; end class BadCommunicationError < CommunicationError; end class ProcessingError < RuntimeError; end class BadProcessingError < ProcessingError; end - subject.rescue_from CommunicationError, rescue_children: true do |e| + subject.rescue_from CommunicationError, rescue_subclasses: true do |e| rack_response("rescued from #{e.class.name}", 500) end - subject.rescue_from ProcessingError do |e| + subject.rescue_from ProcessingError, rescue_subclasses: false do |e| rack_response("rescued from #{e.class.name}", 500) end @@ -1335,6 +1335,17 @@ class BadProcessingError < ProcessingError; end last_response.status.should eql 500 lambda { get '/uncaught_child' }.should raise_error(BadProcessingError) end + + it 'does not rescue child errors if rescue_subclasses is false' do + class CommunicationsError < RuntimeError; end + subject.rescue_from RuntimeError, rescue_subclasses: false do |e| + rack_response("rescued from #{e.class.name}", 500) + end + subject.get '/uncaught' do + raise CommunicationError + end + lambda { get '/uncaught' }.should raise_error(CommunicationError) + end end describe '.error_format' do From 5aad3403331ebdf45d9fbe501baac3423aa8328a Mon Sep 17 00:00:00 2001 From: Alejandro Wainzinger Date: Wed, 1 Jan 2014 18:51:24 +0900 Subject: [PATCH 4/4] Update CHANGELOG and README with rescue_from subclasses feature in #544. --- CHANGELOG.md | 1 + README.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f135470c..649edb44f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Next Release * [#527](https://github.com/intridea/grape/pull/527): `before_validation` now a distinct callback (supersedes [#523](https://github.com/intridea/grape/pull/523)) - [@myitcv](https://github.com/myitcv). * [#531](https://github.com/intridea/grape/pull/531): Helpers are now available to auth middleware, executing in the context of the endpoint - [@joelvh](https://github.com/joelvh). * [#540](https://github.com/intridea/grape/pull/540): Ruby 2.1.0 is now supported - [@salimane](https://github.com/salimane). +* [#544](https://github.com/intridea/grape/pull/544): `rescue_from` now handles subclasses of exceptions by default - [@xevix](https://github.com/xevix). * Your contribution here. #### Fixes diff --git a/README.md b/README.md index f3b0c2938..c624390ac 100644 --- a/README.md +++ b/README.md @@ -792,6 +792,41 @@ class Twitter::API < Grape::API end ``` +By default, `rescue_from` will rescue the exceptions listed and all their subclasses. + +Assume you have the following exception classes defined. + +```ruby +module APIErrors + class ParentError < StandardError; end + class ChildError < ParentError; end +end +``` + +Then the following `rescue_from` clause will rescue exceptions of type `APIErrors::ParentError` and its subclasses (in this case `APIErrors::ChildError`). + +```ruby +rescue_from APIErrors::ParentError do |e| + Rack::Response.new({ + error: "#{e.class} error", + message: e.message + }.to_json, e.status) +end +``` + +To only rescue the base exception class, set `rescue_subclasses: false`. +The code below will rescue exceptions of type `RuntimeError` but _not_ its subclasses. + +```ruby +rescue_from RuntimeError, rescue_subclasses: false do |e| + Rack::Response.new( + status: e.status, + message: e.message, + errors: e.errors + }.to_json, e.status) +end +``` + #### Rails 3.x When mounted inside containers, such as Rails 3.x, errors like "404 Not Found" or