fix(analytics): skip the identity lookup when no token is set by guyofeck · Pull Request #255 · base44/javascript-sdk

guyofeck · 2026-08-17T16:53:45Z

Problem

On a public page, the SDK issues a GET /entities/User/me that can only ever answer 401, and the browser writes Failed to load resource: the server responded with a status of 401 (Unauthorized) to the console. Users read that as a broken app. (reported here — app_id 69f14bfdfba5c17e67d8e81b, where the user won't ship while there are console errors.)

The request comes from analytics, not from the app. createAnalyticsModule runs at client construction → trackInitializationEvent queues an event → the processor flushes the first batch immediately → flush awaits getSessionContext → auth.me(), unconditionally, whether or not a session exists.

Catching it harder does not help, and that is the main thing to know about this PR. The rejection is already handled at three levels:

Location
`analytics.ts:343`	`.catch(() => ({ user_id: null, session_id }))`
`analytics.ts:121`	`try { ... } catch { /* do nothing */ }` in `flush`
`axios-client.ts:261`	rejects with `Base44Error`, consumed by the above

Nothing escapes as an unhandled rejection. The console line is emitted by the browser's network stack before any JS handler runs, so no try/catch can suppress it. The only way to remove it is to not send the request.

Fix

Skip the identity lookup when no access token is set:

// analytics.ts, getSessionContext
if (!userAuthModule.hasToken()) {
  return { user_id: null, session_id: getAnalyticsSessionId() };
}

Anonymous events already reported user_id: null through the existing .catch, so no analytics data changes — the request is dropped only in the case where its sole possible outcome was a 401.

Token presence is tracked in the auth module rather than read off axios.defaults, so it follows the identity transitions that already live there (setToken, logout) instead of a header a caller may have set on the instance directly. It is seeded from the constructor token, which is how the server-side SDK carries a session it never sets explicitly — without that seeding this would have silently stopped resolving user_id in backend functions.

The skip is deliberately not memoized: a visitor who loads a page anonymously and then logs in has to start resolving an identity again.

This does not revert or weaken #245

#245 removed a duplicate User/me on authenticated cold loads by sharing the in-flight promise in auth.me(). That optimization is untouched and still exercised: when a token is present, analytics resolves through auth.me() exactly as before and shares the app's in-flight request.

Scenario	Before #245	After #245 (main)	This PR
Public page, no token	1 request → 401 in console	1 request → 401 in console	0 requests, no console error
Authed cold load, app calls `me()`	2 requests (serialized)	1 shared request	1 shared request
Authed, app never calls `me()`	1 request	1 request	1 request
Backend function (server-side SDK)	1 request	1 request	1 request

The two PRs address different halves of the same endpoint: #245 cut the redundant request for logged-in users, this one cuts the impossible request for anonymous ones.

Scope — what this does not fix

A stale or expired token in localStorage still produces one 401. A token is present, so the request goes out; its validity isn't knowable client-side. Removing that one requires User/me to answer 200 with a null user instead of 401 — a backend change, not this repo. If the goal is "zero 401s on /User/me under any condition", that is the change to make, and this PR is not a substitute for it.

Also unchanged: [Base44 SDK Error] 401: ... from safeErrorLog (axios-client.ts:255) is already gated behind process.env.NODE_ENV !== "production" and is stripped from prod builds. Worth confirming published apps are served as production builds, otherwise that line appears alongside the browser's.

As with #245, the SDK is pinned at ^0.8.x in the app templates, so apps pick this up on their next install or rebuild — the deployed fleet trails.

Testing

npm run lint, npm run build, npm run test:types clean. npm run test:unit — 204 pass (3 new).

New tests:

analytics does not call me() when no token is set
resolution resumes after setToken (guards the not-memoized behavior)
hasToken() tracks setToken → logout

Verified the first genuinely fails with the gate disabled, rather than passing vacuously.

Two existing tests changed, and the reason is worth a look rather than a rubber stamp: should not restore the pre-reset identity when a lookup settles late and should track multiple events both exercise the identity-resolution path, so their client is now token-bearing. With the token restored, their batching and throttle assertions pass unchanged — which is what rules out a regression in the processor loop, since removing an await from flush shifts microtask ordering. The file's axios mock also gains the defaults object that setToken/logout have always written through.

Not covered: no browser-level test asserting the console stays clean; the fix is asserted at the request level.

Generated by Claude Code

On a public page the analytics module still issued a `User/me` at client construction: `trackInitializationEvent` queues an event, the processor flushes the first batch immediately, and `flush` awaits `getSessionContext`, which calls `auth.me()` unconditionally. With no session that request can only answer 401, and the browser logs it to the console from the network layer -- before any handler in the SDK sees the rejection. The existing `.catch` in `getSessionContext` and the try/catch in `flush` already keep the rejection from escaping, so no amount of additional catching removes what users actually see. Skip the lookup when no access token is set. Anonymous events already reported `user_id: null` through that same `.catch`, so no analytics data changes -- the request is dropped only in the case where its sole possible outcome was a 401. The dedupe from #245 is unaffected: when a token is present, analytics still resolves through `auth.me()` and shares the app's in-flight request, so an authenticated cold load continues to issue exactly one `User/me`. Token presence is tracked in the auth module rather than read off `axios.defaults`, so it follows the identity transitions that already exist there (`setToken`, `logout`) and is seeded from the constructor token, which is how the server-side SDK carries a session it never sets explicitly. The skip is deliberately not memoized: a visitor who logs in mid-session must start resolving an identity again. This does not cover a stale or expired token, which still 401s. Removing that one needs `User/me` to answer 200 with a null user instead of 401, which is a backend change. Tests: the two analytics tests that exercise identity resolution now build a token-bearing client; the axios mock gains the `defaults` that `setToken` and `logout` write through. Three new tests cover the anonymous skip, resolution resuming after `setToken`, and token presence across identity changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtKFfzaVR6nWAi8tNvJqfw

claude · 2026-08-17T16:54:00Z

Claude finished @guyofeck's task —— View job

Claude Code Review

Reviewed all 5 changed files. This is a clean, well-reasoned PR. The extensive commit message and inline comments match the actual behavior, and the fix is correctly scoped. Below is my feedback.

Summary

The fix is correct and minimal. getSessionContext now returns { user_id: null, session_id } early when hasToken() is false, avoiding a guaranteed-401 User/me on public pages. Token presence is tracked in the auth module's closure and seeded from options.token, so it correctly follows setToken/logout transitions and preserves the server-side SDK path. This does not weaken #245 — the shared in-flight me() promise is untouched.

Strengths

Right layer for the fix. Skipping the request is the only way to remove the browser-emitted console 401; the PR body correctly notes no try/catch can suppress it. Good problem framing.
hasToken() seeded from options.token (auth.ts:114) is the subtle-but-important detail — without it, backend functions that pass a token via the constructor (never calling setToken) would have silently stopped resolving user_id. Nicely handled and documented.
Not memoizing the skip (analytics.ts:339, returns before the sessionContextPromise memo) is the correct choice: an anonymous visitor who logs in mid-session resumes identity resolution. Guarded by a dedicated test.
No analytics data change — anonymous events already reported user_id: null via the existing .catch, so this only drops the impossible request.
@internal on hasToken() keeps it out of the public API surface. Good.
Test quality is high — the note about verifying the new test fails with the gate disabled (rather than passing vacuously) is exactly the right rigor.

Observations / Minor Points

setToken("") early-return vs. hasAccessToken (auth.ts:227). setToken returns early on a falsy token before setting hasAccessToken = true, so a no-op call can't flip the flag on — good. But note the flag is only ever set false by logout(), not by any "clear token" path, since setToken rejects empty strings. That's consistent with existing behavior, just worth being aware of: there's no way to transition hasToken() back to false without a full logout(). Not a bug given current usage.
hasToken() reflects presence, never validity — clearly documented (auth.types.ts:130-138) and the PR body's "Scope" section is honest that a stale/expired token still yields one 401. Good to see this called out rather than overclaimed.
Consistency with client.ts. The client already had the setToken guard (client.ts:161-166) ensuring the token is applied before analytics constructs. Adding token to the AuthModuleOptions seed (client.ts:153) complements that well — now even the non-browser path (no setToken call) reports hasToken() === true. Consistent and correct.
Test mock defaults object (analytics.test.ts:29-31). Adding defaults.headers.common per-instance to the axios mock is necessary now that setToken/logout write through it. Correct fix; the two changed existing tests becoming token-bearing is well-justified in the PR body and, importantly, their batching/throttle assertions pass unchanged — which rules out a microtask-ordering regression from any await change.

Bugs / Security / Performance

Bugs: None found. The early-return path returns the same shape as the .catch fallback, so transformEventDataToApiRequestData receives a valid SessionContext.
Security: No concerns. No token is logged or exposed; hasToken() returns only a boolean.
Performance: Net positive — one fewer network request per anonymous page load, and the authenticated path is unchanged.

Test Coverage

Solid. Three new tests cover the three behaviors that matter:

me() not called without a token
resolution resumes after setToken (guards non-memoization)
hasToken() tracks setToken → logout

The one gap, acknowledged in the PR, is that there's no browser-level assertion that the console stays clean — the fix is verified at the request level, which is the appropriate boundary for a unit test here.

Verdict

Looks good to merge. The change is small, correct, well-tested, and thoroughly documented. No blocking issues.

--- • claude/message-latest-pr-caqv4d

github-actions · 2026-08-17T16:54:13Z

🚀 Package Preview Available!

Install this PR's preview build with npm:

npm i @base44-preview/sdk@0.8.42-pr.255.c035d2f

Prefer not to change any import paths? Install using npm alias so your code still imports @base44/sdk:

npm i "@base44/sdk@npm:@base44-preview/sdk@0.8.42-pr.255.c035d2f"

Or add it to your package.json dependencies:

{
  "dependencies": {
    "@base44/sdk": "npm:@base44-preview/sdk@0.8.42-pr.255.c035d2f"
  }
}

📦 Preview Package: @base44-preview/sdk@0.8.42-pr.255.c035d2f
🔗 View this commit on GitHub

_{Preview published to npm registry — try new features instantly!}

guyofeck · 2026-08-17T16:56:26Z

Two checks are red, and neither is caused by this diff. Recording the diagnosis so it isn't re-investigated at review time.

npm audit (dependencies) — the registry rejects the request itself:

npm warn audit 400 Bad Request - POST https://registry.npmjs.org/-/npm/v1/security/audits/quick
{ statusCode: 400, error: 'Bad Request', message: 'Invalid request payload JSON format' }
npm notice This endpoint is being retired. Use the bulk advisory endpoint instead.

This reproduces on #254, a CI-only PR that touches no source files, so it predates and is independent of this change. It's the failure mode anticipated in #248's checklist — npm audit POSTs through the pinned gateway, and the legacy quick-audit endpoint isn't being proxied successfully. This PR adds no dependencies and package-lock.json is untouched.

claude-docs-draft — the job died in bun install --production before running any analysis:

error: GET https://registry.npmjs.org/@anthropic-ai/claude-agent-sdk/-/claude-agent-sdk-0.3.228.tgz - 403

The gateway is refusing that tarball, consistent with the 14-day min-release-age cooldown from #248 applying to a recent release of the action's own dependency. No JSDoc was ever evaluated, so the failure carries no signal about the docs in this PR.

Both need workflow-level changes (proxying the audit endpoint or moving to the bulk endpoint; allowing or pinning the agent-sdk tarball). I've deliberately left them alone rather than editing workflow files from a behavioral PR — they belong in their own change, alongside the other #248 follow-ups.

The checks that do exercise this diff — lint, test, publish-preview, claude-review — are green.

Generated by Claude Code

Review feedback on #255: despite @internal, hasToken() landed in the published .d.ts (tsc does not strip @internal members without stripInternal), so every consumer saw it on base44.auth. Move it to InternalAuthModule, an extension of AuthModule that only the factory and the analytics module reference. It is not exported from the package index, and Base44Client.auth stays typed as AuthModule, so the runtime object still carries the method but the public type surface is unchanged from before this PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MtKFfzaVR6nWAi8tNvJqfw

claude · 2026-08-18T07:51:23Z

Claude finished @guyofeck's task —— View job

Claude Code Review

Read changed files (incl. latest InternalAuthModule refactor)
Review code quality & best practices
Check for bugs/edge cases
Assess performance & security
Evaluate test coverage

Summary

Re-reviewing after the InternalAuthModule refactor (79195b0) that addressed @yardend-wix's comment. The change is correct, minimal, and well-tested. getSessionContext returns { user_id: null, session_id } early when hasToken() is false (analytics.ts:339), avoiding a guaranteed-401 User/me on public pages. The refactor cleanly resolves the public-API concern.

The `InternalAuthModule` refactor is right

hasToken() moved to InternalAuthModule extends AuthModule (auth.types.ts:561-570). Confirmed the narrowing is sound:

createAuthModule returns InternalAuthModule (auth.ts:95), so the factory and analytics (AnalyticsModuleArgs.userAuthModule: InternalAuthModule, analytics.ts:61) see hasToken().
client.types.ts:101 types auth: AuthModule, so the public surface is unchanged — exactly what @guyofeck described. The test at analytics.test.ts:174 deliberately casts to InternalAuthModule to reach it, which documents the boundary well.

This is the correct pattern: an internal-only method on a subtype, with the public client narrowed back down. No stripInternal needed.

Strengths

Right layer for the fix. Skipping the request is the only way to remove the browser-emitted console 401 — no try/catch can suppress it. Good framing.
hasAccessToken seeded from options.token (auth.ts:114) preserves the server-side SDK path, where a constructor token is never followed by setToken. Without it, backend functions would have silently stopped resolving user_id.
Not memoized (analytics.ts:334-341): the early return sits inside the !sessionContext block but before the sessionContextPromise memo, so an anonymous visitor who logs in later resumes resolution. Guarded by the should resolve an identity once a token is set test.
setToken guard order (auth.ts:227 returns before hasAccessToken = true): a setToken("") no-op cannot flip the flag on. Correct.
No analytics data change — anonymous events already reported user_id: null via the existing .catch, so only the impossible request is dropped.

Observations / Minor Points

No path clears hasAccessToken short of logout(). setToken rejects falsy tokens, so the flag only goes true → false via logout(). logout() early-returns most of its body outside a browser (auth.ts:204) but still sets hasAccessToken = false first (auth.ts:201), so the flag is correct in both environments. Consistent with existing behavior; not a bug given current usage.
hasToken() reflects presence, never validity — clearly documented (auth.types.ts:565-568) and the PR's "Scope" section is honest that a stale/expired token still yields one 401. Good to see the limitation stated rather than overclaimed.
Test mock defaults (analytics.test.ts:29-31) is now required per-instance since setToken/logout write through axios.defaults.headers.common. The two existing tests becoming token-bearing is well-justified, and their batching/throttle assertions passing unchanged is what rules out a microtask-ordering regression from the await in flush.

Bugs / Security / Performance

Bugs: None found. The early-return returns the same SessionContext shape as the .catch fallback, so transformEventDataToApiRequestData gets a valid value.
Security: No concerns — hasToken() returns only a boolean; no token is logged or exposed.
Performance: Net positive — one fewer request per anonymous page load; authenticated path (and the fix(auth): share the in-flight me() request #245 shared-me() optimization) untouched.

Test Coverage

Solid. Three behaviors covered: me() not called without a token, resolution resumes after setToken (guards non-memoization), and hasToken() tracks setToken → logout. The acknowledged gap — no browser-level assertion that the console stays clean — is the right boundary for a unit test.

Verdict

Looks good to merge. The InternalAuthModule refactor is the correct response to the review feedback and keeps the published AuthModule type unchanged. Small, correct, well-tested, thoroughly documented. No blocking issues.

--- • • claude/message-latest-pr-caqv4d

guyofeck requested a review from yardend-wix August 18, 2026 07:26

yardend-wix reviewed Aug 18, 2026

View reviewed changes

Comment thread src/modules/auth.types.ts Outdated

yardend-wix approved these changes Aug 18, 2026

View reviewed changes

guyofeck merged commit fa75a45 into main Aug 18, 2026
6 of 8 checks passed

Navigation Menu

Uh oh!

fix(analytics): skip the identity lookup when no token is set - #255

fix(analytics): skip the identity lookup when no token is set#255
guyofeck merged 2 commits into
mainfrom
claude/message-latest-pr-caqv4d

guyofeck commented Aug 17, 2026

Uh oh!

claude Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Aug 17, 2026 •

edited

Loading

Uh oh!

guyofeck commented Aug 17, 2026

Uh oh!

Uh oh!

claude Bot commented Aug 18, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

guyofeck commented Aug 17, 2026

Problem

Fix

This does not revert or weaken #245

Scope — what this does not fix

Testing

Uh oh!

claude Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

Strengths

Observations / Minor Points

Bugs / Security / Performance

Test Coverage

Verdict

Uh oh!

github-actions Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🚀 Package Preview Available!

Uh oh!

guyofeck commented Aug 17, 2026

Uh oh!

Uh oh!

claude Bot commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude Code Review

Summary

The InternalAuthModule refactor is right

Strengths

Observations / Minor Points

Bugs / Security / Performance

Test Coverage

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude Bot commented Aug 17, 2026 •

edited

Loading

github-actions Bot commented Aug 17, 2026 •

edited

Loading

claude Bot commented Aug 18, 2026 •

edited

Loading

The `InternalAuthModule` refactor is right