Repository navigation
fix(marketing): suppress expected session fetch errors from Sentry - #1237
saddlepaddle wants to merge 1 commit into
Conversation
…ARKETING-17) The marketing site calls getSession to show logged-in vs logged-out CTAs. When visitors have stale/invalid cookies, BetterAuth throws an APIError which is expected and non-actionable. This was generating noise in Sentry (94 events). Fix by: - Adding beforeSend filter in Sentry server/edge configs to drop these events - Downgrading console.error to console.warn since failed session is expected
📝 WalkthroughWalkthroughThe PR adds Sentry filtering to exclude "Failed to get session" APIError events from error reporting across marketing site edge and server configurations, and updates the CTAButtons component to log session retrieval failures as warnings instead of errors, treating them as expected behavior. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Kitenite
left a comment
There was a problem hiding this comment.
Review: LGTM
Clean, well-scoped PR. The approach (Sentry beforeSend filter + console.warn downgrade) is the standard way to suppress expected-but-noisy errors.
What's good
- Defensive filtering — checks both
e.type === "APIError"ande.value?.includes("Failed to get session"), so it won't accidentally suppress unrelated errors. - Correct log level — downgrading
console.error→console.warnis appropriate since the session fetch failure is expected and non-actionable on the marketing site. - Parameterless
catch {}— clean removal of the unusederrorvariable. - Solid PR description — clear context on why this is expected (stale Clerk cookies), the Sentry noise impact (94 events), and the fallback behavior.
Minor nit (non-blocking)
The beforeSend block is copy-pasted identically across sentry.edge.config.ts and sentry.server.config.ts. These files were already identical before this PR so this follows the existing pattern, but if more filters accumulate over time it may be worth extracting a shared config helper. Not needed now though.
Kitenite
left a comment
There was a problem hiding this comment.
Review: LGTM
Clean, well-scoped PR. The approach (Sentry beforeSend filter + console.warn downgrade) is the standard way to suppress expected-but-noisy errors.
What's good
- Defensive filtering — checks both
e.type === "APIError"ande.value?.includes("Failed to get session"), so it won't accidentally suppress unrelated errors. - Correct log level — downgrading
console.errortoconsole.warnis appropriate since the session fetch failure is expected and non-actionable on the marketing site. - Parameterless
catch {}— clean removal of the unusederrorvariable. - Solid PR description — clear context on why this is expected (stale Clerk cookies), the Sentry noise impact (94 events), and the fallback behavior.
Minor nit (non-blocking)
The beforeSend block is copy-pasted identically across sentry.edge.config.ts and sentry.server.config.ts. These files were already identical before this PR so this follows the existing pattern, but if more filters accumulate over time it may be worth extracting a shared config helper. Not needed now though.
Summary
beforeSendfilter in Sentry server and edge configs to dropAPIError: Failed to get sessionevents (MARKETING-17)console.errortoconsole.warninCTAButtonssince failed session fetch is expected behavior on the marketing siteerrorvariable from catch blockContext
The marketing site's
CTAButtonscomponent callsauth.api.getSession()on every page to show logged-in vs logged-out CTAs in the header. When visitors have stale or invalid cookies (e.g., leftover Clerk cookies from the auth migration), BetterAuth throws anAPIError("Failed to get session"). This is expected and non-actionable — the component already gracefully falls back to showing logged-out CTAs — but it was generating Sentry noise (94 events).Test plan
bun run typecheck --filter=@superset/marketing)bun run lint:fix)Summary by CodeRabbit
Bug Fixes
Chores