Invalidate query cache on disconnect to fix stale auth tokens#839
Invalidate query cache on disconnect to fix stale auth tokens#839threepointone merged 6 commits intomainfrom
Conversation
…nnect When using async query functions with useAgent (e.g., for auth tokens), the cached result was reused on reconnect even if the token had expired. This caused auth failures because reconnects don't trigger cache key changes or wait for TTL expiration. By invalidating the cache entry in onClose, we ensure the async query is always re-executed on reconnect, fetching fresh tokens. Fixes #836
🦋 Changeset detectedLatest commit: 8d3bdb8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Invalidate query cache on disconnect to fix stale auth tokens
Claude Code ReviewIssue: Race condition when In The Problem:
The Race: Suggested Fix: // Track invalidation timestamp
const queryInvalidatedAtRef = useRef<number>(0);
// In onClose:
queryInvalidatedAtRef.current = Date.now();
deleteCacheEntry(cacheKeyRef.current);
// In the effect - only re-enable if query was fetched after invalidation:
useEffect(() => {
if (awaitingQueryRefresh && resolvedQuery !== undefined) {
const entry = getCacheEntry(cacheKey);
if (entry && entry.createdAt > queryInvalidatedAtRef.current) {
setAwaitingQueryRefresh(false);
}
}
}, [awaitingQueryRefresh, resolvedQuery, cacheKey]);This requires tracking cache entry creation time (not currently stored). Alternative: Key the effect off useEffect(() => {
if (awaitingQueryRefresh) {
setAwaitingQueryRefresh(false);
}
}, [cacheInvalidatedAt]);Test Coverage: The test at Otherwise solid implementation - good use of refs for cache key capture, comprehensive test coverage, and the core fix (cache invalidation on disconnect) is sound. |
Updates documentation to reflect that useAgent automatically invalidates the query cache when the WebSocket connection closes. This ensures that async query functions (e.g., for fetching auth tokens) are re-executed on reconnect, providing fresh tokens after network interruptions. Changes: - Added missing query, queryDeps, cacheTtl, and other parameters to UseAgentOptions type - Added dedicated section on authentication with async query functions - Added client-side authentication example in calling-agents.mdx - Documented automatic token refresh behavior on reconnection Related to cloudflare/agents#839
commit: |
- Add unit tests in cache-ttl.test.ts for: - Re-caching after deletion (reconnect scenario) - Graceful handling of non-existent key deletion - Cache isolation by key (different agents/instances) - Add integration tests in cache-invalidation.test.tsx for: - Verifying cache is not affected by static query objects - Cache key generation with different namespaces and deps - onClose callback is called on disconnect - Cache is invalidated before user's onClose callback - Successful reconnection after cache invalidation - Cache isolation when multiple components use different agent instances
Address two issues with cache invalidation on disconnect: 1. Cache key ref timing: The cache key was captured in the onClose closure at render time, which could cause the wrong cache entry to be invalidated if the component re-rendered with different props before onClose fired. Fix: use a ref (cacheKeyRef) that's updated via useEffect when cacheKey changes. 2. Re-render trigger: Just deleting the cache entry doesn't trigger a re-render, so PartySocket would reconnect with the old resolved query value. Fix: call setCacheInvalidatedAt(Date.now()) to bump state, which causes queryPromise useMemo to recompute and fetch fresh tokens before reconnecting. Test improvements: - Export createCacheKey in _testUtils so tests use the exact same key generation as useAgent internally - Update tests to use createCacheKey instead of hardcoded JSON - Add test for the ref timing scenario (name changes before disconnect)
Update cacheKeyRef synchronously during render instead of in useEffect. The useEffect runs asynchronously after render, creating a window where onClose could fire before the effect updates the ref, causing the wrong cache entry to be invalidated. Refs can be safely updated during render when storing derived values.
packages/agents/src/react.tsx
Outdated
| // Invalidate the query cache and trigger re-render so reconnection will | ||
| // re-run the async query with fresh tokens. We must both: | ||
| // 1. Delete the cache entry so getCacheEntry() returns undefined | ||
| // 2. Bump cacheInvalidatedAt to trigger useMemo to recompute queryPromise | ||
| // Without the state bump, the component keeps the old resolved query value | ||
| // and PartySocket would reconnect with stale query params. | ||
| deleteCacheEntry(cacheKeyRef.current); | ||
| setCacheInvalidatedAt(Date.now()); |
There was a problem hiding this comment.
Main remaining concern is a race - the socket’s reconnect loop can fire with the old URL/query before the re‑render (triggered by cache invalidation + state bump) completes. The only obvious way I see to eliminate the race is to pause reconnects while the query refresh happens.
Looks like we can use enabled (UsePartySocketOptions) to handle this? So a pattern like:
- onClose: setEnabled(false) + invalidate cache + bump state
- after query resolves / re-render: setEnabled(true)
Idea being to guarantee no reconnect attempt happens before the new query is ready.
There was a problem hiding this comment.
yes, that's valid, we might just do that in the end
|
Could we add an integration test where the query returns a different token on each invocation, then assert which token appears on the first reconnect? Probably makes sense to make it intentionally race‑y by:
Then assert which token the first reconnect used: Without that timing pressure, the test could pass even if a race still exists, simply because the reconnect happens after the refresh render completes. This would validate the behavior the PR claims (fresh token on reconnect), beyond just cache deletion. |
Prevents race where PartySocket reconnects with stale query params before React re-renders with fresh tokens.
|
@aurewill-gavel give the latest version a go, please, if there will be more gaps, we will revisit |
|
@whoiskatrin Logic looks sound. That test I mentioned would be ideal but obv your decision whether or not to add it. |
|
let's land this, I suspect suspense might help out here a bit. and because it's not an api change, we can revisit and make it stronger if we need to. |
Summary
Problem
When using
useAgentwith an asyncqueryfunction for authentication:The cached query result was reused on reconnect even if the token had expired. Reconnects don't change the cache key or wait for TTL expiration, so stale tokens were sent to the server causing auth failures.
Solution
By calling
deleteCacheEntry(cacheKey)in theonClosehandler, we invalidate the cache on disconnect. When the socket reconnects, the async query will be re-executed to fetch a fresh token.This is simpler than adding new API options (
queryCacheMode,alwaysRefreshQuery, etc.) and provides the correct behavior by default - reconnects should always use fresh auth data.Fixes #836