Skip to content

retrySync silently loses pending changes when sync() is called before the async persist load finishes #659

Description

@LinusOssmann

Summary

With retrySync: true, offline writes that survived an app restart are silently lost if syncState(obs$).sync() is called before the (asynchronous) local persist load has finished. The pending change stays in the local store forever, getPendingChanges() still shows it, but it is never sent to the server — on this launch or any future one.

retrySync: true promises pending changes are persisted and replayed on the next launch. This works when the observable is activated implicitly (by being observed), because that path is gated on the persist load. It breaks when application code calls syncState(obs$).sync() explicitly during startup — a common "bootstrap all collections on launch" pattern.

Root cause

In src/sync/syncObservable.ts, loadLocal() is async (IndexedDB/MMKV reads) and is not awaited. It sets the pending snapshot and lastSync only once its internal awaits resolve, and flips isPersistLoaded at the very end:

// end of loadLocal()
localState.pendingChanges = metadata.pending; // pending arrives HERE, later
// ...
syncState$.isPersistLoaded.set(!(numPendingLocalLoads > 0));

The implicit activation path is correctly gated on the load:

when(onAllPersistLoaded, () => {
  if ((syncOptions.get || syncOptions.subscribe) && syncOptions.syncMode === 'auto') {
    sync();
  }
});

But syncState.sync is assigned synchronously during syncObservable(). An explicit syncState(obs$).sync() made after activation but before isPersistLoaded runs the real sync() immediately, with no gate. sync() captures the pending changes at call time and consumes the one-shot replay with that stale (empty) snapshot:

sync = async (options) => {
  // ...
  const lastSync = metadata?.lastSync;       // still undefined
  const pending = localState.pendingChanges;  // still undefined: load not done
  // ...
  if (!isSynced) {
    isSynced = true;         // one-shot flag consumed
    applyPending(pending);   // applies NOTHING
  }
};

Milliseconds later loadLocal finishes and sets localState.pendingChanges, but isSynced is already true, so applyPending never runs again. The pending change survives locally forever and is never sent.

Secondary symptom

metadata.lastSync is populated by the same not-awaited loadLocal. An early explicit sync() therefore also reads lastSync === undefined and performs a full re-fetch instead of a changesSince: 'last-sync' delta query.

Reproduction

Any collection with persist + retrySync: true + changesSince: 'last-sync':

  1. While offline, create a record (a pending create is persisted to the local metadata).
  2. Restart the app.
  3. During startup — before the local metadata read has resolved — call syncState(collection$).sync() (e.g. an app-level "sync all collections on boot" routine, an early pull-to-refresh, or a deep-linked screen that force-syncs).
  4. The pending create is never sent. getPendingChanges() returns it indefinitely; the server row is never created. Every future launch repeats step 3 and loses the replay again.

Whether it triggers depends on the race between the local read and the first explicit sync() — deterministic on slower stores/devices, intermittent on fast ones, which makes it hard to diagnose in the field.

Proposed fix

Gate the real sync() on the persist load, the same way the implicit activation path already is. Since sync is already async, this is a small addition at the top of sync():

      const metadata = metadatas.get(obs$);
+     // An explicit syncState(obs$).sync() can run before the async loadLocal() has
+     // finished. The one-shot pending replay below (and lastSync) must not be consumed
+     // with a not-yet-loaded snapshot, or persisted pending changes from the previous
+     // session are silently lost forever.
+     if (syncOptions.persist && !syncState$.isPersistLoaded.peek()) {
+         await when(syncState$.isPersistLoaded);
+     }

(placed just after the early not-observed/not-enabled return, before metadata/lastSync/pending are read.)

Why this is correct

  • isPersistLoaded is set at the end of loadLocal(), which has no dependency on sync() — no deadlock.
  • For observables without persist, the guard is skipped (unchanged behaviour).
  • For the implicit path (when(onAllPersistLoaded)sync()), isPersistLoaded is already true, so the await is a no-op.
  • After the guard, metadata, lastSync and pending are all populated, so both the pending replay and the delta query behave exactly as on the implicit path.

Workaround (for consumers until merged)

Route every explicit sync through a wrapper that waits for the load first:

export async function syncAfterPersistLoad(obs$) {
    const state = syncState(obs$);
    obs$.peek(); // lazy synced nodes only activate on access
    await when(state.isPersistLoaded);
    await state.sync?.();
}

Functionally equivalent to the fix, but has to be enforced by convention at every call site — one missed site re-introduces the data loss, which is why the in-library fix is preferable.


I already have a local branch with the fix plus a regression test (an explicit sync() racing a slow loadTable; the test fails — pending creates lost — before the fix and passes after, and also asserts the persisted lastSync is used). Happy to open a PR once this is triaged.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions