-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Avoid bundle exec rescue of SignalException #6092
Avoid bundle exec rescue of SignalException #6092
Conversation
I think the Travis CI failure for one variant is spurious ("RuntimeError: Digest::Base cannot be directly inherited in Ruby") but I don't seem to have a rebuild option, and I'm going to try to resist the temptation of making a cosmetic commit to achieve the same. |
Looks like #6084 is in progress to resolve this same CI failure. |
@dekellum That other Digest-related change has been merged - now master builds green again. |
This allows SignalException to be handled gracefully by the ruby interpreter. (rebased on to master with CI Fix, 6084) github: fixes rubygems#6090
7a79f52
to
a0c1d73
Compare
Yes, thanks all, CI build now passing with rebase. |
Can you please add a test in |
I read that spec file, and adding a spec for SignalException sounds reasonable and should be straightforward. Unfortunately I am unable to get the specs working at all in my local environment, see #6105. To expedite this bug fix, perhaps someone who can run the specs locally could attempt this? |
In the meantime, you can run the individual file with |
I get a different failure ( |
Lacking a workaround for #6105, this was bit of a painful process using Travis as my dev. environment, setup against my own repo. But the test in b10910a passes Bundler's CI. Could you review and let me know if this is sufficient? Note that only the Kernel::load path is relevant to #6090, and that is the path that is tested here. |
Thanks so much for sticking with it <3 @bundlerbot r+ |
📌 Commit b10910a has been approved by |
…ddins Avoid bundle exec rescue of SignalException Avoid rescue of SignalException in kernel_load and with_friendly_errors This allows SignalException to be handled gracefully by the ruby interpreter. This supersedes #6091. ### What was the end-user problem that led to this PR? Fixes #6090 ### What was your diagnosis of the problem? CLI::Exec#kernel_load should not rescue the SignalException. Neither should with_friendly_errors, so it unwinds all the way to the ruby interpreter. ### What is your fix for the problem, implemented in this PR? Two rescue and re-raises specific to SignalException. ### Why did you choose this fix out of the possible options? My first naive attempt was #6091, where I had missed the outer rescue in with_friendly_errors, all of the other special friendly error behavior, and the spec dependencies. While I still suspect that kernel_load and friendly_errors might be rescuing/handling more than they should (for comparison: binstub doesn't do this) this is a much more minimal, safer, and easier fix to #6090.
☀️ Test successful - status-travis |
…ddins Avoid bundle exec rescue of SignalException Avoid rescue of SignalException in kernel_load and with_friendly_errors This allows SignalException to be handled gracefully by the ruby interpreter. This supersedes #6091. ### What was the end-user problem that led to this PR? Fixes #6090 ### What was your diagnosis of the problem? CLI::Exec#kernel_load should not rescue the SignalException. Neither should with_friendly_errors, so it unwinds all the way to the ruby interpreter. ### What is your fix for the problem, implemented in this PR? Two rescue and re-raises specific to SignalException. ### Why did you choose this fix out of the possible options? My first naive attempt was #6091, where I had missed the outer rescue in with_friendly_errors, all of the other special friendly error behavior, and the spec dependencies. While I still suspect that kernel_load and friendly_errors might be rescuing/handling more than they should (for comparison: binstub doesn't do this) this is a much more minimal, safer, and easier fix to #6090. (cherry picked from commit 6d0b74c)
Avoid rescue of SignalException in kernel_load and with_friendly_errors This allows SignalException to be handled gracefully by the ruby interpreter.
This supersedes #6091.
What was the end-user problem that led to this PR?
Fixes #6090
What was your diagnosis of the problem?
CLI::Exec#kernel_load should not rescue the SignalException. Neither should with_friendly_errors, so it unwinds all the way to the ruby interpreter.
What is your fix for the problem, implemented in this PR?
Two rescue and re-raises specific to SignalException.
Why did you choose this fix out of the possible options?
My first naive attempt was #6091, where I had missed the outer rescue in with_friendly_errors, all of the other special friendly error behavior, and the spec dependencies. While I still suspect that kernel_load and friendly_errors might be rescuing/handling more than they should (for comparison: binstub doesn't do this) this is a much more minimal, safer, and easier fix to #6090.