fix(ui): reduce sentry alert noise#11665
Conversation
- Add centralized Sentry actionability filtering - Suppress expected control-flow and validation noise - Preserve actionable runtime and API contract failures
📝 WalkthroughWalkthroughA shared Sentry event policy module now handles filtering, tagging, and deduplication. Client, Edge, and server Sentry configs delegate ChangesCentralized Sentry Event Policy
Sequence Diagram(s)sequenceDiagram
participant Runtime as Client/Edge/Server
participant Config as Sentry beforeSend
participant Policy as applySentryEventPolicy
participant Dedup as isErrorAlreadyReported
participant Tagging as tagActionableEvent
Runtime->>Config: event + hint
Config->>Policy: applySentryEventPolicy(event, hint, source)
Policy->>Dedup: check reported marker
Dedup-->>Policy: reported or not
Policy-->>Config: null for warning/control-flow/expected HTTP
Policy->>Tagging: tag retained event
Tagging-->>Policy: event.tags updated
Policy-->>Config: tagged event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
|
✅ All necessary |
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/lib/server-actions-helper.ts (1)
158-188:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark errors captured by
handleApiErroras reported.Line 160 captures an unreported
Error, but the object is never marked afterward, so a later pass through this helper or the shared policy still sees it as unreported and can duplicate the alert. Mark after capture, matching the 5xx path.Suggested fix
if (error instanceof Error) { Sentry.captureException(error, { tags: { error_source: SentryErrorSource.HANDLE_API_ERROR, error_type: SentryErrorType.UNEXPECTED_ERROR, }, level: "error", contexts: { error_details: { message: error.message, stack: error.stack, }, }, }); + markErrorAsReported(error); } else { // Capture non-Error objects Sentry.captureMessage( `Non-Error object in handleApiError: ${String(error)}`, { level: "warning", tags: { error_source: SentryErrorSource.HANDLE_API_ERROR, error_type: SentryErrorType.NON_ERROR_OBJECT, }, extra: { error: error, }, }, ); + markErrorAsReported(error); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/server-actions-helper.ts` around lines 158 - 188, The handleApiError function captures errors using Sentry.captureException but does not mark them as captured afterward, which can result in duplicate alerts if the same error object is processed again. After calling Sentry.captureException in the code block where error instanceof Error is checked, mark the error as captured using the same mechanism employed in the 5xx path to prevent subsequent processing from treating it as an unreported error.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/lib/server-actions-helper.ts`:
- Around line 238-242: The Sentry fingerprint in the error handling code is
using the full response.url which can contain query strings and resource IDs,
causing over-fragmentation of error grouping and exposing URL values in
metadata. Replace the response.url value in the fingerprint array with a
sanitized, low-cardinality path instead (such as the pathname from the URL
object). Apply this same normalization pattern consistently to both the
fingerprint array at line 238 and the captureApiServerError function at line 92
to ensure consistent URL handling across error capture methods.
In `@ui/sentry/event-policy.ts`:
- Around line 44-54: The interfaces SentryEventPolicyOptions, SentryEventHint,
and SentryPolicyEvent are currently not exported, which limits their
accessibility for type safety at call sites. Add the export keyword before each
of these three interface declarations to make them available to consumers in
client, edge, and server configurations, improving type safety and
documentation.
---
Outside diff comments:
In `@ui/lib/server-actions-helper.ts`:
- Around line 158-188: The handleApiError function captures errors using
Sentry.captureException but does not mark them as captured afterward, which can
result in duplicate alerts if the same error object is processed again. After
calling Sentry.captureException in the code block where error instanceof Error
is checked, mark the error as captured using the same mechanism employed in the
5xx path to prevent subsequent processing from treating it as an unreported
error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 26713a2d-b677-438e-a70f-7b0ea8afa1d0
📒 Files selected for processing (11)
ui/CHANGELOG.mdui/instrumentation-client.tsui/lib/server-actions-helper.test.tsui/lib/server-actions-helper.tsui/sentry/event-policy.test.tsui/sentry/event-policy.tsui/sentry/index.tsui/sentry/sentry-event-filter.test.tsui/sentry/sentry-event-filter.tsui/sentry/sentry.edge.config.tsui/sentry/sentry.server.config.ts
|
CI note: I reran the failed |
Alan-TheGentleman
left a comment
Alan-TheGentleman
left a comment
There was a problem hiding this comment.
Two things to address:
-
Captured
handleApiErrorerrors are not marked as reported.handleApiErrorchecksisErrorAlreadyReported(error)before capturing, but afterSentry.captureException(error, ...)it never callsmarkErrorAsReported(error). If the same Error object goes through the sharedbeforeSendpolicy later, it still looks unreported and can produce the duplicate alert this PR is trying to prevent. -
Fingerprints use the full response URL. Both the 5xx path and unexpected 4xx path include
response.urldirectly in the fingerprint. That can include query strings or resource identifiers, which fragments grouping and can leak high-cardinality URL data into Sentry metadata. Normalize this to a low-cardinality path, and use the same normalization in both fingerprint paths.
The interface export suggestion from CodeRabbit is not blocking; it is useful only if consumers need those exact helper types. The Okta E2E failure also looks unrelated to this diff based on the current logs.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/lib/server-actions-helper.ts (1)
96-98: 🩺 Stability & Availability | 🟠 MajorAvoid setting a sync dedupe flag on the error object immediately after
Sentry.captureException.
captureExceptionreturns immediately after queuing the event, whilebeforeSend(where you likely check for duplicates) runs asynchronously later. PlacingmarkErrorAsReported(serverError)on line 96 synchronously sets a flag on the exception object before Sentry processes the event. If yourbeforeSendlogic uses this flag to deduplicate, it will suppress the very first report.Move the
markErrorAsReportedcall to a point after the event has been sent (if your SDK supports apost_processhook) or decouple the "internal error reported" state from the specific error object reference.// Incorrect: Flag set before Sentry processes the event, potentially causing premature drop in beforeSend. Sentry.captureException(serverError, { /* ... */ }); markErrorAsReported(serverError); throw serverError;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/lib/server-actions-helper.ts` around lines 96 - 98, The `markErrorAsReported(serverError)` call in `server-actions-helper` is happening too early relative to `Sentry.captureException`, which can cause `beforeSend` dedupe logic to drop the first report. Update the error-reporting flow so the reported flag is set only after Sentry has finished queuing/sending the event, or store the “reported” state separately from the `serverError` object reference. Keep the `throw serverError` behavior unchanged, and adjust the surrounding error handling path where `captureException` is invoked.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ui/lib/server-actions-helper.ts`:
- Around line 96-98: The `markErrorAsReported(serverError)` call in
`server-actions-helper` is happening too early relative to
`Sentry.captureException`, which can cause `beforeSend` dedupe logic to drop the
first report. Update the error-reporting flow so the reported flag is set only
after Sentry has finished queuing/sending the event, or store the “reported”
state separately from the `serverError` object reference. Keep the `throw
serverError` behavior unchanged, and adjust the surrounding error handling path
where `captureException` is invoked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b5d53d51-9b6e-47df-ba7a-76b9ffa90973
📒 Files selected for processing (4)
ui/lib/server-actions-helper.test.tsui/lib/server-actions-helper.tsui/sentry/event-policy.test.tsui/sentry/event-policy.ts
2 participants
Context
The UI Sentry Slack alert channel was receiving non-actionable noise from warning-level events, controlled API/form validation, and duplicate backend/API failure paths. The goal is to make UI Sentry alerts actionable: if it reaches the alert channel, it should be worth reviewing.
This work is tied to the Sentry UI alert-channel cleanup tracked in the roadmap.
Description
This PR:
401/403/404API events only when structured status or clear API/HTTP context existsSteps to review
ui/sentry/event-policy.tsand confirm the actionability boundaries:401/403/404is dropped401/403/404without API context are keptui/lib/server-actions-helper.tsand confirm expected 4xx validation is suppressed while unexpected 4xx contract failures are capturedChecklist
Community Checklist
SDK/CLI
UI
API
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
Summary by CodeRabbit