aboutsummaryrefslogtreecommitdiffstats
path: root/activesupport
diff options
context:
space:
mode:
authorAndrew White <andyw@pixeltrix.co.uk>2013-07-11 08:56:47 +0100
committerAndrew White <andyw@pixeltrix.co.uk>2013-07-11 09:05:32 +0100
commit07a4c76a07641840892463eb934400abdf694927 (patch)
tree5b8ac35d3fb08f76e87274aedb3a3d637ce3b347 /activesupport
parente7e81b4580f46c3d5229fdef3f0749802a9206da (diff)
downloadrails-07a4c76a07641840892463eb934400abdf694927.tar.gz
rails-07a4c76a07641840892463eb934400abdf694927.tar.bz2
rails-07a4c76a07641840892463eb934400abdf694927.zip
Only raise DelegationError if it's is the source of the exception
This fixes situations where nested NoMethodError exceptions are masked by delegations. This would cause confusion especially where there was a problem in the Rails booting process because of a delegation in the routes reloading code. Fixes #10559
Diffstat (limited to 'activesupport')
-rw-r--r--activesupport/CHANGELOG.md6
-rw-r--r--activesupport/lib/active_support/core_ext/module/delegation.rb21
-rw-r--r--activesupport/test/core_ext/module_test.rb27
3 files changed, 44 insertions, 10 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 8b0b7c8d70..c00cbaaa08 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,9 @@
+* Only raise `Module::DelegationError` if it's the source of the exception.
+
+ Fixes #10559
+
+ *Andrew White*
+
* Make `Time.at_with_coercion` retain the second fraction and return local time.
Fixes #11350
diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb
index 1aa72da743..0318f9a568 100644
--- a/activesupport/lib/active_support/core_ext/module/delegation.rb
+++ b/activesupport/lib/active_support/core_ext/module/delegation.rb
@@ -183,16 +183,17 @@ class Module
exception = %(raise DelegationError, "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}")
module_eval(<<-EOS, file, line - 2)
- def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block)
- _ = #{to} # _ = client
- _.#{method}(#{definition}) # _.name(*args, &block)
- rescue NoMethodError # rescue NoMethodError
- if _.nil? # if _.nil?
- #{exception} # # add helpful message to the exception
- else # else
- raise # raise
- end # end
- end # end
+ def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block)
+ _ = #{to} # _ = client
+ _.#{method}(#{definition}) # _.name(*args, &block)
+ rescue NoMethodError => e # rescue NoMethodError => e
+ location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, '#{method_prefix}#{method}'] # location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, 'customer_name']
+ if _.nil? && e.backtrace.first == location # if _.nil? && e.backtrace.first == location
+ #{exception} # # add helpful message to the exception
+ else # else
+ raise # raise
+ end # end
+ end # end
EOS
end
end
diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb
index ccf537e075..283b13ff8b 100644
--- a/activesupport/test/core_ext/module_test.rb
+++ b/activesupport/test/core_ext/module_test.rb
@@ -66,6 +66,23 @@ Tester = Struct.new(:client) do
delegate :name, :to => :client, :prefix => false
end
+Product = Struct.new(:name) do
+ delegate :name, :to => :manufacturer, :prefix => true
+ delegate :name, :to => :type, :prefix => true
+
+ def manufacturer
+ @manufacturer ||= begin
+ nil.unknown_method
+ end
+ end
+
+ def type
+ @type ||= begin
+ nil.type_name
+ end
+ end
+end
+
class ParameterSet
delegate :[], :[]=, :to => :@params
@@ -264,6 +281,16 @@ class ModuleTest < ActiveSupport::TestCase
assert_equal [3], se.ints
end
+ def test_delegation_doesnt_mask_nested_no_method_error_on_nil_receiver
+ product = Product.new('Widget')
+
+ # Nested NoMethodError is a different name from the delegation
+ assert_raise(NoMethodError) { product.manufacturer_name }
+
+ # Nested NoMethodError is the same name as the delegation
+ assert_raise(NoMethodError) { product.type_name }
+ end
+
def test_parent
assert_equal Yz::Zy, Yz::Zy::Cd.parent
assert_equal Yz, Yz::Zy.parent