Skip to content

Commit 07a4c76

Browse files
committed
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 rails#10559
1 parent e7e81b4 commit 07a4c76

File tree

3 files changed

+44
-10
lines changed

3 files changed

+44
-10
lines changed

activesupport/CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
* Only raise `Module::DelegationError` if it's the source of the exception.
2+
3+
Fixes #10559
4+
5+
*Andrew White*
6+
17
* Make `Time.at_with_coercion` retain the second fraction and return local time.
28

39
Fixes #11350

activesupport/lib/active_support/core_ext/module/delegation.rb

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -183,16 +183,17 @@ def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &
183183
exception = %(raise DelegationError, "#{self}##{method_prefix}#{method} delegated to #{to}.#{method}, but #{to} is nil: \#{self.inspect}")
184184

185185
module_eval(<<-EOS, file, line - 2)
186-
def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block)
187-
_ = #{to} # _ = client
188-
_.#{method}(#{definition}) # _.name(*args, &block)
189-
rescue NoMethodError # rescue NoMethodError
190-
if _.nil? # if _.nil?
191-
#{exception} # # add helpful message to the exception
192-
else # else
193-
raise # raise
194-
end # end
195-
end # end
186+
def #{method_prefix}#{method}(#{definition}) # def customer_name(*args, &block)
187+
_ = #{to} # _ = client
188+
_.#{method}(#{definition}) # _.name(*args, &block)
189+
rescue NoMethodError => e # rescue NoMethodError => e
190+
location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, '#{method_prefix}#{method}'] # location = "%s:%d:in `%s'" % [__FILE__, __LINE__ - 2, 'customer_name']
191+
if _.nil? && e.backtrace.first == location # if _.nil? && e.backtrace.first == location
192+
#{exception} # # add helpful message to the exception
193+
else # else
194+
raise # raise
195+
end # end
196+
end # end
196197
EOS
197198
end
198199
end

activesupport/test/core_ext/module_test.rb

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,23 @@ def self.table_name
6666
delegate :name, :to => :client, :prefix => false
6767
end
6868

69+
Product = Struct.new(:name) do
70+
delegate :name, :to => :manufacturer, :prefix => true
71+
delegate :name, :to => :type, :prefix => true
72+
73+
def manufacturer
74+
@manufacturer ||= begin
75+
nil.unknown_method
76+
end
77+
end
78+
79+
def type
80+
@type ||= begin
81+
nil.type_name
82+
end
83+
end
84+
end
85+
6986
class ParameterSet
7087
delegate :[], :[]=, :to => :@params
7188

@@ -264,6 +281,16 @@ def test_delegation_invokes_the_target_exactly_once
264281
assert_equal [3], se.ints
265282
end
266283

284+
def test_delegation_doesnt_mask_nested_no_method_error_on_nil_receiver
285+
product = Product.new('Widget')
286+
287+
# Nested NoMethodError is a different name from the delegation
288+
assert_raise(NoMethodError) { product.manufacturer_name }
289+
290+
# Nested NoMethodError is the same name as the delegation
291+
assert_raise(NoMethodError) { product.type_name }
292+
end
293+
267294
def test_parent
268295
assert_equal Yz::Zy, Yz::Zy::Cd.parent
269296
assert_equal Yz, Yz::Zy.parent

0 commit comments

Comments
 (0)