-
-
Notifications
You must be signed in to change notification settings - Fork 443
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
Fix: set correct transaction status for unhandled exceptions in SentryTracingFilter
.
#1406
Conversation
sentry-spring/src/main/java/io/sentry/spring/tracing/SentryTracingFilter.java
Show resolved
Hide resolved
@@ -67,16 +67,22 @@ protected void doFilterInternal( | |||
final ITransaction transaction = startTransaction(httpRequest, sentryTraceHeader); | |||
try { | |||
filterChain.doFilter(httpRequest, httpResponse); | |||
} catch (Exception e) { | |||
// exceptions that are not handled by Spring | |||
transaction.setStatus(SpanStatus.INTERNAL_ERROR); |
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.
should we transaction.setThrowable(e)
and throw e;
?
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.
There is no need to setThrowable as the exception is captured while transaction is active - SentryClient sets the span context on the event.
Codecov Report
@@ Coverage Diff @@
## main #1406 +/- ##
============================================
+ Coverage 75.76% 75.77% +0.01%
- Complexity 1864 1865 +1
============================================
Files 185 185
Lines 6383 6387 +4
Branches 636 637 +1
============================================
+ Hits 4836 4840 +4
Misses 1258 1258
Partials 289 289
Continue to review full report at Codecov.
|
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.
LGTM
📜 Description
Fix: set correct transaction status for unhandled exceptions in
SentryTracingFilter
.💡 Motivation and Context
For exceptions that have not been handled with
@ControllerAdvice
or@ExceptionHandler
, theresponse#status
inSentryTracingFilter
is equal to200
, which results in settingSpanStatus.OK
on transaction.With this change, we handle also the case when the exception is not handled and set correct status code.
💚 How did you test it?
Unit tests.
📝 Checklist