feat(web): associate Sentry errors with users#1452
Conversation
|
@brendan-kellam your pull request is missing a changelog! |
WalkthroughAdds Sentry user context tracking across the web app. A new ChangesSentry user context
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant SessionProvider
participant SentryUserProvider
participant setSentryUser
participant Sentry
SessionProvider->>SentryUserProvider: provide session status and user
SentryUserProvider->>setSentryUser: update resolved user and PII flag
setSentryUser->>Sentry: set or clear user context
sequenceDiagram
participant Request
participant withAuth
participant setSentryUser
participant Sentry
Request->>withAuth: getAuthContext resolves principal
withAuth->>setSentryUser: pass user or null, PII flag
setSentryUser->>Sentry: set or clear user context
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Move setSentryUser out of the memoized auth() resolver and into getAuthContext so it captures the resolved principal for all auth sources (session, OAuth Bearer, API key) rather than clearing the identity on API-key / Bearer requests where auth() returns null. Co-authored-by: Brendan Kellam <[email protected]>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort 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 87756c8. Configure here.
| setSentryUser( | ||
| user ?? null, | ||
| env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true', | ||
| ); |
There was a problem hiding this comment.
Sentry user set after org lookup
Low Severity
setSentryUser runs only after the org lookup succeeds. If that query throws or returns missing, the already-resolved principal is never attached, so the resulting Sentry event lacks user identity even though auth already succeeded.
Reviewed by Cursor Bugbot for commit 87756c8. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/web/src/middleware/withAuth.ts`:
- Around line 87-94: Move the setSentryUser call in the withAuth middleware to
immediately after authResult resolves the principal and before the organization
lookup, preserving the existing PII flag and unauthenticated null behavior.
Ensure lookup failures are captured with the authenticated user context.
🪄 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: CHILL
Plan: Pro
Run ID: 69b9026b-7932-4238-b19a-aa4b1ec44c1e
📒 Files selected for processing (1)
packages/web/src/middleware/withAuth.ts
| // Associate the resolved principal with Sentry so request-scoped errors | ||
| // carry the user's identity, regardless of how they authenticated (session, | ||
| // OAuth Bearer, or API key). Clears the identity for unauthenticated requests. | ||
| setSentryUser( | ||
| user ?? null, | ||
| env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true', | ||
| ); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Associate the user before the organization lookup.
authResult has already resolved the principal at Line 73, but setSentryUser runs only after org.findUnique(). If that lookup fails, the captured server error will not include the authenticated user context.
Suggested ordering
const authResult = await getAuthenticatedUser();
+const user = authResult?.user;
+setSentryUser(
+ user ?? null,
+ env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === "true",
+);
const org = await __unsafePrisma.org.findUnique({
// ...
});
-const user = authResult?.user;
-// setSentryUser(...)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Associate the resolved principal with Sentry so request-scoped errors | |
| // carry the user's identity, regardless of how they authenticated (session, | |
| // OAuth Bearer, or API key). Clears the identity for unauthenticated requests. | |
| setSentryUser( | |
| user ?? null, | |
| env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true', | |
| ); | |
| const authResult = await getAuthenticatedUser(); | |
| const user = authResult?.user; | |
| setSentryUser( | |
| user ?? null, | |
| env.SOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLED === 'true', | |
| ); | |
| const org = await __unsafePrisma.org.findUnique({ | |
| // ... | |
| }); |
🤖 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 `@packages/web/src/middleware/withAuth.ts` around lines 87 - 94, Move the
setSentryUser call in the withAuth middleware to immediately after authResult
resolves the principal and before the organization lookup, preserving the
existing PII flag and unauthenticated null behavior. Ensure lookup failures are
captured with the authenticated user context.
1 participant
Summary
Testing
yarn workspace @sourcebot/web test(1,074 tests passed)yarn workspace @sourcebot/web lint(passes with 4 pre-existing warnings)Note
Low Risk
Observability-only change with no auth behavior changes; PII is limited to user id unless the existing telemetry PII flag is enabled.
Overview
Sentry events can now be tied to the active user on both the browser and server, using a shared
setSentryUserhelper that callsSentry.setUser.On the client,
SentryUserProvideris mounted in the root layout insideSessionProvider. It waits until the NextAuth session is notloading, then sets or clears Sentry user context when auth state changes. On the server,getAuthContextinwithAuthsets the same context right after the request principal is resolved (session, OAuth bearer, or API key), and clears it when there is no user.By default only
idis sent to Sentry; email and name are included only whenSOURCEBOT_TELEMETRY_PII_COLLECTION_ENABLEDis'true', matching existing telemetry PII behavior. Vitest coverage was added for the helper and provider (loading, sign-in, sign-out, PII on/off).Reviewed by Cursor Bugbot for commit 87756c8. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Tests