perf(auth): memoize getAuthToken and refreshToken row read (CLI-13V)#828
Merged
perf(auth): memoize getAuthToken and refreshToken row read (CLI-13V)#828
Conversation
Eliminates the N+1 'SELECT * FROM auth WHERE id = 1' pattern flagged on every API-driven command (issue view, trace view, etc.). Each outgoing request previously hit the auth row three times: two getAuthToken() calls for cache-key building and one raw SELECT inside refreshToken(). Mirrors the existing cachedFingerprint memo-and-reset pattern: both new caches use wrapper-object sentinels to distinguish 'not cached' from 'cached as undefined' (logged-out), and are invalidated at both auth mutation points (setAuthToken, clearAuth) alongside the fingerprint. Also centralizes auth-cache reset in useTestConfigDir to stop module-scoped memos leaking across test files.
Contributor
|
Contributor
Codecov Results 📊✅ 138 passed | Total: 138 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 1944 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 95.27% 95.28% +0.01%
==========================================
Files 284 284 —
Lines 41152 41183 +31
Branches 0 0 —
==========================================
+ Hits 39206 39239 +33
- Misses 1946 1944 -2
- Partials 0 0 —Generated by Codecov Action |
Tighten JSDoc and inline comments added in the previous commit to match the terse style used elsewhere in auth.ts (cf. cachedFingerprint). No behavior change.
Add resetIdentityFingerprintCache() alongside the two new cache resets so the helper invalidates all three module-scoped auth caches, matching the existing lore documentation.
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.
Summary
Fixes CLI-13V — N+1 Query detector flagged
sentry.issue.viewon every run, culpritgetAuthToken.Every outgoing API request hit the SQLite
authrow three times:tryCacheHit→authHeaders(getAuthToken())— cache-key buildfetchWithRetry→refreshToken()— its own rawSELECT * FROM auth WHERE id = 1cacheResponse(..., authHeaders(getAuthToken()))— on cache missEach
getAuthToken()was wrapped bywithDbSpanplus the traced-statement proxy, producing paireddb.operation/dbspans. With 12 offender spans per pattern repetition, the performance detector flagged it.Repeating pattern in the trace before this fix:
Approach
Mirror the existing
cachedFingerprintmemo-and-reset pattern (src/lib/db/auth.ts:290+):cachedAuthTokenmemo forgetAuthToken().cachedAuthRowmemo forrefreshToken()'s full-row read.{ value: T | undefined } | undefinedwrapper objects to distinguish "not cached" from "cached as undefined" (logged-out) — avoids re-querying on every call in the logged-out case.resetAuthTokenCache,resetAuthRowCache) wired into both mutation points (setAuthToken,clearAuth) alongside the existingresetIdentityFingerprintCache().Safe under OAuth token rotation and 401 force-refresh because both paths route writes through
setAuthToken, which now invalidates all three auth caches.Net Impact
getAuthTokenspan appears at most once, not per API request.Testing
test/lib/db/auth.test.ts— newgetAuthToken memoizationsuite (5 tests: cache hit, logged-out caching,setAuthTokeninvalidation,clearAuthinvalidation, env-var contract) andrefreshToken row-read memoizationsuite (2 tests).test/helpers.ts—useTestConfigDirnow invalidates both caches inbeforeEach/afterEachso module-scoped memos can't leak across test files.test/lib/db/auth.property.test.ts—resetAuthCaches()helper called at the top of each property iteration (env-var mutations bypass the internal invalidation hooks).test/lib/db/model-based.test.ts— env-var commands and test setup invalidate the caches; this actually fixed one previously-flaky assertion (clearAuth also clears pagination cursors).All 56 auth-related tests pass. Full suite has the same number of pre-existing flaky failures as baseline (minus one this PR fixed).
Verification
bun test test/lib/db/auth.test.ts test/lib/db/auth.property.test.ts test/lib/db/model-based.test.tsManual trace verification: run
bun run dev -- issue view <id>withSENTRY_DSNset and confirm thegetAuthTokenspan appears at most once per command in the resulting trace.