Skip to content

feat: recover hook state and query caches after a backend restart - #202

Open
gmaclennan wants to merge 17 commits into
mainfrom
feat/backend-restart-recovery
Open

feat: recover hook state and query caches after a backend restart#202
gmaclennan wants to merge 17 commits into
mainfrom
feat/backend-restart-recovery

Conversation

@gmaclennan

@gmaclennan gmaclennan commented Aug 13, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

When the backend process is killed and restarted (Android low-memory kills of the foreground service), the transport layer can reconnect and resubscribe — but this library's caches don't know anything happened: queries hold data from the dead process, the sync store's useSyncExternalStore snapshot freezes or wedges on a sticky error, and in-memory share state hangs. This PR gives the host app one entry point to fix that, plus the error-handling that makes the disconnect window itself survivable.

API

ComapeoCoreProvider accepts subscribeToBackendRestart, which the host calls after the transport has reconnected and resubscribed (comapeo-core-react-native exposes exactly this signal — digidem/comapeo-core-react-native#226). On the notification, resetQueriesAfterBackendRestart runs four steps: remove the queries whose data died with the process (everything below a project instance, plus the media-server origin, whose staleTime: 'static' makes it structurally un-invalidatable); reset the project-instance queries so mounted observers suspend and refetch; invalidate the manager-level rest — which also re-pulls invites, whose backend state machines are in-memory and silently dropped by a restart; and refresh every sync store with an active subscription.

That last step exists because of @comapeo/ipc v10 (digidem/comapeo-ipc#88): project references are now permanent, so the v9-era accidental recovery — new instance ⇒ new SyncStore via the WeakMap — is gone. Stores register while subscribed; refresh drops the stale state and per-device progress baselines (the snapshot honestly reports null rather than flashing 100%), clears any sticky error, and re-reads $sync.getState(). Sticky errors are also now scoped to the subscription that raised them — cleared on re-attach and on last-unsubscribe — which fixes a wedge where an error thrown from getStateSnapshot unmounted the subtree, unregistered the store, and then greeted every error-boundary retry with the same stale throw.

Transport errors, retry, and left projects

Queries retry on RPC_CHANNEL_CLOSED (the code rpc-reflector actually ships) — bounded, 3 retries at 1 s, queries only; mutations keep retry: false, since a mutation in flight at a process death is at-most-once and must surface, not repeat. PROJECT_LEFT is explicitly never retried. Note the matching is by code string deliberately: errors deserialize across the IPC boundary as plain Errors, so instanceof is not just fragile, it is wrong.

Map shares

The SSE monitor now has an error path: a stream that stays down for a ~60 s outage budget (summed from the client's reported reconnect delays; any delivered message resets it) transitions the share to error instead of hanging in downloading forever — and error is no longer absorbing: download() may retry it. In-memory share state received before a restart is not recoverable (the backend neither persists nor re-serves it); that loss is documented rather than papered over.

Compatibility

Peer range is ^9.0.0 || ^10.0.0. The suite passes against both (112 tests: v9 via the lockfile's npm ci state, v10 via locally packed tarballs of the #88 branch); on v9 the new machinery is simply inert where v9 lacks the signals. The invite-rejoin regression test now pins re-join recovery through a direct call on the reference the hooks hand out, which holds under both majors' semantics.

@gmaclennan
feat: recover query caches after a backend restart
30db490 
Adds an optional `subscribeToBackendRestart` prop to `ComapeoCoreProvider`.
When the listener fires, every query under this package's shared key prefix is
invalidated, so per-project API instances bound to the dead backend are
re-fetched instead of being served from a cache that never expires.

Also fixes a `map-share` listener leak: the received map shares store attached
its client API listener at creation and never removed it, so recreating the
store orphaned the previous listener. The listener is now attached from an
effect and removed on cleanup.
@gmaclennan gmaclennan mentioned this pull request Aug 13, 2026
Open
@gmaclennan
fix: tolerate a closed project wrapper in sync teardown
01fced6 
`SyncStore` attaches and removes its `sync-state` listener on the project
wrapper it was built from. A backend restart closes that wrapper, and the
removal runs in React effect cleanup, where a throw takes down the tree.
Upstream `@comapeo/ipc` is being changed so that `off` on a closed wrapper is a
no-op, but this package still supports the versions that throw.
@gmaclennan
fix: make the received map shares listen() idempotent
dfb2e2c 
A second `listen()` on the same store registered the client API listener twice,
so every incoming share was added to the store twice. It now returns the
existing teardown, and a teardown lets a later `listen()` attach again.

Also notes at `monitor()` that the download event source has no error path, so
a transport failure leaves the share stuck in `downloading`.
@gmaclennan
fix: remove dead project queries on restart instead of invalidating
ae9042c 
Invalidating everything under the shared key prefix does not recover the app.
Project-derived queries (settings, members, documents) refetch immediately with
a `queryFn` still closed over the project instance from the backend that went
away; those calls reject, and a `useSuspenseQuery` with `retry: false` then
latches into `status: 'error'`, which `shouldFetchOptionally` in query-core
never retries. Queries cached with `staleTime: 'static'` are excluded from
invalidation altogether, so the media server origin — whose port is ephemeral —
kept every image URL pointing at a dead port for the life of the app.

The restart listener now resets in three steps. It removes every query read
through a project instance, including the static media server origin, so they
cannot refetch with a dead closure. It then resets the cached project instances
themselves: removal alone is invisible to a mounted observer, which keeps
rendering its last result, whereas a reset suspends the component so it fetches
a fresh instance and re-runs the removed queries against it. Finally it
invalidates the remainder — device info, invites, the project list — which are
read through the client API that survives the restart, so a background refetch
is enough.

The subscribe effect now depends only on the subscribe function, with the query
client parked in a ref, and `SubscribeToBackendRestart` is exported so its
contract (including that it should be referentially stable) documents itself.
The README describes what the reset actually does and what it does not cover.
@gmaclennan

gmaclennan commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Reworked in ae9042c, dfb2e2c and 01fced6. The probes were right on all three counts, and I reproduced each one before changing anything.

The invalidate was doing the wrong thing. With a mock client API handing out generation-tagged project instances that reject once the generation is bumped, invalidating the root prefix leaves project_settings in status: 'error' with ProjectClosed — refetched with the pre-restart closure, and never retried because shouldFetchOptionally bails on an errored suspense query. media_server_origin stays success on the dead port, exactly as described.

But removeQueries on its own does not recover either, which was the surprise. A mounted observer is never notified when its query is removed: queryCache.remove calls query.destroy(), which dispatches nothing, and useBaseQuery only subscribes to the observer, not the cache. In the probe the cache empties and the component carries on rendering gen-0 data forever — getProject is never called again. Removal only helps a query that gets rebuilt on a later render, which is why #199 worked (the hook was unmounted at the time).

So the reset is three steps rather than two:

  1. removeQueries for everything read through a project instance — below projects/<projectId>, plus the static media_server_origin key — so nothing can refetch with a dead closure, and the staleTime: 'static' entry gets cleared at all.
  2. resetQueries for the project-instance keys. This is the part that makes mounted screens move: a reset does dispatch, so components suspend on useSingleProject and rebuild the queries removed in step 1 with closures over the fresh instance. Its own queryFn calls clientApi.getProject(), and the client API survives the restart, so this refetch is safe.
  3. invalidateQueries on the root for the manager-level remainder.

Ordering holds because every project-derived hook calls useSingleProject first in the same component, so the component is suspended before its own query can refetch. Two probe tests pin it and both fail against the invalidate-only version: the settings query ends up with new-generation data and never enters status: 'error', and the attachment URL moves from port 5000 to 5001.

document_created_by is the other staleTime: 'static' key. It is content-addressed so its data survives, but it sits inside the project scope and gets dropped with the rest rather than carved out — re-reading an immutable mapping is cheaper than the exception. Commented in place.

Also addressed:

  • Subscribe effect now depends only on the subscribe function, with the query client in a ref. SubscribeToBackendRestart is a named exported type, so the docs live on it, the README example is module-scope rather than an inline arrow, and the contract says outright that the function should be referentially stable.
  • SyncStore's on/off are wrapped, since the off runs in effect cleanup and older @comapeo/ipc throws on a closed wrapper.
  • listen() returns the existing teardown instead of double-registering; two tests cover that and re-attaching after teardown.
  • README now says what the reset actually does step by step, and has a "what it does not cover" section for the in-memory map-share state and the lost disconnect-window events. Added the TODO at monitor() for the missing event-source error path next to the existing timeout one.
  • docs/API.md: the unreadable single-line JSDoc dump is gone — extracting SubscribeToBackendRestart moved the prose into its own entry with proper parameter/return rows. The ReceivedMapSharesContext row is still truncated mid-signature (listen(): (...), but that is tsdoc-markdown truncating a long inline type and predates this PR.

92 tests, lint and typecheck green.

@gmaclennan
feat: retry queries once-over on transport-closed rejections
f60af9f 
A query in flight when the backend's RPC transport drops rejects with
code RPC_TRANSPORT_CLOSED — a read whose response will never arrive.
Without a retry it latches into an error state during the seconds
between the drop and the restart reset, flashing error boundaries for
what is really continued loading. baseQueryOptions now retries exactly
that code (bounded, 1s delay); the retried call sits in the transport's
send queue until the restarted backend answers. Matched by error code,
not instanceof, so a duplicated @comapeo/ipc in the tree can't break
the check. Project-scoped queries reject with a different code on
retry (their instance is closed) and stop — those are recovered by
resetQueriesAfterBackendRestart as before. Mutations keep retry: false.
@gmaclennan

gmaclennan commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

Follow-up (f60af9f): baseQueryOptions now retries queries — and only queries — that reject with code: 'RPC_TRANSPORT_CLOSED' (bounded at 3 attempts, 1s delay). A query in flight when the backend's transport drops is a read whose response will never arrive; without this it latched into an error state (error-boundary flash) during the seconds before the restart reset fires. The retried call waits in the transport's send queue until the restarted backend answers, so the UI just keeps loading. Matched by error code rather than instanceof so a duplicated @comapeo/ipc copy can't break the check; project-scoped queries reject with PROJECT_CLOSED on retry and stop, staying in the reset's domain; mutations keep retry: false. Two new probe tests (retried-and-resolves without touching the error boundary; other errors not retried) — 94/94 passing, tsc/eslint/prettier clean.

@gmaclennan
fix: match the shipped RPC_CHANNEL_CLOSED transport code
2411e04 
Nothing in the dependency tree has ever thrown RPC_TRANSPORT_CLOSED: an
in-flight call whose transport drops rejects with rpc-reflector's
ChannelClosedError (code RPC_CHANNEL_CLOSED), which @comapeo/ipc
re-exports as RpcChannelClosedError and which is also what
notifyTransportReset() rejects with. The retry predicate matched a code
that never arrives, so it never fired.

Also make PROJECT_LEFT explicitly non-retryable. It is already excluded
by only retrying the channel-closed code, but a left project is the one
rejection that is guaranteed never to resolve by waiting, so it is worth
naming.
@gmaclennan
feat: refresh active sync stores after a backend restart
2ddfc17 
Under @comapeo/ipc v10 a project client reference is permanent, so
resetting the project-instance query hands back the same reference and
useSyncStore, which caches one SyncStore per reference in a WeakMap,
hands back the same store. The store keeps throwing the error it latched
when the backend went away, and nothing clears it. Under v9 a restart
produced a new reference, so a fresh store was created as a side effect
of the reset.

A WeakMap cannot be iterated, so track the stores that currently have
listeners in a module-level Set - joined on the first listener, left on
the last, so an unused store is not retained - and re-read state on each
of them as the last step of the restart recovery.
@gmaclennan
fix: surface map-share stream failures instead of hanging
d9086dd 
A download only ever leaves the downloading status because the map
server tells it to over a server-sent event stream. The event source
reconnects on its own and the promise had no failure path, so a map
server that is gone for good - it dies with the backend when Android
kills the process - left the share downloading for the life of the app.

Give up after three reconnect attempts that deliver no event in between
and move the share to error with code EVENT_STREAM_ERROR. A single
dropped connection still recovers silently.
@gmaclennan
test: cover the invites refetch and the left-project error path
6418249 
Invites are in-memory actors on the backend, so a restart drops them all
and `invite.getMany()` is the only way to learn what the new backend
has. Nothing invalidates that query on its own once the events raised
during the disconnect window are lost, so assert that the root
invalidation reaches it.

Also cover a project this device has left, in both shapes @comapeo/ipc
v10 produces it: a call on a reference acquired before leaving, and
`getProject()` for a project left before it was ever fetched. Both must
reach the error boundary with `code: PROJECT_LEFT` and without being
retried.
@gmaclennan
docs: describe v10 project reference semantics
7852654 
A backend restart no longer closes project references: under
@comapeo/ipc v10 a reference is permanent and its channel re-opens
transparently against the new backend. Rewrite the restart section
around what actually goes stale - the data read through the reference,
including the sync state store added in the previous commit - and
document the one case that is not recoverable, a project this device has
left.

Also flag that the media server origin is still read through a project
instance with a fake blob id, which is why the restart reset has to
reach outside the project namespace to drop it
(#96).
@gmaclennan
test: stop asserting a fresh project reference after re-joining
518083b 
The assertion encoded v9 semantics, where re-joining produced a new
wrapper because the old instance was closed. Under @comapeo/ipc v10 the
reference is permanent and the same wrapper comes back, so the identity
is no longer a stable fact to assert on either major. What the
regression actually needs - that the hooks reach a live instance after
re-joining, rather than a closed one - is already covered by the
settings query above.
@gmaclennan
chore: accept @comapeo/ipc v10 as a peer dependency
3538101 
Nothing in this package requires v10 to be correct: the channel-closed
retry, the sync store refresh and the query reset all work against
either major. Widen the range so consumers can move at their own pace.
The devDependency stays on the published 9.0.1 until v10 ships.
@gmaclennan
fix: stop a sync store error outliving the subscription that raised it
acdabaa 
An error latched by the store is thrown from getStateSnapshot(), which
runs during render, so it reaches an error boundary - and the boundary
unmounting the subtree removes the last listener. Two things then went
wrong. The store left the active-store registry as it unsubscribed, so
the restart refresh could not reach it; and the error stayed set, so the
remount threw it again during render, before subscribe could run. With a
permanent project reference (@comapeo/ipc v10) the remount is handed
back that very store, so the screen was wedged for the life of the app -
with or without a restart notification.

Scope the error to the subscription that produced it: #connect() clears
it before each read attempt, and #stopSubscription() clears it when the
last listener goes. An unsubscribed store then needs no refresh at all -
the next subscribe reads from scratch - so the registry rule stays as it
was, and a store still being listened to is reached the way it always
was intended to be.

Also drop the state, not just the progress baselines, when refreshing:
getDataProgressSnapshot() divides by the largest sync count seen so far,
so the previous backend's state over cleared baselines read as 1 and every
restart flashed "sync complete" before the real value arrived.
@gmaclennan
fix: make a lost map-share stream survivable rather than terminal
24bd8db 
Three problems compounded. The give-up threshold was three reconnect
attempts, which at eventsource-client's 2s default with no backoff is
about six seconds - shorter than the Android backend restart this whole
branch exists to handle, so it would fail exactly the downloads it was
meant to rescue. The error status was absorbing, and the share stores
are never reset, so a failed share stayed a dead row for the life of the
app. And the downloads map was only cleared on success, so anything that
did retry would abort against a download id that no longer existed.

Measure the outage in elapsed reconnect delay against a 60s budget
instead of counting attempts - there is no option to set the delay, and
the server can change it with a `retry:` field, so summing the delays
the client reports follows whatever it actually uses. Allow error ->
downloading, the one status the user did not choose. Clear the downloads
entry in `finally`.
@gmaclennan
docs: replace the v9 rationale for the restart reset
f7daa33 
The comment block still explained the reset as recovering from closed
project instances and queryFn closures over a dead backend, and counted
three steps where there are four - contradicting the README, which was
already rewritten for v10. State the actual reason each step exists:
references survive, the data read through them does not, the
static-staleTime media server origin cannot be invalidated at all, and a
mounted observer only refetches if its query is reset.

Give the same treatment to the media server origin hack, which is what
forces the reset to reach outside the projects namespace, and to the
generational client-api fake, whose closed-instance behaviour models v9
rather than v10 - the harsher of the two, so it still covers both.
@gmaclennan
test: pin the re-joined project reference through a direct call
986562c 
Dropping the identity assertion left the regression without anything to
fail on. Assert instead what #199 was actually about: a call made
directly through the reference the hooks hand out after re-joining
reaches a live instance rather than the closed one that was cached
before the leave.

Asserted through the reference rather than only through the settings
query, and deliberately not through the pre-leave reference - that one
recovers under v10, where references are permanent, but stays closed
under v9, so it is not a property both majors share.
@gmaclennan
docs: regenerate the API reference against the pinned @comapeo/ipc
857221a 
The committed file carried v10 type names, but the devDependency and
lockfile pin 9.0.1, so `npm ci && npm run docs:generate` silently
reverted it for anyone who ran it. Generate it from the pinned version
so the checked-in file is the one the toolchain reproduces.
@gmaclennan gmaclennan changed the title feat: recover query caches after a backend restart feat: recover hook state and query caches after a backend restart Aug 20, 2026
@gmaclennan gmaclennan mentioned this pull request Aug 20, 2026
Open
RangerMauve
RangerMauve approved these changes Aug 26, 2026
View reviewed changes

@RangerMauve RangerMauve left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but maybe andrew can take a peek too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RangerMauve RangerMauve RangerMauve approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@gmaclennan @RangerMauve