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

Skip loading rack middleware with Rails' error reporting enabled? #1163

Open
tisba opened this issue Sep 2, 2024 · 4 comments
Open

Skip loading rack middleware with Rails' error reporting enabled? #1163

tisba opened this issue Sep 2, 2024 · 4 comments
Assignees
Labels

Comments

@tisba
Copy link

tisba commented Sep 2, 2024

With @waltjones' #1161 (adding support for Rails error reporter), shouldn't it be possible and actually desired to not load the Rack middleware?

Copy link

linear bot commented Sep 2, 2024

@zdavis-rollbar zdavis-rollbar added the Ruby label Sep 3, 2024 — with Linear
@morgoth
Copy link
Contributor

morgoth commented Sep 5, 2024

I second that question.
I didn't try yet the new reporter integration, but wouldn't the error be double submitted with the middleware and Active Job plugin enabled?

@waltjones
Copy link
Contributor

@morgoth Reporting in the middleware is disabled when the Rails reporter is enabled, though the middleware itself isn't unloaded. Any double send should be reported as a bug. I didn't look at Active Job. If there's an issue there, I'll update.

For people interested in fully unloading the middleware, there may be a way to do this that works for everyone. Here's the rationale for the way it is now.

The middleware loads much earlier than initializers, where the Rollbar config is applied. Configuring the middleware to load after initializers (so a flag could be used to skip the middleware) would change current behavior for people who use it, and would leave no uncaught error handler enabled during that part of the init sequence.

Loading it by default, and then unloading it during Rollbar init may be preferred by some people, but the middleware would still be there during that part of Rails init for everyone.

Even when using the Rails error reporter/subscriber, the request and current user information collected from the Rack request by the middleware is added to those Rollbar payloads. Those may be available via the controller objects sent as context to the subscriber, but the Rails doc encourages people to reuse that argument for other things. It doesn't appear to be as consistent, or unlikely to change in the future. People who use the Rails reporter but also call methods like Rollbar.info() would no longer have these keys automatically populated in those payloads.

Because the Rails subscriber loads based on a config option, it loads at Rollbar init. The middleware is active until that time and then reporting is disabled when the subscriber is enabled. This provides uncaught error reporting until the subscriber is enabled.

I may have overlooked a better way to do this, and I'm open to suggestions.

@tisba
Copy link
Author

tisba commented Sep 5, 2024

A fairly straight forward solution at least for a transition phase could be to offer an alternative require that would not load the middleware at all:

gem "rollbar", require: ["no_middleware"]

I'm not terribly familiar with the internal structure of the rollbar gem, but I know this to work well for other gems, like rack-mini-profiler, see MiniProfiler/rack-mini-profiler#444.

@waltjones waltjones self-assigned this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants