experiments in a post-browser web
peek notes sync-edge-cases.md
8.3 kB · 286 lines
Markdown
at main

Sync Edge Cases Investigation #

This document details the investigation of sync edge cases between mobile, desktop, and server. It documents current behavior, known limitations, and recommendations for future fixes.

Summary #

Edge Case Priority Current Behavior Impact
Deleted items not synced HIGH Deletes are local-only Items "resurrect" on other devices
Push failures not retried HIGH Failed items lost Potential data loss
Tagset sync LOW Works correctly None
Unicode content LOW Works correctly None
Identical timestamps LOW Item skipped None (correct behavior)
Clock skew LOW Not validated Incorrect conflict resolution

Category 1: Deleted Items (HIGH PRIORITY) #

Current Behavior #

When an item is soft-deleted on desktop:

  1. deleteItem() sets deletedAt = timestamp in local database
  2. Item is excluded from queryItems() results (which filter WHERE deletedAt = 0)
  3. Critically: Push query also filters WHERE deletedAt = 0, so deleted items are never pushed to server
  4. Server never learns about the delete
  5. Other devices (mobile) never learn about the delete

Code Evidence #

Desktop sync.ts - Pull (line 187):

const localItem = db.prepare(
  'SELECT * FROM items WHERE syncId = ? AND deletedAt = 0'
).get(serverItem.id);

Desktop sync.ts - Push (lines 288-297):

items = db.prepare(`
  SELECT * FROM items
  WHERE deletedAt = 0 AND (syncedAt = 0 OR updatedAt > syncedAt)
`).all();

Server db.js - getItemsSince (line 717):

WHERE deleted_at IS NULL AND updated_at > ?

Impact #

  1. Delete item on Desktop → Still exists on Server → Mobile still sees it
  2. Delete item on Mobile → Still exists on Server → Desktop still sees it
  3. Items effectively "resurrect" when syncing to other devices
  4. User confusion when deleted items keep reappearing

Option A: Push delete tombstones

  • When deletedAt is set, keep item in push query
  • Push includes deleted_at timestamp to server
  • Server marks item as deleted
  • Other clients pull the deletion and apply locally

Option B: Push explicit delete operations

  • Add DELETE /items/:id call when item is deleted locally
  • Track pending deletes separately from pending creates/updates
  • Retry failed deletes

Recommended: Option A (tombstone approach) is cleaner and matches the existing last-write-wins model.

Test Added #

testDeletedItemsNotSynced() in sync-e2e.test.js - Documents and verifies this limitation.


Category 2: Push Failure Recovery (HIGH PRIORITY) #

Current Behavior #

When pushing items to server:

  1. Each item is pushed individually in pushSingleItem()
  2. If push fails, error is caught and logged
  3. failed counter is incremented
  4. Critically: After sync completes, lastSyncTime is updated regardless of failures
  5. On next sync, failed items have updatedAt < lastSyncTime
  6. Failed items are never retried

Code Evidence #

Desktop sync.ts - Push loop (lines 305-312):

for (const item of items) {
  try {
    await pushSingleItem(serverUrl, apiKey, item);
    pushed++;
  } catch (error) {
    DEBUG && console.log(`[sync] Failed to push item ${item.id}:`, (error as Error).message);
    failed++;  // Just logged and counted, no retry mechanism
  }
}

Desktop sync.ts - syncAll (line 408):

setSyncConfig({ lastSyncTime: startTime });  // Updated even if pushes failed

Impact #

  1. Network glitch during push → Item fails to sync
  2. lastSyncTime advances past item's updatedAt
  3. Item is never retried on subsequent syncs
  4. Data loss: Item exists only on device that created it

Option A: Track failed items

// Add to extension_settings or separate table
interface FailedPushItem {
  itemId: string;
  lastAttempt: number;
  errorMessage: string;
  retryCount: number;
}
  • On push failure, add item to failed queue
  • On each sync, retry failed items first
  • Remove from failed queue on success
  • Exponential backoff after multiple failures

Option B: Don't advance lastSyncTime if any failures

if (pushResult.failed === 0) {
  setSyncConfig({ lastSyncTime: startTime });
}
  • Simpler but may cause extra work on subsequent syncs
  • Could retry already-synced items

Recommended: Option A provides more robust tracking. Option B is a quick fix.

Test Added #

testPushFailureNotRetried() in sync-e2e.test.js - Documents this issue.


Category 3: Timestamp Edge Cases (MEDIUM PRIORITY) #

3a. lastSyncTime Update Timing #

Current behavior: lastSyncTime is set to startTime (beginning of sync) not end.

This is correct because:

  • Items created during sync have createdAt > startTime
  • They will be caught on the next sync
  • If we used end time, items created mid-sync could be missed

3b. Clock Skew #

Current behavior: No validation of client vs server times.

Impact:

  • If client clock is significantly ahead, local items always "win"
  • If client clock is behind, server items always "win"
  • Extreme skew (>24 hours) could cause data loss

Recommendation: Add optional clock skew warning when server time differs significantly.

3c. Identical Timestamps #

Current behavior: In mergeServerItem(), line 241:

if (localItem.updatedAt > serverUpdatedAt) {
  return 'conflict';  // Local wins
}
// Same timestamp falls through to 'skipped'
return 'skipped';

This is correct - equal timestamps mean no update needed.

Test Added #

testIdenticalTimestamps() in sync-e2e.test.js - Verifies no duplicates on repeated pulls.


Category 4: Data Edge Cases (MEDIUM PRIORITY) #

4a. Tagset Sync #

Current behavior: Works correctly.

Tagsets are items with null content that exist solely to hold tags. Testing confirmed tagsets sync properly between desktop and server.

Test: testTagsetSync() creates a tagset with specific tags on desktop, pushes to server, and verifies the tagset appears with correct tags.

4b. Unicode Content #

Current behavior: Full Unicode support works correctly.

Tested:

  • Emoji (🌍, 🎉)
  • CJK characters (日本語)
  • Greek (Ελληνικά)
  • URLs with Unicode query params
  • Control characters (newlines, tabs)

All sync correctly through SQLite and JSON serialization.

4c. Large Content #

Current behavior: No size limits enforced.

Potential issues:

  • Very large text items could cause memory issues
  • No pagination on pull

Recommendation: Consider adding soft limits (e.g., warn on >1MB content).

Tests Added #

testEmptyContent() and testUnicodeContent() in sync-e2e.test.js.


Category 5: Network Failures (LOW PRIORITY) #

Current Behavior #

  • Sync throws on network error
  • Error is propagated to caller
  • No offline queue

Recommendation #

For mobile especially, consider:

  1. Queue sync operations when offline
  2. Auto-retry when network returns
  3. Show pending sync status to user

This is documented in sync-architecture.md:178 as a known limitation.


Category 6: Image Binary Sync (OUT OF SCOPE) #

This is a documented limitation (sync-architecture.md:179). Images sync metadata only, not binary data. This is intentional for now to avoid bandwidth issues.


Test Summary #

Added 5 new edge case tests to backend/tests/sync-e2e.test.js:

Test Purpose
testDeletedItemsNotSynced Documents that soft deletes don't propagate
testPushFailureNotRetried Documents that failed pushes can cause data loss
testEmptyContent Verifies empty string and null content handling
testUnicodeContent Verifies non-ASCII content syncs correctly
testIdenticalTimestamps Verifies no duplicates on repeated pulls

Run with:

yarn test:sync

Recommendations Summary #

Immediate (Should Fix) #

  1. Push failure retry - At minimum, don't advance lastSyncTime if any pushes fail
  2. Soft delete sync - Push deletions as tombstones to server

Future Improvements #

  1. Offline queue with retry logic
  2. Clock skew detection and warning
  3. Content size limits
  4. Pagination for large syncs