fix(auth): share the in-flight me() request - #245
Open
guyofeck wants to merge 1 commit into
Open
Conversation
The analytics module resolves its session context through `auth.me()` at client construction, which is the same moment most apps issue their own `me()`. Browsers serialize the two identical GETs, so the second pays the first's full latency — ~190ms on a cold load (BUG-837). `auth.me()` now shares its pending promise between concurrent callers. The promise is cleared as soon as the request settles, so no resolved user is ever retained: caching identity across requests would leave an app rendering a logged-in view after logout or a session swap. `setToken` and `logout` drop the pending request for the same reason — a `me()` already in flight must not resolve into callers that run after the identity changed. Deduping inside the analytics module would not have helped; the duplicate is between analytics and the app, and both go through `auth.me()`. Two related fixes in the analytics module: - The session context memoized the resolved `user_id` for the lifetime of the session and was never reset, so a visitor who loaded a page anonymously and then logged in kept reporting `user_id: null` on every subsequent event. It is now cleared on any identity change. - The heartbeat is browser-only, matching the other automatic events here. Outside a browser it fired a `me()` every 60s for the lifetime of a long-lived server-side client and kept the Node event loop alive. Explicit `analytics.track()` calls are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
guyofeck
marked this pull request as ready for review
August 9, 2026 09:59
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.
Problem
BUG-837: an app is ~190ms slower on every cold load because the SDK issues a secondGET /entities/User/meat client construction while the app is already issuing its own. Chrome serializes the two identical GETs, so the second pays the first's full latency.The source is the analytics module:
createAnalyticsModuleruns at client construction,trackInitializationEventqueues an event,startAnalyticsProcessorsends the first batch before thethrottleTimesleep, andflush()awaitsgetSessionContext()→auth.me().Fix
Share the in-flight request in
auth.me(). Both the app and analytics call this same method, so collapsing it there is what removes the duplicate — deduping inside the analytics module would not help, andgetSessionContextalready shares its own promise.The pending promise is cleared as soon as the request settles, so no resolved user is ever retained. Caching identity across requests would leave an app rendering a logged-in view after logout or a session swap.
setTokenandlogoutalso drop the pending request: ame()already in flight must not resolve into callers that run after the identity changed.This also collapses apps that call
me()twice on their own.Two related fixes in the analytics module
user_id. The session context memoized the resolveduser_idfor the lifetime of the session and nothing ever reset it — notlogout, notsetToken. A visitor who loaded a page anonymously and then logged in kept reportinguser_id: nullon every subsequent event. It is now cleared on any identity change. This is a data-quality bug, not a latency one.startHeartBeatProcessorwas not window-guarded, unlike the other automatic events here (initialization, session duration, visibility). On a long-lived server-side client it fired ame()every 60s and its interval kept the Node event loop alive. Explicitanalytics.track()calls from backend functions are unaffected.Scope — what this does not fix
Prod data (24h) shows ~88% of the 5.34M daily
User/mecalls come from the server-side SDK in backend functions, not browsers. This PR removes one request per cold browser load; it does not touch that server-side share, and it does not touch the ~33ms of per-request app-metadata reload that dominates the endpoint's 40ms p50. That work is separate, inapper.The Datadog p50 will not move on merge day. The SDK is pinned at
^0.8.35in the app templates, so apps pick this up on their next install or rebuild; the deployed fleet trails.Testing
npm run test:unit— 199 pass (7 new). Lint, type-tests, and build clean.The concurrency test uses a single nock interceptor with
disableNetConnect, so a second GET fails the test; verified it fails against the unfixedme().Not covered: no browser-level test that the two GETs actually stop serializing — the fix is asserted at the request-count level.
Noticed, not fixed
flush()'s beacon branch looks inverted:beaconRequestreturnstruewhen the beacon cannot be used, soif (!options.isBeacon || !beaconRequest(events))skips the axios fallback exactly when the beacon failed, and double-sends when it succeeded. Left alone as unrelated to this fix — worth its own look.🤖 Generated with Claude Code