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

The "raise" with no arguments raises the exception in $!, nested case too #185

Closed
wants to merge 1 commit into from

Conversation

aycabta
Copy link
Member

@aycabta aycabta commented Dec 29, 2016

Ruby 2.4 and later, raise's behavior has been changed. Nested raise with an argument raises only the argument, it doesn't have outer exception. So I removed the argument. The raise with no arguments raises what contains nested exceptions.

This fixes a failure of TestRakeApplication#test_display_exception_details_cause_loop that @hsbt refers at #184. This passed all Ruby versions in Travis CI.

...And this will fix Travis CI failure of #183.

Copy link
Contributor

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

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

Your explanation of 2.4's behavior makes this fix of the test code very clear.

@nobu
Copy link
Member

nobu commented Jan 4, 2017

This just re-raises "cause_b" exception, and makes no sense.

@aycabta
Copy link
Member Author

aycabta commented Jan 4, 2017

This just re-raises "cause_b" exception

I think so, but I didn't want to say so.

I do the complementary explanation that I did. The "nested exception" means chaining Exception with Exceptin#cause. The test checks just it.

@aycabta
Copy link
Member Author

aycabta commented Jan 4, 2017

https://twitter.com/hsbt/status/816458882541228032 (Japanese, I read with translation)

I grasped the overview around this tweet and its reply tree. If that tells you anything, I just made this Pull Request for 2.4's behavior, and I didn't understand the purpose of the test. I will leave it to appropriate one's judgement.

ref. The test is added for jimweirich#272 at first.

@nobu
Copy link
Member

nobu commented Jan 4, 2017

With this change, the test is almost same as test_display_exception_details_cause, except for an extra re-raising, and no meaning of loop.

@aycabta
Copy link
Member Author

aycabta commented Jan 5, 2017

OK, I got the picture. I'm thinking what test_display_exception_details_cause_loop should be deleted.

@hsbt hsbt closed this in 25f2e3b Jan 6, 2017
aycabta pushed a commit to aycabta/rake that referenced this pull request Jan 23, 2017
`test_display_exception_details_cause_loop` is same as
`test_display_exception_details_cause`.

Fixed ruby#185
@aycabta aycabta deleted the fix-exception-cause-loop branch April 1, 2017 07:36
kratob pushed a commit to rails-lts/rake that referenced this pull request Mar 16, 2022
`test_display_exception_details_cause_loop` is same as
`test_display_exception_details_cause`.

Fixed ruby#185
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

3 participants