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

Deduplicate error wrappers and error causes #1366

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

Conversation

tombruijn
Copy link
Member

We got a report that an Action View error was reported twice. Once the ActionView::Template::Error wrapper and once the error that actually occurred in the view (the error cause, e.g. NoMethodError).

This was caused by receiving the error cause from the Rails error reporter and the error wrapper in our error middleware. This was detailed in issue https://github.com/appsignal/support/issues/345 and Rails issue rails/rails#53938. (This only happened in production due to a Rails behavior quirk between development and production, addressed in the issue I linked above.)

This change will make sure we do not report both errors, but only report the ActionView::Template::Error.

Closes https://github.com/appsignal/support/issues/345

We got a report that an Action View error was reported twice. Once the
`ActionView::Template::Error` wrapper and once the error that actually
occurred in the view (the error cause, e.g. `NoMethodError`).

This was caused by receiving the error cause from the Rails error
reporter and the error wrapper in our error middleware. This was
detailed in issue appsignal/support#345 and
Rails issue rails/rails#53938.
(This only happened in production due to a Rails behavior quirk between
development and production, addressed in the issue I linked above.)

This change will make sure we do not report both errors, but only report
the `ActionView::Template::Error`.

Closes appsignal/support#345
@tombruijn tombruijn added the bug label Jan 10, 2025
@tombruijn tombruijn self-assigned this Jan 10, 2025
@tombruijn tombruijn marked this pull request as ready for review January 13, 2025 09:19
lib/appsignal/transaction.rb Show resolved Hide resolved
@backlog-helper

This comment has been minimized.

3 similar comments
@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper

This comment has been minimized.

@backlog-helper
Copy link


This is a message from the daily scheduled checks.

New issue guide | Backlog management | Rules | Feedback

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

Successfully merging this pull request may close these issues.

3 participants