-
-
Notifications
You must be signed in to change notification settings - Fork 494
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
Set sentry.error_event_id in request env if the middleware captures errors #1849
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1849 +/- ##
=======================================
Coverage 98.39% 98.39%
=======================================
Files 146 146
Lines 8735 8744 +9
=======================================
+ Hits 8595 8604 +9
Misses 140 140
Continue to review full report at Codecov.
|
@@ -32,7 +32,9 @@ def capture_exception(exception, env) | |||
current_scope.set_transaction_name(original_transaction) | |||
end | |||
|
|||
Sentry::Rails.capture_exception(exception) | |||
Sentry::Rails.capture_exception(exception).tap do |event| | |||
env[ERROR_EVENT_ID_KEY] = event.event_id if event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should also go into the rack middleware here ?
def capture_exception(exception, _env) | ||
Sentry.capture_exception(exception) | ||
def capture_exception(exception, env) | ||
Sentry.capture_exception(exception).tap do |event| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grosser This also has the same logic.
I had to replicate them in 2 places because the receivers are different: Sentry
and Sentry::Rails
and I don't want to introduce another abstraction just for the 2 occurrences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, very useful!
It will allow users/libraries to implement mechanisms displaying relevant error events' ids. Which will help their users report issues.
Closes #1846