fix(sync): atomic file-based sync writes — backup-before-overwrite + remove force-overwrite race (#8786)

* fix(sync): atomic file-based sync writes - backup-before-overwrite + remove force-overwrite race

Adds a two-phase write for file-based sync (Dropbox/WebDAV/LocalFile): the current remote sync-data.json is copied to sync-data.json.bak before being overwritten (non-fatal if unsupported), and downloads recover from .bak when the main file is corrupt/empty/unparseable, surfacing a recovery notice.

Removes the unconditional isForceOverwrite=true on the rev-match retry path in _uploadWithMismatchFallback: retries stay conditional and a genuine concurrent change throws a retryable error so the next cycle merges the concurrent ops. Force-overwrite of the primary file now originates only from explicit user actions. Also reconciles a stale in-memory syncVersion against the remote on download.

SPAP-8

* fix(sync): recover encrypted/compressed corrupt files and heal the primary

Addresses review feedback on the atomic-writes .bak recovery path:

1. Recovery now triggers for encrypted/compressed primary files. A truncated
   or garbage ENCRYPTED/COMPRESSED sync-data.json fails during decrypt/decompress
   (DecryptError/DecompressError), not JSON parse, so those cases were never
   recoverable — .bak recovery silently no-opped for exactly the users who enable
   encryption or compression, the interrupted-write scenario the feature targets.
   Add both errors to _isRecoverableCorruption. Safe: an undecodable .bak makes
   _recoverFromBackup return null and the original error still surfaces.

2. Recovery heals the primary instead of getting stuck. _downloadSyncFile now
   annotates the corrupt primary's rev onto the decode error, and the recovery
   path seeds the sync-cycle cache with THAT rev (not the .bak rev). The next
   conditional upload then matches sync-data.json and overwrites (repairs) it,
   instead of mismatching forever and re-recovering/re-failing every cycle. Also
   de-dup the recovery snack per corrupt rev so a download-only client (which
   cannot heal the file) does not re-toast on every sync.

Tests: cover both via a REAL decode failure (a compressed body that is not valid
gzip, producing a genuine DecompressError rather than an injected error type) —
one asserting .bak recovery, one asserting the follow-up upload conditions on the
corrupt primary rev and heals the file.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
aakhter 2026-07-07 05:57:25 -04:00 committed by GitHub
parent 2fd9acbe60
commit cb6780933b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 638 additions and 74 deletions

View file

@ -20,12 +20,14 @@ import { ArchiveDbAdapter } from '../../../core/persistence/archive-db-adapter.s
import { ArchiveModel } from '../../../features/time-tracking/time-tracking.model';
import { StateSnapshotService } from '../../backup/state-snapshot.service';
import { DEFAULT_GLOBAL_CONFIG } from '../../../features/config/default-global-config.const';
import { SnackService } from '../../../core/snack/snack.service';
describe('FileBasedSyncAdapterService', () => {
let service: FileBasedSyncAdapterService;
let mockProvider: jasmine.SpyObj<FileSyncProvider<SyncProviderId>>;
let mockArchiveDbAdapter: jasmine.SpyObj<ArchiveDbAdapter>;
let mockStateSnapshotService: jasmine.SpyObj<StateSnapshotService>;
let mockSnackService: jasmine.SpyObj<SnackService>;
let adapter: OperationSyncCapable;
const mockCfg: EncryptAndCompressCfg = {
@ -151,11 +153,14 @@ describe('FileBasedSyncAdapterService', () => {
},
} as any);
mockSnackService = jasmine.createSpyObj('SnackService', ['open']);
TestBed.configureTestingModule({
providers: [
FileBasedSyncAdapterService,
{ provide: ArchiveDbAdapter, useValue: mockArchiveDbAdapter },
{ provide: StateSnapshotService, useValue: mockStateSnapshotService },
{ provide: SnackService, useValue: mockSnackService },
],
});
@ -397,24 +402,38 @@ describe('FileBasedSyncAdapterService', () => {
UploadRevToMatchMismatchAPIError,
);
// Only one upload attempt (no retry loop)
expect(mockProvider.uploadFile).toHaveBeenCalledTimes(1);
// Only one CONDITIONAL attempt on the MAIN sync file (no retry loop), and it
// must never force-overwrite. A separate .bak backup write is allowed.
const mainUploads = mockProvider.uploadFile.calls
.allArgs()
.filter((args) => args[0] === FILE_BASED_SYNC_CONSTANTS.SYNC_FILE);
expect(mainUploads.length).toBe(1);
expect(mainUploads.every((args) => args[3] === false)).toBe(true);
// Download called twice: initial cache + re-download to check rev
expect(mockProvider.downloadFile).toHaveBeenCalledTimes(2);
});
it('should force upload on retry when freshRev equals original revToMatch', async () => {
it('retries CONDITIONALLY (never force) when freshRev equals original revToMatch (SPAP-8)', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
// Initial download returns rev-1
// Every download returns the same rev-1 → server rev inconsistency, NOT a real
// concurrent upload. The old code force-overwrote here; the new code must retry
// the conditional upload and NEVER pass isForceOverwrite=true.
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
let uploadCalls = 0;
const mainUploadForceFlags: (boolean | undefined)[] = [];
let mainUploadAttempts = 0;
mockProvider.uploadFile.and.callFake(
(_path: string, _dataStr: string, _rev: string | null, _force: boolean) => {
uploadCalls++;
if (uploadCalls === 1) {
(path: string, _dataStr: string, _rev: string | null, force?: boolean) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
// Non-fatal backup write — irrelevant to the force assertion.
return Promise.resolve({ rev: 'bak-1' });
}
mainUploadAttempts++;
mainUploadForceFlags.push(force);
if (mainUploadAttempts === 1) {
// First conditional attempt hits a spurious rev mismatch.
return Promise.reject(new UploadRevToMatchMismatchAPIError('Rev mismatch'));
}
return Promise.resolve({ rev: 'rev-2' });
@ -424,18 +443,14 @@ describe('FileBasedSyncAdapterService', () => {
// Download to populate cache with rev-1
await adapter.downloadOps(0);
// Re-download on retry also returns rev-1 (same rev = server timestamp inconsistency)
// mockProvider.downloadFile already returns rev-1
const op = createMockSyncOp();
const result = await adapter.uploadOps([op], 'client1');
expect(result.results[0].accepted).toBe(true);
expect(uploadCalls).toBe(2);
// Retry upload should be called with isForceOverwrite: true
const retryCall = mockProvider.uploadFile.calls.argsFor(1);
expect(retryCall[3]).toBe(true); // isForceOverwrite
// Two attempts on the main file: initial + one conditional retry.
expect(mainUploadAttempts).toBe(2);
// CRUCIAL (SPAP-8): no attempt on the main sync file used force-overwrite.
expect(mainUploadForceFlags.every((f) => f === false)).toBe(true);
});
it('should clear cache after successful upload', async () => {
@ -1861,4 +1876,340 @@ describe('FileBasedSyncAdapterService', () => {
expect(result.ops).toEqual([]);
});
});
describe('atomic remote writes (SPAP-8)', () => {
const compactOp = (id: string, client = 'client1'): Record<string, unknown> => ({
id,
c: client,
a: 'HA',
o: 'ADD',
e: 'TASK',
d: `task-${id}`,
v: { [client]: 1 },
t: Date.now(),
s: 1,
p: { title: `Task ${id}` },
});
it('(a) writes .bak with the PREVIOUS remote content before overwriting sync-data.json', async () => {
const prevData = createMockSyncData({
syncVersion: 3,
clientId: 'prev-client',
recentOps: [compactOp('prev-op') as never],
});
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(prevData), rev: 'rev-1' }),
);
const uploadOrder: string[] = [];
let backupDataStr = '';
mockProvider.uploadFile.and.callFake((path: string, dataStr: string) => {
uploadOrder.push(path);
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
backupDataStr = dataStr;
}
return Promise.resolve({ rev: 'rev-2' });
});
// Populate the sync-cycle cache (mirrors a real download→upload cycle).
await adapter.downloadOps(0);
const op = createMockSyncOp();
await adapter.uploadOps([op], 'client1');
const bakIdx = uploadOrder.indexOf(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE);
const mainIdx = uploadOrder.indexOf(FILE_BASED_SYNC_CONSTANTS.SYNC_FILE);
// Backup must exist AND be written strictly before the main overwrite.
expect(bakIdx).toBeGreaterThanOrEqual(0);
expect(mainIdx).toBeGreaterThanOrEqual(0);
expect(bakIdx).toBeLessThan(mainIdx);
// The backup must contain the PREVIOUS remote content, not the new upload.
const bak = parseWithPrefix(backupDataStr);
expect(bak.syncVersion).toBe(3);
expect(bak.clientId).toBe('prev-client');
expect(bak.recentOps[0].id).toBe('prev-op');
});
it('(a) does NOT write a backup on first sync when no remote file exists yet', async () => {
mockProvider.downloadFile.and.throwError(
new RemoteFileNotFoundAPIError('sync-data.json'),
);
mockProvider.uploadFile.and.returnValue(Promise.resolve({ rev: 'rev-1' }));
await adapter.uploadOps([createMockSyncOp()], 'client1');
expect(mockProvider.uploadFile).not.toHaveBeenCalledWith(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
jasmine.any(String),
jasmine.anything(),
jasmine.anything(),
);
});
it('(a) backup-write failure is non-fatal — the main upload still succeeds', async () => {
const prevData = createMockSyncData({ syncVersion: 2 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(prevData), rev: 'rev-1' }),
);
mockProvider.uploadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
// Provider without copy support / transient failure on the backup.
return Promise.reject(new Error('backup not supported'));
}
return Promise.resolve({ rev: 'rev-2' });
});
await adapter.downloadOps(0);
const result = await adapter.uploadOps([createMockSyncOp()], 'client1');
expect(result.results[0].accepted).toBe(true);
});
it('(b) recovers from .bak when the main file is corrupt and surfaces a recovery snack', async () => {
const backupData = createMockSyncData({
syncVersion: 2,
recentOps: [compactOp('recovered-op') as never],
});
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ dataStr: addPrefix(backupData), rev: 'bak-rev-1' });
}
return Promise.reject(
new SyncDataCorruptedError('corrupt', FILE_BASED_SYNC_CONSTANTS.SYNC_FILE),
);
});
const result = await adapter.downloadOps(0);
expect(result.ops.length).toBe(1);
expect(result.ops[0].op.id).toBe('recovered-op');
expect(result.latestSeq).toBe(2);
// User-visible recovery notice.
expect(mockSnackService.open).toHaveBeenCalled();
});
it('(b) recovers from .bak when the main file is empty (InvalidDataSPError)', async () => {
const backupData = createMockSyncData({
syncVersion: 4,
recentOps: [compactOp('from-empty-recovery') as never],
});
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ dataStr: addPrefix(backupData), rev: 'bak-rev-2' });
}
return Promise.reject(
new InvalidDataSPError('empty body', FILE_BASED_SYNC_CONSTANTS.SYNC_FILE),
);
});
const result = await adapter.downloadOps(0);
expect(result.ops.length).toBe(1);
expect(result.latestSeq).toBe(4);
expect(mockSnackService.open).toHaveBeenCalled();
});
it('(b) rethrows the original corruption error when no usable backup exists', async () => {
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.reject(new RemoteFileNotFoundAPIError('no backup'));
}
return Promise.reject(
new SyncDataCorruptedError('corrupt', FILE_BASED_SYNC_CONSTANTS.SYNC_FILE),
);
});
await expectAsync(adapter.downloadOps(0)).toBeRejectedWith(
jasmine.any(SyncDataCorruptedError),
);
expect(mockSnackService.open).not.toHaveBeenCalled();
});
it('(b) recovers from .bak when the primary file fails to DECODE (real DecompressError, not an injected error)', async () => {
// The tests above inject SyncDataCorruptedError/InvalidDataSPError directly.
// This one drives a REAL decode failure end-to-end: the primary file's prefix
// claims gzip compression but the body is not valid gzip, so the actual
// decompressAndDecryptData path throws DecompressError. The encrypted-file
// case (DecryptError) is symmetric. Without both in _isRecoverableCorruption,
// recovery silently no-ops for users who enable compression or encryption.
const backupData = createMockSyncData({
syncVersion: 5,
recentOps: [compactOp('recovered-from-decode-failure') as never],
});
const undecodableMain =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 2 }) +
'this-is-not-valid-gzip-base64';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({
dataStr: addPrefix(backupData),
rev: 'bak-rev-decode',
});
}
return Promise.resolve({ dataStr: undecodableMain, rev: 'corrupt-main-rev' });
});
const result = await adapter.downloadOps(0);
expect(result.ops.length).toBe(1);
expect(result.ops[0].op.id).toBe('recovered-from-decode-failure');
expect(result.latestSeq).toBe(5);
expect(mockSnackService.open).toHaveBeenCalled();
});
it('(b) after recovery, the next upload heals the corrupt primary via ITS rev (no revToMatch pollution)', async () => {
// Regression for the self-perpetuating degraded state: recovery must seed the
// cache with the CORRUPT PRIMARY rev, not the .bak rev, so the follow-up
// conditional upload matches sync-data.json and overwrites (heals) it.
const backupData = createMockSyncData({ syncVersion: 2 });
const CORRUPT_MAIN_REV = 'corrupt-main-rev-42';
const undecodableMain =
getSyncFilePrefix({ isCompress: true, isEncrypt: false, modelVersion: 2 }) +
'not-valid-gzip';
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ dataStr: addPrefix(backupData), rev: 'bak-rev-heal' });
}
return Promise.resolve({ dataStr: undecodableMain, rev: CORRUPT_MAIN_REV });
});
const mainRevToMatch: (string | null)[] = [];
mockProvider.uploadFile.and.callFake(
(path: string, _dataStr: string, revToMatch: string | null) => {
if (path === FILE_BASED_SYNC_CONSTANTS.SYNC_FILE) {
mainRevToMatch.push(revToMatch);
}
return Promise.resolve({ rev: 'healed-rev' });
},
);
// Download recovers from .bak; a subsequent upload should heal the primary.
await adapter.downloadOps(0);
await adapter.uploadOps([createMockSyncOp()], 'client1');
expect(mainRevToMatch).toContain(CORRUPT_MAIN_REV);
expect(mainRevToMatch).not.toContain('bak-rev-heal');
});
it('(c) never issues isForceOverwrite=true on the rev-mismatch retry path', async () => {
const syncData = createMockSyncData({ syncVersion: 1 });
mockProvider.downloadFile.and.returnValue(
Promise.resolve({ dataStr: addPrefix(syncData), rev: 'rev-1' }),
);
const mainForceFlags: (boolean | undefined)[] = [];
let mainAttempts = 0;
mockProvider.uploadFile.and.callFake(
(path: string, _d: string, _r: string | null, force?: boolean) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ rev: 'bak-1' });
}
mainAttempts++;
mainForceFlags.push(force);
// Deterministic conditional rejection → exhausts retries → retryable throw.
return Promise.reject(new UploadRevToMatchMismatchAPIError('Rev mismatch'));
},
);
await adapter.downloadOps(0);
await expectAsync(
adapter.uploadOps([createMockSyncOp()], 'client1'),
).toBeRejectedWithError(UploadRevToMatchMismatchAPIError);
// Multiple attempts occurred, but NOT ONE forced.
expect(mainAttempts).toBeGreaterThan(1);
expect(mainForceFlags.every((f) => f === false)).toBe(true);
});
it("(d) interleaved clients: client A's ops survive when B races; B fails retryably without forcing", async () => {
// Honest rev model: the remote rev changes whenever its content changes.
let remote = {
dataStr: addPrefix(createMockSyncData({ syncVersion: 1, recentOps: [] })),
rev: 'rev-1',
};
const aOp = compactOp('op-A', 'client-A');
mockProvider.downloadFile.and.callFake((path: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.reject(new RemoteFileNotFoundAPIError('no bak'));
}
return Promise.resolve({ dataStr: remote.dataStr, rev: remote.rev });
});
let sawForceOnMain = false;
mockProvider.uploadFile.and.callFake(
(path: string, dataStr: string, revToMatch: string | null, force?: boolean) => {
if (path === FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE) {
return Promise.resolve({ rev: 'bak-1' });
}
if (force) {
sawForceOnMain = true;
}
// Conditional upload: reject if the expected rev no longer matches remote.
if (!force && revToMatch !== null && revToMatch !== remote.rev) {
return Promise.reject(new UploadRevToMatchMismatchAPIError('rev changed'));
}
remote = { dataStr, rev: `rev-${Math.random()}` };
return Promise.resolve({ rev: remote.rev });
},
);
// Client B downloads (caches rev-1).
await adapter.downloadOps(0);
// Client A concurrently commits op-A → remote advances to rev-2.
remote = {
dataStr: addPrefix(
createMockSyncData({ syncVersion: 2, recentOps: [aOp as never] }),
),
rev: 'rev-2',
};
// Client B now uploads its own op; its cached rev-1 is stale.
const bOp = createMockSyncOp({ id: 'op-B', clientId: 'client-B' });
await expectAsync(adapter.uploadOps([bOp], 'client-B')).toBeRejectedWithError(
UploadRevToMatchMismatchAPIError,
);
// A's op must NOT have vanished from the remote, and B must not have forced.
const finalRemote = parseWithPrefix(remote.dataStr);
expect(finalRemote.recentOps.some((o) => o.id === 'op-A')).toBe(true);
expect(sawForceOnMain).toBe(false);
});
it('(guard) reconciles a stale in-memory expected syncVersion that is ahead of remote', async () => {
// Establish expected syncVersion = 5.
mockProvider.downloadFile.and.returnValue(
Promise.resolve({
dataStr: addPrefix(createMockSyncData({ syncVersion: 5, recentOps: [] })),
rev: 'rev-5',
}),
);
await adapter.downloadOps(0);
// Remote regresses to syncVersion 2 (e.g. another client's recovery snapshot).
mockProvider.downloadFile.and.returnValue(
Promise.resolve({
dataStr: addPrefix(createMockSyncData({ syncVersion: 2, recentOps: [] })),
rev: 'rev-2',
}),
);
const dl = await adapter.downloadOps(1);
expect(dl.gapDetected).toBe(true);
// The next upload must build on the REMOTE version (→ 3), proving the stale
// in-memory counter (5) was reconciled rather than trusted.
let uploadedMain = '';
mockProvider.uploadFile.and.callFake((path: string, dataStr: string) => {
if (path === FILE_BASED_SYNC_CONSTANTS.SYNC_FILE) {
uploadedMain = dataStr;
}
return Promise.resolve({ rev: 'rev-new' });
});
await adapter.uploadOps([createMockSyncOp()], 'client1');
expect(parseWithPrefix(uploadedMain).syncVersion).toBe(3);
});
});
});

View file

@ -1,4 +1,4 @@
import { inject, Injectable } from '@angular/core';
import { inject, Injectable, Injector } from '@angular/core';
import { SyncProviderId } from '../provider.const';
import {
FileSyncProvider,
@ -27,12 +27,17 @@ import {
} from './file-based-sync.types';
import { OpLog } from '../../../core/log';
import {
DecompressError,
DecryptError,
InvalidDataSPError,
JsonParseError,
LegacySyncFormatDetectedError,
RemoteFileNotFoundAPIError,
SyncDataCorruptedError,
UploadRevToMatchMismatchAPIError,
} from '../../core/errors/sync-errors';
import { SnackService } from '../../../core/snack/snack.service';
import { T } from '../../../t.const';
import { mergeVectorClocks } from '../../../core/util/vector-clock';
import { ArchiveDbAdapter } from '../../../core/persistence/archive-db-adapter.service';
import { StateSnapshotService } from '../../backup/state-snapshot.service';
@ -77,6 +82,16 @@ export class FileBasedSyncAdapterService {
private _encryptAndCompressHandler = new EncryptAndCompressHandlerService();
private _archiveDbAdapter = inject(ArchiveDbAdapter);
private _stateSnapshotService = inject(StateSnapshotService);
// Resolved lazily (not eagerly injected) to avoid a construction-time DI cycle:
// SnackService's dependency graph transitively reaches the sync providers. We only
// need it for the non-fatal backup-recovery notice, well after construction.
private _injector = inject(Injector);
/**
* Max CONDITIONAL upload retries after a rev mismatch before surfacing a
* retryable error. The retry path never force-overwrites the primary sync file.
*/
private readonly _MAX_UPLOAD_RETRIES = 2;
/** Expected sync version for optimistic locking, keyed by provider+user */
private _expectedSyncVersions = new Map<string, number>();
@ -84,6 +99,13 @@ export class FileBasedSyncAdapterService {
/** Local sequence counters (simulates server seq for file-based) */
private _localSeqCounters = new Map<string, number>();
/**
* Last corrupt primary rev we surfaced a backup-recovery notice for, keyed by
* provider+user. Prevents re-toasting the same corruption every sync cycle for a
* download-only client that cannot heal the primary file itself.
*/
private _lastRecoveredCorruptRev = new Map<string, string>();
/** Cache for downloaded sync data within a sync cycle (avoids redundant downloads) */
private _syncCycleCache = new Map<
string,
@ -444,10 +466,22 @@ export class FileBasedSyncAdapterService {
}
/**
* Attempts to upload sync data. On revision mismatch, re-downloads once to
* distinguish a WebDAV server timestamp inconsistency (same rev force-upload)
* from a genuine concurrent upload (different rev throw so the next sync cycle
* can download the concurrent ops and retry with a consistent snapshot).
* Uploads sync data with a CONDITIONAL write (optimistic lock on `revToMatch`).
*
* On a rev mismatch it re-downloads to classify the failure, but it NEVER
* force-overwrites the primary sync file that would silently clobber a
* concurrent client's ops if the rev check is fooled (caching / rev reuse /
* eventual consistency). Force-overwrite of `sync-data.json` is reachable ONLY
* from explicit user actions (forceUploadLocalState / conflict "Use Local").
*
* - Same rev after re-download transient server rev/timestamp inconsistency
* (no real concurrent upload). Retry the CONDITIONAL upload (bounded by
* `_MAX_UPLOAD_RETRIES`); if still failing, surface a retryable error.
* - Different rev after re-download a genuine concurrent upload. Throw a
* retryable error so the next sync cycle downloads the concurrent ops
* (applying them to the store) and rebuilds a consistent snapshot via
* `_buildMergedSyncData()`. Re-uploading here would embed a stale in-memory
* snapshot whose state never saw those ops.
*/
private async _uploadWithMismatchFallback(
provider: FileSyncProvider<SyncProviderId>,
@ -467,53 +501,61 @@ export class FileBasedSyncAdapterService {
'FileBasedSyncAdapter._uploadWithMismatchFallback',
);
try {
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
uploadData,
revToMatch,
false,
);
return { finalSyncVersion: newData.syncVersion };
} catch (e) {
if (!(e instanceof UploadRevToMatchMismatchAPIError)) {
throw e;
const maxAttempts = 1 + this._MAX_UPLOAD_RETRIES;
let currentRevToMatch = revToMatch;
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
try {
// CONDITIONAL upload only (isForceOverwrite=false). Never forces.
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
uploadData,
currentRevToMatch,
false,
);
return { finalSyncVersion: newData.syncVersion };
} catch (e) {
if (!(e instanceof UploadRevToMatchMismatchAPIError)) {
throw e;
}
}
}
// Rev mismatch — re-download once to check whether a real concurrent upload occurred.
OpLog.normal(
'FileBasedSyncAdapter: Rev mismatch detected, re-downloading to check...',
);
const { rev: freshRev } = await this._downloadSyncFile(provider, cfg, encryptKey);
if (attempt === maxAttempts) {
break;
}
if (freshRev === revToMatch) {
// Rev unchanged after re-download → WebDAV server timestamp/ETag inconsistency
// (no real concurrent upload). Reuse the already-built uploadData — the remote
// file is confirmed identical to what we were trying to overwrite, so no need
// to re-snapshot state, re-merge ops, or re-encrypt.
// Rev mismatch — re-download to check whether a real concurrent upload occurred.
OpLog.normal(
'FileBasedSyncAdapter: Rev mismatch detected, re-downloading to check...',
);
const { rev: freshRev } = await this._downloadSyncFile(provider, cfg, encryptKey);
if (freshRev !== currentRevToMatch) {
// Genuine concurrent upload: another client wrote between our download and
// upload. Do NOT overwrite. Throw retryable so the next sync cycle downloads
// the concurrent ops (applying them to the store) and rebuilds a consistent
// snapshot. This guarantees the concurrent client's ops are never clobbered.
throw new UploadRevToMatchMismatchAPIError(
'FileBasedSyncAdapter: Concurrent upload detected. Next sync cycle will ' +
'download the concurrent ops and retry with a consistent snapshot.',
);
}
// Same rev after re-download → transient server rev/timestamp inconsistency
// (the remote is confirmed identical to what we were trying to overwrite).
// Retry the CONDITIONAL upload — never force.
OpLog.warn(
'FileBasedSyncAdapter: Rev unchanged after re-download, server has inconsistent ' +
'timestamp handling. Force-uploading.',
'FileBasedSyncAdapter: Rev unchanged after re-download (server rev/timestamp ' +
'inconsistency). Retrying conditional upload without force-overwrite.',
);
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
uploadData,
freshRev,
true,
);
return { finalSyncVersion: newData.syncVersion };
currentRevToMatch = freshRev;
}
// Real concurrent upload: another client uploaded between our download and upload.
// Re-uploading here would embed a stale NgRx snapshot (their ops were never applied
// to our store), creating a recentOps/state inconsistency for fresh-bootstrap clients.
// Throw so SyncWrapperService can handle it as a known-transient condition.
// The next sync cycle downloads the concurrent ops (applying them to NgRx), then
// uploads with a consistent snapshot.
// Retries exhausted without a successful conditional upload. Surface a retryable
// error rather than silently force-overwriting the primary sync file.
throw new UploadRevToMatchMismatchAPIError(
'FileBasedSyncAdapter: Concurrent upload detected. Next sync cycle will ' +
'download the concurrent ops and retry with a consistent snapshot.',
'FileBasedSyncAdapter: Upload retries exhausted after repeated rev mismatch. ' +
'Sync will retry on the next cycle.',
);
}
@ -560,7 +602,16 @@ export class FileBasedSyncAdapterService {
currentSyncVersion,
);
// Step 3: Upload; on rev mismatch re-download once and either force-upload or throw
// Step 2.5: Backup-before-overwrite (two-phase write). Copy the CURRENT remote
// content to sync-data.json.bak before overwriting the primary file, so an
// interrupted/corrupt write can be recovered on the next download. Non-fatal:
// a provider without copy support must still be able to sync.
if (fileExists && currentData) {
await this._backupCurrentRemote(provider, cfg, encryptKey, currentData);
}
// Step 3: Upload conditionally; on rev mismatch re-download and retry
// conditionally or throw retryably (never force-overwrite).
const { finalSyncVersion } = await this._uploadWithMismatchFallback(
provider,
cfg,
@ -628,7 +679,41 @@ export class FileBasedSyncAdapterService {
latestSeq: 0,
};
}
throw e;
if (this._isRecoverableCorruption(e)) {
// Primary sync-data.json is corrupt/empty/unparseable (e.g. interrupted
// write). Try to recover from the .bak artifact before failing.
const recovered = await this._recoverFromBackup(provider, cfg, encryptKey);
if (recovered) {
OpLog.warn(
'FileBasedSyncAdapter: Primary sync file unreadable; recovered from backup',
);
syncData = recovered.data;
// Seed the cache with the CORRUPT PRIMARY file's rev (annotated onto the
// error in _downloadSyncFile), not the .bak rev. The next conditional
// upload then matches sync-data.json and OVERWRITES (heals) it. Using the
// .bak rev would mismatch the primary on every upload, leaving sync stuck
// in a re-recover/re-fail loop. Fall back to the .bak rev if the primary
// rev is unavailable (a later mismatch just re-recovers — no data loss).
const primaryRev =
(e as { primaryRev?: string } | null)?.primaryRev ?? recovered.rev;
rev = primaryRev;
this._setCachedSyncData(providerKey, syncData, rev);
// De-dup the user-facing notice: only surface it the first time we see a
// given corrupt primary rev, so a download-only client (which cannot heal
// the file itself) does not re-toast on every sync cycle.
if (this._lastRecoveredCorruptRev.get(providerKey) !== primaryRev) {
this._lastRecoveredCorruptRev.set(providerKey, primaryRev);
// Lazy resolve — absent in some unit harnesses (returns null → no notice).
this._injector
.get(SnackService, null)
?.open({ msg: T.F.SYNC.S.SYNC_DATA_RECOVERED_FROM_BACKUP });
}
} else {
throw e;
}
} else {
throw e;
}
}
// Detect syncVersion reset (e.g., another client uploaded a snapshot).
@ -638,6 +723,19 @@ export class FileBasedSyncAdapterService {
const versionWasReset =
previousExpectedVersion > 0 && syncData.syncVersion < previousExpectedVersion;
// Guard against stale in-memory state: if the persisted expected syncVersion is
// AHEAD of what the remote file reports, our counter is stale (e.g. after another
// client uploaded a lower-version recovery snapshot). Log and reconcile to the
// remote's authoritative value rather than trusting the stale counter — the
// `_expectedSyncVersions.set(...)` below always adopts the remote version, and
// `versionWasReset` drives gap detection so the caller re-downloads from seq 0.
if (previousExpectedVersion > syncData.syncVersion) {
OpLog.warn(
`FileBasedSyncAdapter: Persisted expected syncVersion (${previousExpectedVersion}) is ` +
`ahead of remote (${syncData.syncVersion}); reconciling to remote (stale in-memory counter).`,
);
}
// Also detect snapshot replacement: if client expected ops (sinceSeq > 0) but file has
// no recent ops AND has a snapshot state, another client uploaded a fresh snapshot.
// This happens when "Use Local" is chosen in conflict resolution - the snapshot replaces
@ -912,6 +1010,106 @@ export class FileBasedSyncAdapterService {
}
}
/**
* Writes the CURRENT remote content to sync-data.json.bak before the primary
* file is overwritten (two-phase write). We re-encode the already-in-hand
* `currentData` from `_getCurrentSyncState()` rather than issuing another GET.
*
* MUST be non-fatal: a provider that cannot write the backup (e.g. no copy
* support, transient failure) must still be able to sync. The backup is a
* recovery artifact losing it only degrades recoverability, never sync.
*
* Force-overwrite is used here because .bak is a disposable recovery artifact,
* NOT the source-of-truth sync file the lost-update guarantee on
* sync-data.json is unaffected.
*/
private async _backupCurrentRemote(
provider: FileSyncProvider<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
currentData: FileBasedSyncData,
): Promise<void> {
try {
const backupData = await this._encryptAndCompressHandler.compressAndEncryptData(
cfg,
encryptKey,
currentData,
FILE_BASED_SYNC_CONSTANTS.FILE_VERSION,
);
if (!backupData || backupData.trim().length === 0) {
OpLog.warn(
'FileBasedSyncAdapter: Skipping backup — encoded backup data was empty',
);
return;
}
await provider.uploadFile(
FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE,
backupData,
null,
true,
);
OpLog.normal('FileBasedSyncAdapter: Wrote backup before overwrite');
} catch (e) {
// Non-fatal by design — proceed with the primary upload regardless.
OpLog.warn(
'FileBasedSyncAdapter: Backup-before-overwrite failed (non-fatal), proceeding',
e,
);
}
}
/**
* Whether a download error indicates a corrupt/empty/unparseable primary file
* that we should attempt to recover from the .bak artifact. Missing files are
* NOT included that is a legitimate fresh-start signal handled separately.
*/
private _isRecoverableCorruption(e: unknown): boolean {
return (
e instanceof SyncDataCorruptedError ||
// Covers EmptyRemoteBodySPError (empty file) via its InvalidDataSPError base.
e instanceof InvalidDataSPError ||
e instanceof JsonParseError ||
// A truncated/garbage ENCRYPTED or COMPRESSED primary file fails during the
// decrypt/decompress stage rather than JSON parse. Without these, .bak
// recovery silently no-ops for exactly the users who enable encryption or
// compression — the interrupted-write case this feature targets. Safe to
// include: if the .bak is also undecodable, _recoverFromBackup returns null
// and the original error still surfaces.
e instanceof DecryptError ||
e instanceof DecompressError
);
}
/**
* Attempts to recover sync data from sync-data.json.bak. Returns null if the
* backup is missing, unreadable, or has an unsupported version.
*/
private async _recoverFromBackup(
provider: FileSyncProvider<SyncProviderId>,
cfg: EncryptAndCompressCfg,
encryptKey: string | undefined,
): Promise<{ data: FileBasedSyncData; rev: string } | null> {
try {
const response = await provider.downloadFile(FILE_BASED_SYNC_CONSTANTS.BACKUP_FILE);
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedSyncData>(
cfg,
encryptKey,
response.dataStr,
);
if (data.version !== FILE_BASED_SYNC_CONSTANTS.FILE_VERSION) {
OpLog.warn(
`FileBasedSyncAdapter: Backup has unsupported version ${data.version}; cannot recover`,
);
return null;
}
return { data, rev: response.rev };
} catch (e) {
OpLog.warn('FileBasedSyncAdapter: Backup recovery failed', e);
return null;
}
}
/**
* Downloads and decrypts the sync file.
*
@ -956,19 +1154,32 @@ export class FileBasedSyncAdapterService {
}
throw e;
}
const data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedSyncData>(
cfg,
encryptKey,
response.dataStr,
);
let data: FileBasedSyncData;
try {
data =
await this._encryptAndCompressHandler.decompressAndDecryptData<FileBasedSyncData>(
cfg,
encryptKey,
response.dataStr,
);
// Validate file version
if (data.version !== FILE_BASED_SYNC_CONSTANTS.FILE_VERSION) {
throw new SyncDataCorruptedError(
`Unsupported file version: ${data.version} (expected ${FILE_BASED_SYNC_CONSTANTS.FILE_VERSION})`,
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
);
// Validate file version
if (data.version !== FILE_BASED_SYNC_CONSTANTS.FILE_VERSION) {
throw new SyncDataCorruptedError(
`Unsupported file version: ${data.version} (expected ${FILE_BASED_SYNC_CONSTANTS.FILE_VERSION})`,
FILE_BASED_SYNC_CONSTANTS.SYNC_FILE,
);
}
} catch (decodeErr) {
// Annotate the corrupt primary file's current rev onto the error so the
// download-time recovery path can seed the cache with IT (not the .bak rev)
// and heal sync-data.json via a matching conditional overwrite, instead of
// mismatching forever and re-recovering every cycle. We already hold the rev
// here (from the download above), so no extra request is needed.
if (decodeErr && typeof decodeErr === 'object' && !('primaryRev' in decodeErr)) {
(decodeErr as { primaryRev?: string }).primaryRev = response.rev;
}
throw decodeErr;
}
return { data, rev: response.rev };

View file

@ -1644,6 +1644,7 @@ const T = {
STORAGE_RECOVERED_AFTER_COMPACTION: 'F.SYNC.S.STORAGE_RECOVERED_AFTER_COMPACTION',
SUCCESS_DOWNLOAD: 'F.SYNC.S.SUCCESS_DOWNLOAD',
SUCCESS_VIA_BUTTON: 'F.SYNC.S.SUCCESS_VIA_BUTTON',
SYNC_DATA_RECOVERED_FROM_BACKUP: 'F.SYNC.S.SYNC_DATA_RECOVERED_FROM_BACKUP',
SYNC_VALIDATION_FAILED: 'F.SYNC.S.SYNC_VALIDATION_FAILED',
TIMEOUT_ERROR: 'F.SYNC.S.TIMEOUT_ERROR',
TOO_MANY_OPS_TO_DOWNLOAD: 'F.SYNC.S.TOO_MANY_OPS_TO_DOWNLOAD',

View file

@ -1596,6 +1596,7 @@
"STORAGE_RECOVERED_AFTER_COMPACTION": "Storage was running low. Old data cleaned up successfully.",
"SUCCESS_DOWNLOAD": "Synced data from remote",
"SUCCESS_VIA_BUTTON": "Data successfully synced",
"SYNC_DATA_RECOVERED_FROM_BACKUP": "Recovered sync data from backup after the main file could not be read.",
"SYNC_VALIDATION_FAILED": "State validation failed after sync. Some data may be inconsistent.",
"TIMEOUT_ERROR": "Sync operation timed out. {{suggestion}}",
"TOO_MANY_OPS_TO_DOWNLOAD": "Too many sync operations to download. Some changes may be missing.",