This is a submission for DEV's Summer Bug Smash: Clear the Lineup powered by Sentry.
The Part of Sync Nobody Warns You About
ctrodb is an offline-first database for the browser. I built it to learn how sync engines actually work and to have a mongoDB structured schema on the client. I had used IndexedDB wrappers and sync libraries for years, but I had never looked closely at the part that can quietly destroy user data.
A quick glimpse into it if you want to contribute.
ctrodb is a small library: reads and writes land in a local store, the change tracker records pending edits, and the sync engine pushes changes up and pulls changes down whenever the network returns.
The sync engine is the whole point. It is also where the dangerous bugs live. A database failure usually throws an error. A sync failure often does not. The edit simply disappears, because the code path that overwrote it never knew it existed.
The bug was not obvious in the code. A ten-line regression test is what finally exposed it.
A Four-Step Race That Erased User Data
This is a data-loss bug fix, and I did not find it in production. I found it by writing a test whose whole job was to destroy an edit on purpose.
The pull path had a hole. Every individual step looked reasonable. The bug only appeared when all four happened in exactly the wrong order.
- A record is edited locally. The edit sits safely in the local store, and the change tracker holds a pending change so the next sync can push it up.
- The server still has an older snapshot of that same record, maybe synced from another device before the local edit existed.
- The pull step fetches that older snapshot and writes it over the local record.
- The edit is gone. The change tracker still holds the pending change, but the store no longer matches what the pending change claims.
The next push then does one of two things, and both are catastrophic. It either uploads the now-stale local record back to the server, or the conflict resolver chooses remote-wins and permanently discards the local edit. The user sees the same thing either way: the text they typed is gone. No error. No warning. The console stays quiet. They reload the page, the work is missing, and they assume the application failed them.
The code looked completely reasonable. That is what made the bug expensive to find. Before the fix, #applyRemoteChange fetched the local record and overwrote it, and it never once asked the change tracker whether a pending local edit existed for that record:
// BEFORE: the pull path never asked the change tracker anything
async #applyRemoteChange(change): Promise<void> {
const adapter = this.#adapter
const local = await adapter.findById(change.collection, change.recordId)
switch (change.type) {
case "create":
case "update": {
if (change.data === null) break
if (local) {
await adapter.update(change.collection, change.recordId, change.data)
} else {
await adapter.create(change.collection, { id: change.recordId, ...change.data })
}
break
}
case "delete": {
if (local) await adapter.delete(change.collection, change.recordId)
break
}
}
}
There is no off-by-one here and no inverted condition to spot. The bug is an absence: the pull path and the change tracker are two subsystems that never spoke to each other. Each behaved exactly as designed. Together they produced silent data loss.
A Timestamp Check Saved the Edit
The fix lives in PR #1: fix(sync): stop pull from clobbering pending local edits.
The fix is surprisingly small. Before the pull loop applies any remote change, it snapshots which records already have pending local edits:
// In #pullChanges, before the pull loop:
const pending = await this.#tracker.getPending()
const pendingByKey = new Map<string, SyncChangeRecord>()
for (const change of pending) {
pendingByKey.set(this.#changeKey(change.collection, change.recordId), change)
}
The important part is not the map. The important part is the timestamp comparison.
// AFTER: remote changes must beat the local edit on time, not on arrival order
async #applyRemoteChange(change, pendingByKey): Promise<void> {
const adapter = this.#adapter
const pendingLocal = pendingByKey.get(this.#changeKey(change.collection, change.recordId))
if (pendingLocal) {
const localTs = this.#parseTs(pendingLocal.timestamp)
const remoteTs = this.#parseTs(change.timestamp)
if (remoteTs < localTs) {
// The remote snapshot is a replay older than the pending local edit.
// Skip it. The push cycle owns this record now.
return
}
// Remote is newer or equal: it wins, matching the conflict resolver's LWW
// (remote wins ties). Drop the stale pending change so the next push
// does not resurrect it.
await this.#tracker.markCommitted(pendingLocal.id, {
serverTimestamp: change.timestamp,
})
pendingByKey.delete(this.#changeKey(change.collection, change.recordId))
}
const local = await adapter.findById(change.collection, change.recordId)
// ... apply create/update/delete as before
}
The subtle detail is the tie. Equal timestamps resolve to remote, on purpose. The existing ConflictResolver uses last-write-wins, and when timestamps tie it picks remote, so the pull path now matches that rule exactly.
If pull and conflict resolution disagree about the same record, the next sync can flip the outcome back and forth. They have to agree.
Now Realizing the First Fix Was Wrong
The regression suite did the real work. I wrote the test before the fix, and I wrote it to lose data: create a record, edit it locally so a pending change exists, pull an older server snapshot for the same record, run sync, then assert the local edit survived.
Against the old code it failed, exactly as a reproduction test should. Proof of the bug first. The fix came after.
The suite now covers four cases in tests/unit/sync/pull-clobber-race.test.ts:
- A stale remote update must not overwrite a newer pending local edit.
- A stale remote delete must not delete a pending local create.
- Remote updates still apply when nothing is pending locally, so the fix cannot block normal sync.
- Unacknowledged pushed changes roll back to pending instead of leaking into a stuck state.
That test uncovered the original bug. The rest of the suite uncovered something I was not even looking for.
When a transport accepted some changes but not all, the unacknowledged ones stayed marked in-flight forever. They never errored and never returned to pending, so retry logic could not see them. A silent orphan.
The push cycle now rolls unacknowledged results back to pending so they get retried, and the batching test asserts it: a cycle that only half-acknowledges ends with the rest back in pending, not stuck in limbo.
Two bugs in one PR, because the second one only shows up once you start testing the first one hard.
I almost shipped the wrong fix. My first version skipped every remote change for any record with a pending local edit, regardless of timestamps. The existing suite failed immediately, because newer remote updates and deletes are supposed to win.
That failure forced the timestamp guard, and the final implementation is better because the tests disagreed with me.
Full suite after the fix: 481 tests passing, 26 files. Zero regressions.
And actually this is the part I keep coming back to.
Sync engines are judged by the mistakes they do not make, because the mistakes they do make often look like nothing happened.
There is no crash and no 500. The dashboard stays green and the edit is simply gone. The only witness to this bug was a test that knew the edit should still be there.
The test that catches this bug is ten lines. The fix is a timestamp comparison. The cost of not having them is every user who reloads the page, sees their work missing, and concludes the app is broken.
The safest sync systems are not the ones with the most code. They are the ones that deliberately try to lose data and prove that they cannot.
What is the one sync or retry path in your own app that you have never tested for the "should have lost my data" case? So you check it before the next release 😂
Top comments (0)