Skip to content

Fix Apollo store-reset race when a scan/clean task completes - #7093

Draft
ordureconnoisseur wants to merge 2 commits into
stashapp:developfrom
ordureconnoisseur:fix-resetstore-race-condition
Draft

Fix Apollo store-reset race when a scan/clean task completes#7093
ordureconnoisseur wants to merge 2 commits into
stashapp:developfrom
ordureconnoisseur:fix-resetstore-race-condition

Conversation

@ordureconnoisseur

@ordureconnoisseur ordureconnoisseur commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Description

createClient.ts calls client.resetStore() when the scan/clean-complete subscription fires. resetStore() is cache.reset() + reFetchObservableQueries(), but the cache.reset() step also cancels any query still in flight, rejecting it with "store reset while query was in flight" (Apollo error code 42). If a scan finishes while a list page is loading, this flashes an "Error loading items" screen for about a second until resetStore's own follow-up refetch lands and the list corrects itself.

Fix: keep calling client.resetStore() unchanged (full Apollo semantics, including its in-flight cancellation safety net), but only once no watched query is loading. When the subscription fires, the client checks its active queries (via the public, non-deprecated getObservableQueries() and getCurrentResult()) every 100ms and resets once nothing is in flight. The idle check and the resetStore() call run back to back in the same task; the only remaining gap is the single microtask resetStore() itself defers by internally (Promise.resolve().then(...)) before cancelling anything. A watched query can only start from a task (React renders and schedules in tasks), so in practice nothing can slip in between; a query started imperatively from a promise continuation in that exact microtask would get today's cancellation behavior, nothing worse.

Two bounds keep this safe:

  • A 10 second deadline: if queries are still in flight after that (sustained slow network, etc.), reset anyway. That path cancels in-flight queries exactly like today, so the worst case is the current behavior, just rarer.
  • Resets are serialized and coalesced: several task completions in quick succession produce one reset instead of overlapping resetStore() calls, which could previously cancel each other's refetches and cause the same flash.

Two scope notes:

  • An earlier revision of this PR called cache.reset({ discardWatches: false }) + reFetchObservableQueries() directly, which skips Apollo's cancelPendingFetches safety net and left a residual risk if a custom incremental merge function is ever added to typePolicies. This version has no such tradeoff; resetStore() is used as-is.
  • The idle check only sees watched queries. A one-off client.query() in flight at reset time gets the same cancellation it gets today; the flash bug specifically comes from watched list queries, which this covers.

Related Issue

Same stack trace as #6458 (closed, root cause not found there, reporter's actual fix was removing a plugin that triggered scans more often).

Testing

  • pnpm run check (tsc), biome lint, biome format all pass.
  • Verified behavior with a harness against @apollo/client 3.14 itself (the version in this repo), using a link with controllable response delay and a watched query simulating a mounted list page:
    • Baseline: resetStore() mid-flight reproduces the identical error (message 42).
    • New logic, event mid-flight: the query resolves normally with data, no error is delivered, and the reset plus its refetch run after the query settles.
    • Full-cache-wipe parity with resetStore(): a watched query that resolved and was then unsubscribed (simulating navigating away from a list) has its cache entry wiped by the reset, confirmed via cache.extract().
    • Deadline fallback: a query outlasting the deadline gets cancelled by the reset at the deadline, i.e. the old behavior, and the reset still completes.
    • Coalescing: three completion events during one wait produce a single reset.
    • Serialization: an event arriving after a reset has started queues a second reset rather than being dropped, and no refetch is cancelled by an overlapping reset.

Screenshots

No visual change.

Checklist

  • I have read and understood the Contributing document.
  • I have read and understood the AI Usage Policy document.
  • I have made corresponding changes to the documentation (if applicable).

AI Usage Disclosure

  • I have used AI tools to assist with this pull request, and I have disclosed the tools and how I used them below.

Used Claude Code to investigate the error (decoded the Apollo error link to error code 42, traced it to the resetStore() call in createClient.ts) and to explore fix options. An earlier revision replaced resetStore() with a direct cache.reset(), which traded away Apollo's in-flight cancellation safety net; I rejected that tradeoff, and this revision (wait for watched queries to settle, then call resetStore() unchanged) came out of that iteration. The behavioral tests in the Testing section were run with Claude Code against my own instance's dependency tree. I reviewed the diagnosis, the approach, and the diff, and take responsibility for the change.

@ordureconnoisseur
ordureconnoisseur marked this pull request as draft July 1, 2026 10:29
@ordureconnoisseur
ordureconnoisseur force-pushed the fix-resetstore-race-condition branch from 9564588 to 7d2df41 Compare July 1, 2026 10:34
client.resetStore() is cache.reset() + reFetchObservableQueries(), but
the cache.reset() step also cancels any query still in flight,
rejecting it with "Store reset while query was in flight". If a scan
or clean task finishes while a list page is loading, this briefly
flashes an "Error loading items" screen (Apollo error code 42) before
resetStore's own follow-up refetch lands and the list corrects itself.

Call cache.reset() directly instead (the same method clearStore() uses
internally), then reFetchObservableQueries(). This keeps the same
full-cache-wipe behavior resetStore() had, so no stale data lingers for
screens the user isn't currently on, without the in-flight cancellation
that causes the flash.

Tradeoff: this skips cancelPendingFetches, a general Apollo safety net
against custom merge functions assuming stale cache state. Not
exploitable in this codebase today (no custom incremental merge
functions in typePolicies, and plugins can't add any), but worth
flagging as a real, if currently theoretical, cost of this approach.
@ordureconnoisseur
ordureconnoisseur force-pushed the fix-resetstore-race-condition branch from 7d2df41 to d6b0b7e Compare July 1, 2026 10:54
@ordureconnoisseur
ordureconnoisseur marked this pull request as ready for review July 17, 2026 11:04
@ordureconnoisseur
ordureconnoisseur marked this pull request as draft July 17, 2026 11:11
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