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

Do::Halt - use Exception instead of StandardError #98

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

Conversation

gogiel
Copy link

@gogiel gogiel commented Jun 26, 2019

Do::Halt is a StandardError. Because of that rescue block can unexpectedly catch it in a wrapped method. This problem has been reported at #72

If Do::Halt was a direct Exception derivative it would be nearly invisible to end-user.

Both ActiveRecord and Sequel catch Exception. While this indeed is a breaking change it shouldn't affect majority of users.

In Discourse discussion we agreed that this breaking change will be included in some future major release.

@flash-gordon flash-gordon added this to the 2.0 milestone Jul 24, 2019
@nixme
Copy link

nixme commented Sep 18, 2019

What about using throw/catch instead?

@flash-gordon
Copy link
Member

@nixme using exceptions makes do notation transparent to transaction-like code (like one in AR and sequel), this is an important use case. We could have two versions of do notation with raise/rescue and throw/catch and let the user decide but that would be an overkill.

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

Successfully merging this pull request may close these issues.

None yet

3 participants