Skip to content

Cut redundant requests: Sentry sessions, /api/me, missing-property lookups - #2207

Closed
joepio wants to merge 4 commits into
developfrom
claude/project-thread-98bwy8
Closed

joepio wants to merge 4 commits into
developfrom
claude/project-thread-98bwy8

Conversation

@joepio

@joepio joepio commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Requested by Joep · project thread

Before: a Playwright recording of production (app.atomic.place guest demo) showed three kinds of wasted requests. Every in-app navigation sent two Sentry session POSTs. A signed-out visitor called atomic.place/api/me about 9 times in the first 25 s and then every few seconds for as long as the page stayed open. The demo fetched atomicdata.dev/properties/demo/speaker (a 404) 12+ times a minute.

After: one Sentry session per page load; the session check is asked about once per 30 s window and again when something could have changed; a missing external property is asked for once, and the demo speaker property is bundled so it is never asked for.

What changed, one commit each:

  1. Sentry: browserSessionIntegration({ lifecycle: 'page' }).
  2. getManagedAccount serves the last settled answer for 30 s. It forgets it on focus, pageshow, visibility and when a sign-in lands in this tab (noteManagedSessionChanged); callers that wait for a sign-in read fresh. useHostedAI no longer runs its timed retry loop with no session, and the vault backup scheduler drops its queue when signed out instead of re-asking on every flush.
  3. Db (Rust/WASM store) remembers a 404/410 for the session and no longer fetches the same URL a second time through handle_not_found; 5xx and network errors are still retried. demo/speaker is added to lib/defaults/chatroom.json.

How: not all of it ran in the cloud container. The new Rust tests and the new store-not-found-once vitest passed; the TS changes for 1 and 2 (session, useHostedAI, vault backup, sign-in paths) are untested because node_modules could not be installed, so CI needs to run session.test.ts, useHostedAI.test.tsx, vaultAutoBackup.test.ts, agentSession.test.ts and accountPasskey.test.ts. The lifecycle option name is from memory of the v10 SDK. One atomic_lib test, db::compaction::tests::startup_compaction_shrinks_a_bloated_store_and_keeps_every_resource, failed in the sub-agent's full run and was not checked against a clean checkout.

@joepio joepio self-assigned this Oct 10, 2026
…flush

A signed-out visitor called /api/me about nine times in the first 25 s and
then every few seconds for as long as the page stayed open: the hosted-AI
status retried, the vault backup scheduler re-queued, and every other caller
asked again. getManagedAccount now serves the last settled answer for 30 s,
forgets it on focus and when a sign-in lands in this tab, and callers that
wait for a sign-in read fresh.
… property

A property the remote answers 404 for was fetched again on every put of a
message that carries it, twice per miss (the fetch, then handle_not_found
fetching the same URL). Db now remembers a definitive 404/410 for the
session and no longer retries it; 5xx and network errors are still retried.
The demo speaker property is added to the bundled defaults so it resolves
locally and is never asked for.
@joepio joepio changed the title Report one Sentry session per page load Cut redundant requests: Sentry sessions, /api/me, missing-property lookups Oct 10, 2026

joepio commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Complexity and risk review (19 files, +462/-47, mostly tests and plumbing; I read the patches, not the surrounding session.ts, store.ts or the rest of db.rs; nothing was run).

Please change before merge

  1. remote_not_found in db.rs (~5212) never expires or resets. A property that 404s once stays "missing" for the life of the Db even if it is created later or the bundled ontology changes; only a reload recovers. Give it a short TTL (5-10 minutes) or clear it on an explicit refresh/reconnect.
  2. Sentry option name. The PR says browserSessionIntegration({ lifecycle: 'page' }) is "from memory", and the unit test mocks the integration so it cannot catch a wrong name. Confirm the name and that passing an integrations array does not drop the SDK defaults, against the installed @sentry/react version.
  3. Transient errors cached for 30 s? If fetchManagedAccount returns null on a network error, the new /api/me cache turns a blip into "signed out" in the UI for 30 s. Do not cache the error case.
  4. The PR says the TS Store "remembers" 404s, but the only TS change in lib is store-not-found-once.test.ts. Say what that test actually exercises (a regression test of existing behaviour, or a gap).

Simplification (optional)

  • session.ts:33-100 now has settled, inFlight, sessionGeneration and a listener Set, and onManagedSessionChanged is a near-copy of onManagedLogout; merge the two registries.
  • noteManagedSessionChanged() is called manually from 7 sign-in paths (a future path will forget it), and cloudSync and AccountSignInPanel use both it and fresh: true. Prefer invalidating inside the one place the session is established.

CI note: the PR body says the five TS test files were not run and an unrelated compaction test failed; that is for CI to settle, not me.


Generated by Claude Code

…r registry

Review of the redundant-requests PR: a property that 404ed once stayed
missing for the life of the Db. The memory now expires after 5 minutes.
Also proves a failed session read is not cached, merges the two listener
sets in session.ts, and states what the not-found-once test covers.

joepio commented Oct 10, 2026

Copy link
Copy Markdown
Member Author

Addressed in 3453276:

  1. remote_not_found now expires after 5 minutes (REMOTE_NOT_FOUND_TTL, using web_time::Instant as search/mod.rs does, so wasm32 stays safe). New test an_expired_404_is_fetched_again.
  2. The option name is confirmed from the production bundle (app.atomic.place, release 0.41.0-beta.7): browserSessionIntegration reads e.lifecycle ?? 'route' and registers as BrowserSession. Passing it in integrations replaces only that same-named default; the other SDK defaults stay. tsc in CI is still the final check of the types.
  3. Errors are not cached: only the success path of getManagedAccount stores settled, so a network failure or a non-ok status rejects and the next call asks again. Two new tests in session.test.ts (rejected fetch, 503) prove it.
  4. store-not-found-once.test.ts is a regression test for existing behaviour (an errored Resource stays in store.resources, so a 404 is fetched once). There is no TS source change; the header comment now says so.

Optional: the two listener sets in session.ts now share one small listenerRegistry() helper. I left the seven noteManagedSessionChanged() calls as they are: there is no single place where a session is established (passkey, email link, agent sign-in, device link and portal polling each create it differently), so a central hook would need the same seven call sites anyway.

Not run in the cloud container: the vitest files (no node_modules). Rust remote_not_found (3) and populate (8) tests pass.


Generated by Claude Code

joepio commented Oct 11, 2026

Copy link
Copy Markdown
Member Author

Landed in develop through the batch #2210 (squash f39e460), tested together on Mancave run 5204.


Generated by Claude Code

@joepio joepio closed this Oct 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant