bridge: attribute server errors to the user who hit them - #1902
Open
njrini99-code wants to merge 1 commit into
Open
bridge: attribute server errors to the user who hit them#1902njrini99-code wants to merge 1 commit into
njrini99-code wants to merge 1 commit into
Conversation
61.7% of error rows in the last 30 days carry no user_id, so the Bridge shows an error with no one attached to it. The read side already renders the user when there is one (admin/errors/[fingerprint] links user_id to the user page) — this was purely a capture gap. The cause: enrichTraceContext defaults traceId ambiently but never userId, so a row is attributed only when the call site happens to pass one. There are ~583 auth.getUser() call sites; threading userId through each of them is not a fix anyone finishes. Instead, attribute once at the source. createClient() (server) wraps auth.getUser and writes the resolved id into the existing AsyncLocalStorage request scope; enrichTraceContext reads it as a default. Call sites change nothing. Failure path: when getUser() returns no user — the common "session expired mid-round" shape, which is exactly when we most want to know who — the wrapper decodes the sub claim out of the sb-*-auth-token cookie WITHOUT verifying it and tags the row user_id_unverified. It deliberately does not call getSession(): with autoRefreshToken:false getSession still refreshes on demand and rotates the refresh token, and logging must never mutate auth state. The ambient userId is ATTRIBUTION ONLY. Authorization keeps calling supabase.auth.getUser() directly; a verified id is never downgraded by a later unverified read. Reach, measured against requestId (same scope, already shipped), 30d server_action errors: 4,924 total, 4,246 in a request scope, 1,927 attributed. 2,319 rows gain a user_id; 678 sit outside any scope and are unreachable by this change. Sources without a withAdminObserved scope (integrity, cron, server_component, auth) stay at 0% — several of those legitimately have no user. Forward-only; the 3,402 existing unattributed rows cannot be back-filled. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
61.7% of error rows in the last 30 days carry no
user_id, so the Bridge shows an error with nobody attached to it.The read side already works —
src/app/admin/errors/[fingerprint]/page.tsxlinksuser_idto the user page whenever there is one. This is purely a capture gap:enrichTraceContextdefaultstraceIdambiently but neveruserId, so a row is attributed only when the call site happened to pass one. There are ~583auth.getUser()call sites; threadinguserIdthrough each is not a fix anyone finishes.The fix
Attribute once at the source.
createClient()(server) wrapsauth.getUserand writes the resolved id into the existing AsyncLocalStorage request scope (request-context.ts, the same storerequestIdalready uses).enrichTraceContextreads it as a default. Call sites change nothing.getUser()returns no user — the common "session expired mid-round" shape, which is exactly when we most want to know who — the wrapper decodes thesubclaim out of thesb-*-auth-tokencookie without verifying it and tags the rowuser_id_unverified.Two deliberate constraints
getSession(). WithautoRefreshToken: falseon the SSR client,getSession()still refreshes on demand and rotates the refresh token. Logging must never mutate auth state. There is a test assertinggetSessionis not called and no cookie is written.userIdis ATTRIBUTION ONLY. Authorization keeps callingsupabase.auth.getUser()directly. A verified id is never downgraded by a later unverified read.user_id_unverifiedmeans "getUser() did not return this id", not "this session is bad" — it also fires on transient auth-check failures where the session is fine. Documented at the tag site.Measured reach
Against
requestId(same scope, already shipped), 30dserver_actionerrors:requestIdpresent)user_idfrom this changeSources with no
withAdminObservedscope —integrity(278),cron(32),server_component(35),auth(41) — stay at 0%; several of those legitimately have no user. Forward-only: the 3,402 existing unattributed rows cannot be back-filled.Tests
src/lib/__tests__/server-error-logger-user-attribution.test.ts(7) — ambient attribution with no call-site change; both tables agree; explicit call-siteuserIdwins; unverified subject flagged; no-op outside a scope; unverified never downgrades verified; verified upgrades unverified.src/lib/supabase/__tests__/server-user-attribution.test.ts(6) —getSessionnever called, zero cookie writes, chunked-cookie reassembly, non-base64 cookie, silence on malformed cookies, originalgetUserresult returned identically.Verified the logger tests fail (3 of 7) with the change stashed. Full local gates green: typecheck 0, lint 0, 7,565 tests pass.
🤖 Generated with Claude Code