ref(core): Ensure error span status is always valid#22522
Conversation
size-limit report 📦
|
b5b62ef to
60f9673
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 60f9673. Configure here.
| } from '@sentry/core'; | ||
| import type { Span, SpanAttributes } from '@sentry/core'; | ||
| import { inferStatusFromAttributes, isStatusErrorMessageValid } from './utils/mapStatus'; | ||
| import { inferStatusFromAttributes } from './utils/mapStatus'; |
There was a problem hiding this comment.
Unreachable status normalization branch
Low Severity
applyOtelSpanStatus still tries to normalize invalid error messages for non-streaming spans, but it receives spanJSON.status from getStatusMessage, which already always returns a valid value. That makes the isStatusErrorMessageValid check and live setStatus call unreachable leftover after the core change.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 60f9673. Configure here.
There was a problem hiding this comment.
this is fine, we apply a more in-depth normalization here, we do not look at grpc status code in core implementation etc.
Lms24
left a comment
There was a problem hiding this comment.
Bundle size hit isn't ideal but I don't see an easy way to limit this to server runtimes only. Briefly thought about storing this on the client but then we'd need to get the client for span serialization which is also far from ideal 😅 Also, I think it could theoretically also affect the client-side.


We have some code in otel span serialization that ensures that span statuses are correct. if we remove this layer, however we can "invalid" span statuses (e.g. random error messages etc). This change ensures we always have valid span statuses everywhere, in the legacy transaction mode.