Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle empty backtrace from external filter #696

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

iNecas
Copy link

@iNecas iNecas commented May 19, 2017

In the wild the backtrace_filter can be set by external filter (Rails do
it for their silencer). This can lead to the case there the external
backtrace_filer returns empty array. Other parts of minitest count on
the fact that there will still be something in the backtrace to use, and
can lead to issues such as "undefined method `split' for nil:NilClass"
when producing deprecation warnings.

Since we already had the logic to use all the backtrace when filtering
too much, I've moved this logic to be used also for external filters.

In the wild the backtrace_filter can be set by external filter (Rails do
it for their silencer). This can lead to the case there the external
backtrace_filer returns empty array. Other parts of minitest count on
the fact that there will still be something in the backtrace to use, and
can lead to issues such as "undefined method `split' for nil:NilClass"
when producing deprecation warnings.

Since we already had the logic to use all the backtrace when filtering
too much, I've moved this logic to be used also for external filters.
dLobatog added a commit to dLobatog/katello that referenced this pull request Jun 2, 2017
Due to a bug on minitest that is being currently fixed, our tests are
broken when they run as the "test_develop_pr_katello" job in Jenkins.

The fix is published as minitest/minitest#696
however there is no version of minitest with it yet. A possible
workaround is to prevent these assertions like this PR does.
jlsherrill pushed a commit to Katello/katello that referenced this pull request Jun 5, 2017
Due to a bug on minitest that is being currently fixed, our tests are
broken when they run as the "test_develop_pr_katello" job in Jenkins.

The fix is published as minitest/minitest#696
however there is no version of minitest with it yet. A possible
workaround is to prevent these assertions like this PR does.
@zenspider zenspider self-assigned this Jun 29, 2017
@k0kubun
Copy link

k0kubun commented Aug 24, 2017

By rails/rails#29572 and upgrading Rails from 5.1.2 -> 5.1.3, I hit an error (undefined method 'split' for nil:NilClass) by Rails.backtrace_cleaner. I hope this is merged.

Copy link

@rickhull rickhull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about:

new_bt = backtrace_filter.filter bt
new_bt.empty? ? bt.dup : new_bt

Also, this suggestion is not meant as any rejection or "no" vote. If it is understood to be that, then I wish to retract it.

@jacobo
Copy link

jacobo commented Jan 27, 2020

seeing this as seemingly just a side effect of Rails (5.2.3) being loaded. No application code setting a custom backtrace filter or anything like that. Checking the value of Minitest.backtrace_filter at runtime shows a Rails::BacktraceCleaner object

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants