The current implementation of handle_exception formats the exception backtrace using Exception#full_message prior to applying filter_backtrace on split lines of the full_message. Other projects that use irb intuitively expect that backtrace filtering will act on objects that conform to the Exception#backtrace API, and this discrepancy has introduced a bug in Rails where backtraces in the Rails console are entirely silenced (https://github.com/rails/rails/issues/49259)
Closes #913
Why this solution?
We considered working around this in Rails, but the backtrace_cleaner class in Rails is not only used for the console; it still needs to work in other contexts where backtrace cleaning is actually being applied to Exception#backtrace.
This proposed change is the least amount of change to fix the behaviour - the filter now acts on Exception#backtrace, but the rest of the logic for formatting and order remains unchanged and relies on Exception#full_message. This seems to be a reasonable use case for Exception#full_message and avoids needing to rewrite the formatting and ordering logic that is already working.
The filtering previously did not apply to irb bugs (it was within unless irb_bug), so it was added to the else branch of the irb_bug conditional to preserve that behaviour.
Testing
This branch passes the tests run with bundle exec rake. It looks like handle_exception is tested by the tests eval_input within test_context.rb, which I briefly looked over and the test coverage seems good to me. This is my first contribution to irb, please let me know if there are other testing requirements.
With respect to fixing the bug in Rails, here is a screenshot showing the correct/expected backtrace in the Rails console in a fresh Rails test app pointing to the version of irb proposed in this PR.
tompng
commented
8 months ago
Why is this change needed?
The current implementation of
handle_exceptionformats the exception backtrace usingException#full_messageprior to applyingfilter_backtraceon split lines of thefull_message. Other projects that use irb intuitively expect that backtrace filtering will act on objects that conform to theException#backtraceAPI, and this discrepancy has introduced a bug in Rails where backtraces in the Rails console are entirely silenced (https://github.com/rails/rails/issues/49259)Closes #913
Why this solution?
We considered working around this in Rails, but the backtrace_cleaner class in Rails is not only used for the console; it still needs to work in other contexts where backtrace cleaning is actually being applied to
Exception#backtrace.This proposed change is the least amount of change to fix the behaviour - the filter now acts on
Exception#backtrace, but the rest of the logic for formatting and order remains unchanged and relies onException#full_message. This seems to be a reasonable use case forException#full_messageand avoids needing to rewrite the formatting and ordering logic that is already working.The filtering previously did not apply to irb bugs (it was within
unless irb_bug), so it was added to the else branch of theirb_bugconditional to preserve that behaviour.Testing
This branch passes the tests run with
bundle exec rake. It looks likehandle_exceptionis tested by the testseval_inputwithintest_context.rb, which I briefly looked over and the test coverage seems good to me. This is my first contribution to irb, please let me know if there are other testing requirements.With respect to fixing the bug in Rails, here is a screenshot showing the correct/expected backtrace in the Rails console in a fresh Rails test app pointing to the version of irb proposed in this PR.