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

Allow jobs to rescue all exceptions #41214

Merged

Conversation

etiennebarrie
Copy link
Contributor

Before this commit, only StandardError exceptions can be handled by rescue_from handlers.

This changes the rescue clause to catch all Exception objects, allowing rescue handlers to be defined for Exception classes not inheriting from StandardError.

This means that rescue handlers that are rescuing Exceptions outside of StandardError exceptions may rescue exceptions that were not being rescued before this change.

Other Information

When this was introduced in ef4aff0, ActionController was already rescuing all Exception objects, so maybe this was intentional, but there's no specific test about it.

There is some incompatibility in the sense that if an app is using rescue_from(Exception), previously it would not have caught some exceptions, and now it will. But it was probably the expected behavior they wanted. We're not sure how we would go about making it fully backwards compatible anyway.

cc @rafaelfranca @byroot

@rails-bot rails-bot bot added the activejob label Jan 22, 2021
Copy link
Member

@rafaelfranca rafaelfranca left a comment

Choose a reason for hiding this comment

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

Can you add a CHANGELOG entry?

Before this commit, only StandardError exceptions can be handled by
rescue_from handlers.

This changes the rescue clause to catch all Exception objects, allowing
rescue handlers to be defined for Exception classes not inheriting from
StandardError.

This means that rescue handlers that are rescuing Exceptions outside of
StandardError exceptions may rescue exceptions that were not being
rescued before this change.

Co-authored-by: Adrianna Chang <adrianna.chang@shopify.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants