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

Using rescue_from in ApplicationMailer has unexpected results under rails 6 and rollbar 3.4.0 #1126

Closed
fastjames opened this issue Apr 17, 2023 · 2 comments · Fixed by #1143

Comments

@fastjames
Copy link

While reviewing dependabot version upgrades recently for a rails app that I maintain, I noticed an unexpected failure in the rollbar update from 3.3.0 to 3.4.0. After investigating further, I narrowed the error source to a use of rescue_from in the rails app's ApplicationMailer class to log mail errors in a model attribute. It appears that the new ActiveJob integration may have led to this bug.

I have created a demonstration app visible at https://github.com/fastjames/rollbar_postmark_test that describes the problem. With the rollbar gem version set to 3.4.0, the single spec will fail. If the rollbar gem version is set to 3.3.0 (or 3.3.3), the single spec will pass.

If there is a more recommended way to get what we need with rescue_from in the rails app itself, I am happy to open a documentation PR to add it to this gem.

@fastjames
Copy link
Author

The use of rescue_from here is inspired by the postmark-rails gem's wiki entry:

https://github.com/ActiveCampaign/postmark-rails/wiki/Error-Handling

@waltjones
Copy link
Contributor

I see there's a conflict between the rescue_from in rollbar-gem and the one in the app.

There are two fixes needed in rollbar-gem:

  • A config option to skip the ActiveJob plugin. (This exists for most other plugins, but is missing from this one.)
  • The plugin init needs to execute before the app init. This allows the multiple rescue_froms to be added in the correct order, and the one in rollbar-gem will only handle exceptions that weren't handled in the app.

I don't see a workaround for now, except to stay pinned to 3.3.3 until a fix is released.

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 a pull request may close this issue.

2 participants