Migrations/bookorbit
1
0
Fork 0
mirror of https://github.com/bookorbit/bookorbit.git synced 2026-11-03 08:21:39 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(metadata): fail hardcover progress sync when page count is missing

Browse Source 
When a matched Hardcover edition was missing page counts, progress sync
would silently skip sending updates and save a null progress value to the
local DB. This caused the book to be perpetually stuck in a "Pending sync"
state and triggered endless redundant sync attempts.

We now throw a clear missing pages error during progress calculation to
correctly register the sync error state, log the failure, and trigger background
sync immediately when sync is first enabled for a book.

Closes #544
This commit is contained in:
neon
2026-07-04 17:12:18 +00:00
parent 4df0ed7423
commit 7ae98d3000
2 changed files with 70 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/modules/hardcover/hardcover-sync.service.test.ts
+57 -8
View File
@@ -120,7 +120,7 @@ describe('HardcoverSyncService', () => {
mockSettingsService.getSettings.mockResolvedValue({ ...defaultSettings, bookSyncMode: 'selected_only' });
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue({ syncOverride: 'included', syncExcluded: false });
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, matchMethod: 'isbn' });
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'isbn' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
@@ -133,6 +133,31 @@ describe('HardcoverSyncService', () => {
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ hardcoverUserBookId: 55, hardcoverReadId: 77 }));
});
it('fails when progress is present but the matched edition has no page count', async () => {
const errorSpy = vi.spyOn(Logger.prototype, 'error').mockImplementation(() => undefined);
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(undefined);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: null, matchMethod: 'isbn' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } });
await expect(makeService().syncBook(1, 1)).resolves.toBe('failed');
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(
expect.objectContaining({
userId: 1,
bookId: 1,
hardcoverBookId: 10,
matchMethod: 'isbn',
syncError: 'missing_edition_pages',
}),
);
errorSpy.mockRestore();
});
it('skips when the local sync snapshot has no changes', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
@@ -233,7 +258,7 @@ describe('HardcoverSyncService', () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(null);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, matchMethod: 'isbn' });
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: 300, matchMethod: 'isbn' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
@@ -309,26 +334,24 @@ describe('HardcoverSyncService', () => {
logSpy.mockRestore();
});
it('keeps progress pending when edition pages are unavailable', async () => {
it('stores error when edition pages are unavailable', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.findBookState.mockResolvedValue(null);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: 20, editionPages: null, matchMethod: 'cached' });
mockClient.query
.mockResolvedValueOnce({ insert_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } })
.mockResolvedValueOnce({ user_book_reads: [] })
.mockResolvedValueOnce({ insert_user_book_read: { user_book_read: { id: 77 }, error: null } });
.mockResolvedValueOnce({ update_user_book: { user_book: { id: 55 }, error: null } });
await makeService().syncBook(1, 1);
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ lastSyncedProgress: null }));
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ syncError: 'missing_edition_pages' }));
});
it('stores error on API failure without throwing', async () => {
mockSettingsService.getTokenForUser.mockResolvedValue('tok');
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: null, matchMethod: 'isbn' });
mockMatchService.matchBook.mockResolvedValue({ hardcoverBookId: 10, hardcoverEditionId: null, editionPages: 300, matchMethod: 'isbn' });
mockClient.query.mockRejectedValue(new Error('timeout'));
await makeService().syncBook(1, 1);
expect(mockRepo.upsertBookState).toHaveBeenCalledWith(expect.objectContaining({ syncError: 'timeout' }));
@@ -570,6 +593,32 @@ describe('HardcoverSyncService', () => {
});
expect(mockRepo.setBookSyncOverride).toHaveBeenCalledWith(1, 42, 'excluded');
});
it('triggers sync when per-book sync is enabled', async () => {
mockRepo.findBookSyncData.mockResolvedValue(readingBook);
mockRepo.setBookSyncOverride.mockResolvedValue({
syncOverride: 'included',
syncExcluded: false,
lastSyncedAt: null,
syncError: null,
});
const svc = makeService();
const syncBookSpy = vi.spyOn(svc, 'syncBook').mockResolvedValue('synced');
await expect(svc.updateBookSyncState(1, 42, { syncEnabled: true })).resolves.toEqual({
bookId: 42,
syncOverride: 'included',
syncEnabled: true,
canSyncNow: true,
effectiveReason: null,
lastSyncedAt: null,
syncError: null,
});
expect(mockRepo.setBookSyncOverride).toHaveBeenCalledWith(1, 42, null);
expect(syncBookSpy).toHaveBeenCalledWith(1, 42);
});
});
describe('cancelSync', () => {
server/src/modules/hardcover/hardcover-sync.service.ts
+13
View File
@@ -265,6 +265,15 @@ export class HardcoverSyncService {
const syncOverride = resolveHardcoverBookSyncOverrideForToggle(settings.bookSyncMode, payload.syncEnabled);
const state = await this.repo.setBookSyncOverride(userId, bookId, syncOverride);
if (payload.syncEnabled) {
void this.syncBook(userId, bookId).catch((err) => {
this.logger.error(
`[hardcover.sync_book_on_enable] [fail] userId=${userId} bookId=${bookId} errorClass=${err?.constructor?.name ?? 'Error'} error="${sanitizeLogValue(err instanceof Error ? err.message : String(err))}" - immediate sync on enable failed`,
);
});
}
return this.toBookSyncState(bookId, book, settings, state);
}
@@ -388,6 +397,10 @@ export class HardcoverSyncService {
let progressSynced = true;
if (startDate || endDate || book.progress != null) {
if (book.progress != null && !match.editionPages) {
throw new Error('missing_edition_pages');
}
const progressPages = book.progress != null && match.editionPages ? Math.round((book.progress / 100) * match.editionPages) : undefined;
progressSynced = book.progress == null || progressPages != null;