fix(sync): abort the file-sync run on auth failure instead of marching the library (#4981)

With an expired Google Drive web session, syncLibrary swallowed the
AUTH_FAILED from the index pull and proceeded with remoteIndex = null,
which is indistinguishable from a first sync: every book looked
unpushed, uploadedHashes was empty, and the engine attempted to upload
the entire library (Uploading 16 / 682), logging one failure per book.
Worse, a null index also skips the peers-tombstone union in the final
index re-push, so a transient index-pull failure could have rewritten
library.json and resurrected deleted books (#4860 class).

- An unreadable index (throw) now aborts the run; an absent one
  (404 -> null) keeps first-sync semantics.
- A terminal-failure latch stops the work pools on the first mid-run
  AUTH_FAILED (a token can expire mid-run), skips the index re-push,
  and rethrows so callers surface one re-auth error instead of a
  per-book failure list. Mirrors the existing deleteRemoteBookDir
  contract: auth failures rethrow, everything else aggregates.
- On web, the auto library sync now skips while the Drive session is
  expired (hasValidWebDriveToken), matching the settings form's
  disabled Sync now with its Reconnect CTA.

Tests: unreadable-index abort with zero writes, mid-run abort bounded
to in-flight work with no index re-push, non-auth failures still
non-aborting, absent-index first sync unchanged.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Huang Xin
2026-07-07 02:14:54 +09:00
committed by GitHub
parent db1d63cdcc
commit 3503b0234f
3 changed files with 361 additions and 175 deletions
@@ -0,0 +1,134 @@
import { beforeEach, describe, expect, test, vi } from 'vitest';
import type { Book, BookConfig } from '@/types/book';
import { FileSyncEngine } from '@/services/sync/file/engine';
import { FileSyncError, type FileSyncProvider } from '@/services/sync/file/provider';
import type { LocalStore } from '@/services/sync/file/localStore';
import type { RemoteLibraryIndex } from '@/services/sync/file/wire';
/**
* Terminal-failure semantics (the Drive web token expiry incident): an
* unreadable index must not be treated as "no index yet" (which turns an
* expired session into an attempted mass re-upload and would drop peers'
* tombstones from the re-pushed index), and a mid-run AUTH_FAILED must stop
* the per-book march instead of failing every remaining book identically.
*/
const makeBook = (hash: string): Book => ({
hash,
format: 'EPUB',
title: `Book ${hash}`,
sourceTitle: `Book ${hash}`,
author: 'Author',
createdAt: 1,
updatedAt: 100,
});
const authError = () =>
new FileSyncError('Google Drive session expired; reconnect in Settings', 'AUTH_FAILED', 401);
const makeStore = (overrides: Partial<LocalStore> = {}): LocalStore => ({
loadConfig: async (): Promise<BookConfig> => ({ updatedAt: 50, booknotes: [] }),
saveBookConfig: async () => {},
loadBookFile: async () => null,
resolveLocalBookPath: async () => null,
saveBookFile: async () => {},
prepareLocalBookPath: async () => '/local/path',
loadBookCover: async () => null,
saveBookCover: async () => {},
addBookToLibrary: async () => {},
updateBookMetadata: async () => {},
deleteBookLocally: async () => {},
...overrides,
});
const baseProvider = (overrides: Partial<FileSyncProvider> = {}): FileSyncProvider => ({
rootPath: '/',
readText: vi.fn(async () => null),
readBinary: vi.fn(async () => new ArrayBuffer(8)),
head: vi.fn(async () => null),
list: vi.fn(async () => []),
writeText: vi.fn(async () => {}),
writeBinary: vi.fn(async () => {}),
ensureDir: vi.fn(async () => {}),
deleteDir: vi.fn(async () => {}),
...overrides,
});
const syncOptions = { strategy: 'silent', syncBooks: false, deviceId: 'd1' } as const;
beforeEach(() => {
vi.clearAllMocks();
vi.spyOn(console, 'warn').mockImplementation(() => {});
});
describe('FileSyncEngine terminal auth failures', () => {
test('an unreadable index aborts the run before any push (expired session != first sync)', async () => {
const provider = baseProvider({
readText: vi.fn(async (path: string) => {
if (path.endsWith('library.json')) throw authError();
return null;
}),
});
const engine = new FileSyncEngine(provider, makeStore());
await expect(
engine.syncLibrary([makeBook('h1'), makeBook('h2')], syncOptions),
).rejects.toMatchObject({ code: 'AUTH_FAILED' });
expect(provider.writeText).not.toHaveBeenCalled();
expect(provider.writeBinary).not.toHaveBeenCalled();
});
test('a mid-run AUTH_FAILED stops the march and skips the index re-push', async () => {
const books = Array.from({ length: 50 }, (_, i) => makeBook(`h${i}`));
const writeText = vi.fn(async (path: string) => {
if (path.endsWith('config.json')) throw authError();
// Recording an index write is the failure we care about below.
});
const provider = baseProvider({ writeText });
const engine = new FileSyncEngine(provider, makeStore());
await expect(engine.syncLibrary(books, syncOptions)).rejects.toMatchObject({
code: 'AUTH_FAILED',
});
// The pool must stop scheduling once the session is known-dead: far
// fewer attempts than the 50-book library (bounded by in-flight work).
expect(writeText.mock.calls.length).toBeLessThanOrEqual(8);
// No library.json write from a run that could not read or sync anything.
const indexWrites = writeText.mock.calls.filter(([p]) => String(p).endsWith('library.json'));
expect(indexWrites).toHaveLength(0);
});
test('non-auth per-book failures still do not abort the rest (one bad apple)', async () => {
const books = Array.from({ length: 10 }, (_, i) => makeBook(`h${i}`));
const capture: { index?: RemoteLibraryIndex } = {};
const writeText = vi.fn(async (path: string, body: string) => {
if (path.endsWith('config.json')) throw new FileSyncError('boom', 'NETWORK', 500);
if (path.endsWith('library.json')) capture.index = JSON.parse(body) as RemoteLibraryIndex;
});
const provider = baseProvider({ writeText });
const engine = new FileSyncEngine(provider, makeStore());
const result = await engine.syncLibrary(books, syncOptions);
expect(result.failures).toBe(10);
// The run completed and the index was still re-pushed.
expect(capture.index).toBeDefined();
});
test('an absent index (404 -> null) keeps first-sync semantics', async () => {
const capture: { index?: RemoteLibraryIndex } = {};
const writeText = vi.fn(async (path: string, body: string) => {
if (path.endsWith('library.json')) capture.index = JSON.parse(body) as RemoteLibraryIndex;
});
const provider = baseProvider({ writeText });
const engine = new FileSyncEngine(provider, makeStore());
const result = await engine.syncLibrary([makeBook('h1')], syncOptions);
expect(result.failures).toBe(0);
expect(capture.index?.books.map((b) => b.hash)).toEqual(['h1']);
});
});
@@ -7,6 +7,8 @@ import { useSettingsStore } from '@/store/settingsStore';
import { useLibraryStore } from '@/store/libraryStore';
import { useFileSyncStore } from '@/store/fileSyncStore';
import { isCloudSyncAllowed } from '@/utils/access';
import { isWebAppPlatform } from '@/services/environment';
import { hasValidWebDriveToken } from '@/services/sync/providers/gdrive/auth/webTokenStore';
import { debounce } from '@/utils/debounce';
import { FileSyncEngine } from '@/services/sync/file/engine';
import { createAppLocalStore } from '@/services/sync/file/appLocalStore';
@@ -66,7 +68,14 @@ export const useLibraryFileSync = () => {
const w = settings.webdav;
return !!(w?.enabled && w?.serverUrl && w?.username);
}
if (activeKind === 'gdrive') return !!settings.googleDrive?.enabled;
if (activeKind === 'gdrive') {
// Web Drive tokens are session-scoped with no refresh; once expired,
// every run would abort with AUTH_FAILED on the index pull. Skip the
// auto-sync until the user reconnects (the Drive settings form shows
// the Reconnect CTA), mirroring its disabled "Sync now".
if (isWebAppPlatform() && !hasValidWebDriveToken()) return false;
return !!settings.googleDrive?.enabled;
}
return false;
}, [isAllowed, activeKind, settings.webdav, settings.googleDrive]);
+217 -174
View File
@@ -162,11 +162,12 @@ const runPool = async <T>(
items: T[],
limit: number,
worker: (item: T, index: number) => Promise<void>,
stopped?: () => boolean,
): Promise<void> => {
if (items.length === 0) return;
let cursor = 0;
const runners = Array.from({ length: Math.min(Math.max(1, limit), items.length) }, async () => {
while (cursor < items.length) {
while (cursor < items.length && !stopped?.()) {
const index = cursor;
cursor += 1;
await worker(items[index]!, index);
@@ -384,13 +385,28 @@ export class FileSyncEngine {
let remoteIndex: RemoteLibraryIndex | null = null;
if (canPull) {
try {
remoteIndex = await this.pullLibraryIndex();
} catch (e) {
console.warn('file sync: failed to pull index', e);
}
// An UNREADABLE index (throw — expired session, network) is NOT the
// same as an ABSENT one (404 → null, first-sync semantics). Proceeding
// with a null index here would treat every local book as unpushed (an
// attempted mass re-upload against a dead session) and the final index
// re-push would drop the peers' tombstones it failed to read (#4860),
// resurrecting deleted books. Abort the run instead; callers surface
// one error.
remoteIndex = await this.pullLibraryIndex();
}
// Terminal-failure latch: once any remote call fails with AUTH_FAILED the
// session is gone for every subsequent call too. Stop scheduling work
// instead of marching the whole library through identical failures, skip
// the index re-push (a partial run must not rewrite library.json), and
// rethrow so the caller shows a single re-auth error. Mirrors the
// deleteRemoteBookDir contract (AUTH failures rethrow; the rest aggregate).
let abort: FileSyncError | null = null;
const noteAbort = (e: unknown): void => {
if (!abort && e instanceof FileSyncError && e.code === 'AUTH_FAILED') abort = e;
};
const aborted = (): boolean => abort !== null;
const allBooksMap = new Map<string, Book>();
for (const b of books) {
allBooksMap.set(b.hash, b);
@@ -450,47 +466,54 @@ export class FileSyncEngine {
const local = allBooksMap.get(rb.hash);
return !!local && !local.deletedAt && shouldApplyRemoteBookMetadata(local, rb);
});
await runPool(remoteNewer, concurrency, async (rb) => {
const local = allBooksMap.get(rb.hash)!;
const merged = mergeBookMetadata(local, rb);
// Re-pull the cover so a changed cover travels with the metadata. The
// subsequent push-side pushBookCover HEAD/size short-circuit then
// matches (local now equals remote), so we never bounce it back up.
try {
const coverBytes = await this.pullBookCover(rb.hash);
if (coverBytes) await this.store.saveBookCover(merged, coverBytes);
} catch (e) {
console.warn('file sync: metadata cover pull failed', rb.hash, e);
}
// Incremental only: the per-book push loop below skips remote-newer
// books, so pull their config here too — otherwise a peer's progress /
// notes wouldn't propagate without re-walking every book. In full-sync
// mode the push loop pulls each config, so we skip this to avoid a
// duplicate GET.
if (!fullSync) {
await runPool(
remoteNewer,
concurrency,
async (rb) => {
const local = allBooksMap.get(rb.hash)!;
const merged = mergeBookMetadata(local, rb);
// Re-pull the cover so a changed cover travels with the metadata. The
// subsequent push-side pushBookCover HEAD/size short-circuit then
// matches (local now equals remote), so we never bounce it back up.
try {
const localConfig = (await this.store.loadConfig(merged)) ?? {
updatedAt: 0,
booknotes: [],
};
const pull = await this.pullBookConfig(merged, localConfig);
if (pull.applied && pull.mergedConfig) {
await this.store.saveBookConfig(merged, pull.mergedConfig);
result.configsDownloaded += 1;
}
const coverBytes = await this.pullBookCover(rb.hash);
if (coverBytes) await this.store.saveBookCover(merged, coverBytes);
} catch (e) {
console.warn('file sync: metadata config pull failed', rb.hash, e);
noteAbort(e);
console.warn('file sync: metadata cover pull failed', rb.hash, e);
}
}
try {
await this.store.updateBookMetadata(merged);
allBooksMap.set(rb.hash, merged);
result.metadataUpdated += 1;
syncedHashes.add(rb.hash);
} catch (e) {
console.warn('file sync: metadata update failed', rb.hash, e);
}
});
// Incremental only: the per-book push loop below skips remote-newer
// books, so pull their config here too — otherwise a peer's progress /
// notes wouldn't propagate without re-walking every book. In full-sync
// mode the push loop pulls each config, so we skip this to avoid a
// duplicate GET.
if (!fullSync) {
try {
const localConfig = (await this.store.loadConfig(merged)) ?? {
updatedAt: 0,
booknotes: [],
};
const pull = await this.pullBookConfig(merged, localConfig);
if (pull.applied && pull.mergedConfig) {
await this.store.saveBookConfig(merged, pull.mergedConfig);
result.configsDownloaded += 1;
}
} catch (e) {
noteAbort(e);
console.warn('file sync: metadata config pull failed', rb.hash, e);
}
}
try {
await this.store.updateBookMetadata(merged);
allBooksMap.set(rb.hash, merged);
result.metadataUpdated += 1;
syncedHashes.add(rb.hash);
} catch (e) {
console.warn('file sync: metadata update failed', rb.hash, e);
}
},
aborted,
);
}
// Deletion propagation (#4860): a book a peer tombstoned in the shared index
@@ -559,12 +582,14 @@ export class FileSyncEngine {
}
} catch (e) {
// 404 is normal if the user has never pushed anything yet.
noteAbort(e);
console.warn('file sync: failed to list books directory', e);
}
// 3) For every candidate, look inside its hash directory to find the
// actual book file (the only entry that isn't config.json/cover.png).
for (const hash of candidateHashes) {
if (aborted()) break;
try {
const hashDirPath = `${buildBasePath(this.provider.rootPath)}/${SYNC_BOOKS_DIR}/${hash}`;
const hashDirEntries = await this.provider.list(hashDirPath);
@@ -608,6 +633,7 @@ export class FileSyncEngine {
remoteBooksToDownload.push(book);
allBooksMap.set(hash, book);
} catch (e) {
noteAbort(e);
console.warn('file sync: failed to inspect hash dir', hash, e);
}
}
@@ -618,79 +644,85 @@ export class FileSyncEngine {
// that exist remotely but not locally is the whole point of a sync.
if (canPull) {
let downloadStarted = 0;
await runPool(remoteBooksToDownload, concurrency, async (rb) => {
options.onProgress?.({
book: rb,
index: downloadStarted,
total: remoteBooksToDownload.length,
action: 'downloading',
});
downloadStarted += 1;
try {
const explicitPath = explicitRemotePaths.get(rb.hash);
// Prefer the streaming downloader. On Tauri/Android we MUST take
// this path — moving a 30 MB epub through the WebView<->Rust IPC
// bridge as a single Uint8Array crashes the renderer.
let written = false;
if (this.provider.downloadStream && explicitPath) {
const dst = await this.store.prepareLocalBookPath(rb);
written = await this.provider.downloadStream(explicitPath, dst);
} else {
const remotePath = explicitPath ?? buildBookFilePath(this.provider.rootPath, rb);
const fileBytes = await this.provider.readBinary(remotePath);
if (fileBytes) {
await this.store.saveBookFile(rb, fileBytes);
written = true;
}
}
if (written) {
try {
const coverBytes = await this.pullBookCover(rb.hash);
if (coverBytes) await this.store.saveBookCover(rb, coverBytes);
} catch (e) {
console.warn('file sync: cover download failed', rb.hash, e);
}
// Pull the remote config so progress, bookmarks and annotations
// travel with the book. Best-effort: a missing config is not a
// failure.
try {
const emptyLocal: BookConfig = { updatedAt: 0, booknotes: [] };
const pullResult = await this.pullBookConfig(rb, emptyLocal);
if (pullResult.applied && pullResult.mergedConfig) {
await this.store.saveBookConfig(rb, pullResult.mergedConfig);
result.configsDownloaded += 1;
await runPool(
remoteBooksToDownload,
concurrency,
async (rb) => {
options.onProgress?.({
book: rb,
index: downloadStarted,
total: remoteBooksToDownload.length,
action: 'downloading',
});
downloadStarted += 1;
try {
const explicitPath = explicitRemotePaths.get(rb.hash);
// Prefer the streaming downloader. On Tauri/Android we MUST take
// this path — moving a 30 MB epub through the WebView<->Rust IPC
// bridge as a single Uint8Array crashes the renderer.
let written = false;
if (this.provider.downloadStream && explicitPath) {
const dst = await this.store.prepareLocalBookPath(rb);
written = await this.provider.downloadStream(explicitPath, dst);
} else {
const remotePath = explicitPath ?? buildBookFilePath(this.provider.rootPath, rb);
const fileBytes = await this.provider.readBinary(remotePath);
if (fileBytes) {
await this.store.saveBookFile(rb, fileBytes);
written = true;
}
} catch (e) {
console.warn('file sync: config download failed', rb.hash, e);
}
await this.store.addBookToLibrary(rb);
result.booksDownloaded += 1;
syncedHashes.add(rb.hash);
// We just pulled its bytes, so the file is on the remote — record it
// so a later push-side sync doesn't HEAD-probe it back.
uploadedHashes.add(rb.hash);
} else {
// No bytes returned (typically a 404 we couldn't resolve).
if (written) {
try {
const coverBytes = await this.pullBookCover(rb.hash);
if (coverBytes) await this.store.saveBookCover(rb, coverBytes);
} catch (e) {
console.warn('file sync: cover download failed', rb.hash, e);
}
// Pull the remote config so progress, bookmarks and annotations
// travel with the book. Best-effort: a missing config is not a
// failure.
try {
const emptyLocal: BookConfig = { updatedAt: 0, booknotes: [] };
const pullResult = await this.pullBookConfig(rb, emptyLocal);
if (pullResult.applied && pullResult.mergedConfig) {
await this.store.saveBookConfig(rb, pullResult.mergedConfig);
result.configsDownloaded += 1;
}
} catch (e) {
console.warn('file sync: config download failed', rb.hash, e);
}
await this.store.addBookToLibrary(rb);
result.booksDownloaded += 1;
syncedHashes.add(rb.hash);
// We just pulled its bytes, so the file is on the remote — record it
// so a later push-side sync doesn't HEAD-probe it back.
uploadedHashes.add(rb.hash);
} else {
// No bytes returned (typically a 404 we couldn't resolve).
result.failures += 1;
result.failedBooks.push({
hash: rb.hash,
title: rb.title || rb.hash,
phase: 'download',
reason: 'No bytes returned (file may have been moved or deleted on the server)',
});
console.warn('file sync: book download produced no bytes', rb.hash, explicitPath);
}
} catch (e) {
noteAbort(e);
result.failures += 1;
result.failedBooks.push({
hash: rb.hash,
title: rb.title || rb.hash,
phase: 'download',
reason: 'No bytes returned (file may have been moved or deleted on the server)',
reason: formatFailureReason(e),
});
console.warn('file sync: book download produced no bytes', rb.hash, explicitPath);
console.warn('file sync: book download failed', rb.hash, e);
}
} catch (e) {
result.failures += 1;
result.failedBooks.push({
hash: rb.hash,
title: rb.title || rb.hash,
phase: 'download',
reason: formatFailureReason(e),
});
console.warn('file sync: book download failed', rb.hash, e);
}
});
},
aborted,
);
}
// Books we just downloaded already exist on the remote — don't re-push
@@ -718,81 +750,92 @@ export class FileSyncEngine {
if (canPush && booksToPush.length > 0) {
let pushStarted = 0;
await runPool(booksToPush, concurrency, async (book) => {
options.onProgress?.({
book,
index: pushStarted,
total: booksToPush.length,
action: 'uploading',
});
pushStarted += 1;
let phase: SyncFailureEntry['phase'] = 'upload-config';
try {
if (configChanged(book)) {
const config = await this.store.loadConfig(book);
if (config) {
// Mirror the reader hook's pull-merge-push discipline so a manual
// "Sync now" can't blind-overwrite state this device hasn't pulled
// yet. Only in two-way ('silent') mode — 'send' keeps the blind
// push. A failed pull-merge falls back to the local config.
let configToPush = config;
if (canPull) {
try {
const pull = await this.pullBookConfig(book, config);
if (pull.applied && pull.mergedConfig) {
configToPush = pull.mergedConfig;
// Persist the merged superset locally so this device
// converges too, not just the remote.
await this.store.saveBookConfig(book, pull.mergedConfig);
await runPool(
booksToPush,
concurrency,
async (book) => {
options.onProgress?.({
book,
index: pushStarted,
total: booksToPush.length,
action: 'uploading',
});
pushStarted += 1;
let phase: SyncFailureEntry['phase'] = 'upload-config';
try {
if (configChanged(book)) {
const config = await this.store.loadConfig(book);
if (config) {
// Mirror the reader hook's pull-merge-push discipline so a manual
// "Sync now" can't blind-overwrite state this device hasn't pulled
// yet. Only in two-way ('silent') mode — 'send' keeps the blind
// push. A failed pull-merge falls back to the local config.
let configToPush = config;
if (canPull) {
try {
const pull = await this.pullBookConfig(book, config);
if (pull.applied && pull.mergedConfig) {
configToPush = pull.mergedConfig;
// Persist the merged superset locally so this device
// converges too, not just the remote.
await this.store.saveBookConfig(book, pull.mergedConfig);
}
} catch (e) {
console.warn('file sync: config pull-merge failed', book.hash, e);
}
} catch (e) {
console.warn('file sync: config pull-merge failed', book.hash, e);
}
}
await this.pushBookConfig(book, configToPush, options.deviceId);
result.configsUploaded += 1;
syncedHashes.add(book.hash);
}
// Covers ride along with the config-level sync, NOT with syncBooks:
// the receiving device can't regenerate them without the book bytes.
// Failures here are warnings, not hard failures.
try {
const coverResult = await this.pushBookCover(book);
if (coverResult.uploaded) {
result.coversUploaded += 1;
await this.pushBookConfig(book, configToPush, options.deviceId);
result.configsUploaded += 1;
syncedHashes.add(book.hash);
}
} catch (e) {
console.warn('file sync: cover failed', book.hash, e);
// Covers ride along with the config-level sync, NOT with syncBooks:
// the receiving device can't regenerate them without the book bytes.
// Failures here are warnings, not hard failures.
try {
const coverResult = await this.pushBookCover(book);
if (coverResult.uploaded) {
result.coversUploaded += 1;
syncedHashes.add(book.hash);
}
} catch (e) {
console.warn('file sync: cover failed', book.hash, e);
}
}
}
if (needsFilePush(book)) {
phase = 'upload-file';
const fileResult = await this.pushBookFile(book);
if (fileResult.uploaded) {
result.filesUploaded += 1;
syncedHashes.add(book.hash);
uploadedHashes.add(book.hash);
} else if (fileResult.reason === 'remote-matches') {
result.filesAlreadyInSync += 1;
uploadedHashes.add(book.hash);
if (needsFilePush(book)) {
phase = 'upload-file';
const fileResult = await this.pushBookFile(book);
if (fileResult.uploaded) {
result.filesUploaded += 1;
syncedHashes.add(book.hash);
uploadedHashes.add(book.hash);
} else if (fileResult.reason === 'remote-matches') {
result.filesAlreadyInSync += 1;
uploadedHashes.add(book.hash);
}
// 'no-source' → the file isn't on this device; leave it unrecorded
// so a device that does have it can upload and record it later.
}
// 'no-source' → the file isn't on this device; leave it unrecorded
// so a device that does have it can upload and record it later.
} catch (e) {
noteAbort(e);
result.failures += 1;
result.failedBooks.push({
hash: book.hash,
title: book.title || book.hash,
phase,
reason: formatFailureReason(e),
});
console.warn('file sync: book failed', book.hash, e);
}
} catch (e) {
result.failures += 1;
result.failedBooks.push({
hash: book.hash,
title: book.title || book.hash,
phase,
reason: formatFailureReason(e),
});
console.warn('file sync: book failed', book.hash, e);
}
});
},
aborted,
);
}
// A terminal auth failure surfaced mid-run: rethrow instead of re-pushing
// an index built from a partial run, and let the caller show one re-auth
// error rather than a per-book failure list.
if (abort) throw abort;
// The final index whenever we're allowed to write, even if no binaries
// moved this turn (keeps library.json authoritative). Union in any remote
// entries this device never materialised (chiefly peers' tombstones):