Do not fail if an Exception has no backtrace #2903
Conversation
|
👋 we would need a test indicating how this can occur |
@JonRowe sure! I've added a test for this case. |
| @@ -202,6 +202,26 @@ def initialize(message, backtrace = [], cause = nil) | |||
| EOS | |||
| end | |||
|
|
|||
| it 'wont fail for the exception with a nil backtrace', :if => RSpec::Support::RubyFeatures.supports_exception_cause? do | |||
| allow(first_exception).to receive(:backtrace) { nil } | |||
There was a problem hiding this comment.
Can you find a real scenario where this would occur? We don't tend to defensively program around "maybes", we would want an actual scenario where this can occur.
There was a problem hiding this comment.
begin
raise StandardError, 'oops', nil
rescue => e
puts "backtrace: #{e.backtrace}"
endfails for me with:
backtrace: ["1.rb:2:in `<main>'"]
Maybe a custom exception would do?
There was a problem hiding this comment.
It won't work with the StandardError because the StandardError doesn't have the cause:
StandardError.new('foo').cause
# => nilBut it does fails for me in the real test when a custom error having the #cause is raised, but the last error in the chain doesn't have a backtrace set.
There was a problem hiding this comment.
The best is to use such a custom exception in a scenario, then.
There was a problem hiding this comment.
Sure! I've updated the test: https://github.com/rspec/rspec-core/pull/2903/files#diff-829be18287a3c0ec09a70d0505b7cc835c298063c15edce2e5a4bf20677e42f2R206
I realized that we don't have a scenario for the [] case too. So I've added both scenarios.
Does it make more sense now?
There was a problem hiding this comment.
Hey @JonRowe @pirj! Please let me know if the proposed changes are fine or should I improve something else here 😄
Also some of the environment specs on CI are failling. That's what I see in the output:
script/functions.sh: line 42: bin/rspec: No such file or directory
Error: Process completed with exit code 127.
Do you think it is fine or is it something that I might have touched with that change?
There was a problem hiding this comment.
I suggest you to rebase the PR, the error should disappear.
Will take a look a bit later. Thanks for pushing this forward
zinovyev
force-pushed
the
patch-1
branch
3 times, most recently
from
cf3f595 to
a6270f6
Compare
Currently `backtrace` of an exception is checked by the `empty?` method. But it sometimes occur that the Exception class will return `nil` for backtrace: ``` StandardError.new.backtrace.empty? NoMethodError: undefined method `empty?' for nil:NilClass ```
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thank you guys for your reviews and for being so helpful! |
|
I'll handle this later in the day. |
|
Thanks for your contribution! |
Do not fail if an Exception has no backtrace --- This commit was imported from rspec/rspec-core@9de11e5.
Do not fail if an Exception has no backtrace
Do not fail if an Exception has no backtrace
Do not fail if an Exception has no backtrace --- This commit was imported from rspec/rspec-core@90a15b2.
Do not fail if an Exception has no backtrace --- This commit was imported from rspec/rspec-core@6fc2b92.
Currently
backtraceof an exception is checked by theempty?method. But it sometimes occur that the Exception class will returnnilfor backtrace: