fix(library): isolate per-type sync failures, suppress dry-run notices
syncAll() ran each enabled type's sync in sequence with no per-type error handling, so one type throwing (e.g. its vault folder missing) aborted the whole run and skipped libraryLastSync -- every other enabled type silently never synced. Each type's runSync is now wrapped individually: failures are logged (and surfaced via Notice unless quiet) and the loop continues; libraryLastSync still updates after the loop regardless. Also: dry-run sync previews now route spec-level flip notifications (the "chapter/issue out" Notices) through the log instead of a real Notice, since a dry run never actually writes the change it's describing.
This commit is contained in:
parent
ea5ddd9454
commit
548e4c8ce0
2 changed files with 170 additions and 7 deletions
|
|
@ -91,17 +91,25 @@ export class LibraryController {
|
|||
};
|
||||
}
|
||||
|
||||
private makeSpecDeps(): SpecDeps {
|
||||
/** dryRun=true routes flip notifications to the log instead of a real Notice -- a dry-run
|
||||
* preview shouldn't pop user-facing notices for changes that were never actually written. */
|
||||
private makeSpecDeps(dryRun = false): SpecDeps {
|
||||
return {
|
||||
http: this.makeHttp(),
|
||||
httpText: this.makeHttpText(),
|
||||
getKey: name => this.getKey(name),
|
||||
log: msg => console.log(`[media-db-library] ${msg}`),
|
||||
notify: msg => this.notify(msg),
|
||||
notify: msg => {
|
||||
if (dryRun) {
|
||||
console.log(`[media-db-library] [dry] ${msg}`);
|
||||
} else {
|
||||
this.notify(msg);
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
private makeDeps(spec: MediaTypeSpec): LibraryEngineDeps {
|
||||
private makeDeps(spec: MediaTypeSpec, dryRun = false): LibraryEngineDeps {
|
||||
const { app } = this.plugin;
|
||||
const folderPath = this.folderFor(spec);
|
||||
const folder = app.vault.getAbstractFileByPath(folderPath);
|
||||
|
|
@ -122,7 +130,7 @@ export class LibraryController {
|
|||
},
|
||||
sleep: ms => new Promise(r => setTimeout(r, ms)),
|
||||
log: msg => console.log(`[media-db-library] ${msg}`),
|
||||
specDeps: this.makeSpecDeps(),
|
||||
specDeps: this.makeSpecDeps(dryRun),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
@ -140,7 +148,7 @@ export class LibraryController {
|
|||
|
||||
private async runSync(spec: MediaTypeSpec, full: boolean, dryRun: boolean, quiet: boolean): Promise<LibraryReport> {
|
||||
this.warnMissingKey(spec, quiet);
|
||||
const deps = this.makeDeps(spec);
|
||||
const deps = this.makeDeps(spec, dryRun);
|
||||
const report = await libraryFolderSync(spec, deps, { full, dryRun });
|
||||
const mode = dryRun ? 'DRY-RUN ' : '';
|
||||
if (shouldNotifySync(quiet, report.written, report.errors.length)) {
|
||||
|
|
@ -172,7 +180,7 @@ export class LibraryController {
|
|||
}
|
||||
this.syncing = true;
|
||||
try {
|
||||
const deps = this.makeDeps(spec);
|
||||
const deps = this.makeDeps(spec, dryRun);
|
||||
const report = await libraryFolderResolve(spec, deps, { dryRun });
|
||||
const mode = dryRun ? 'DRY-RUN ' : '';
|
||||
this.notify(
|
||||
|
|
@ -194,7 +202,15 @@ export class LibraryController {
|
|||
try {
|
||||
for (const spec of this.specs) {
|
||||
if (!this.enabledFor(spec)) continue;
|
||||
await this.runSync(spec, full, false, quiet);
|
||||
try {
|
||||
await this.runSync(spec, full, false, quiet);
|
||||
} catch (e) {
|
||||
// one type's failure (e.g. its vault folder is missing) must not abort the
|
||||
// rest of the run -- log + surface it, then keep going with the other types
|
||||
const msg = `Library sync failed for ${spec.typeName}: ${e instanceof Error ? e.message : String(e)}`;
|
||||
console.log(`[media-db-library] ${msg}`);
|
||||
if (!quiet) this.notify(msg);
|
||||
}
|
||||
}
|
||||
if (!full) {
|
||||
this.plugin.settings.libraryLastSync = Date.now();
|
||||
|
|
|
|||
|
|
@ -274,3 +274,150 @@ describe('missing API key handling', () => {
|
|||
expect(notices.filter(m => m.includes('API key not configured')).length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('syncAll error isolation (I5)', () => {
|
||||
test('first spec throws (e.g. missing vault folder) -> remaining specs still synced, lastSync updated', async () => {
|
||||
const c = new LibraryController(fakePlugin());
|
||||
const notices: string[] = [];
|
||||
(c as any).notify = (msg: string) => notices.push(msg);
|
||||
const calledTypes: string[] = [];
|
||||
(c as any).makeDeps = (spec: { typeName: string }) => {
|
||||
calledTypes.push(spec.typeName);
|
||||
if (spec.typeName === 'manga') {
|
||||
return {
|
||||
listNotes: async () => {
|
||||
throw new Error('Library folder not found: Mangas');
|
||||
},
|
||||
readNote: async () => '',
|
||||
writeNote: async () => {},
|
||||
sleep: async () => {},
|
||||
log: () => {},
|
||||
specDeps: fakeSpecDeps(),
|
||||
};
|
||||
}
|
||||
return {
|
||||
listNotes: async () => [],
|
||||
readNote: async () => '',
|
||||
writeNote: async () => {},
|
||||
sleep: async () => {},
|
||||
log: () => {},
|
||||
specDeps: fakeSpecDeps(),
|
||||
};
|
||||
};
|
||||
|
||||
const before = (c as any).plugin.settings.libraryLastSync;
|
||||
await c.syncAll(false, false);
|
||||
|
||||
// manga (first in LIBRARY_TYPES) threw, but book/game/comic still got their turn
|
||||
expect(calledTypes.sort()).toEqual(['book', 'comic', 'game', 'manga']);
|
||||
expect(notices.some(m => m.includes('Library sync failed for manga'))).toBe(true);
|
||||
expect((c as any).plugin.settings.libraryLastSync).toBeGreaterThan(before);
|
||||
});
|
||||
|
||||
test('quiet sync: per-type failure logged only, no Notice', async () => {
|
||||
const c = new LibraryController(fakePlugin());
|
||||
const notices: string[] = [];
|
||||
(c as any).notify = (msg: string) => notices.push(msg);
|
||||
const logs: string[] = [];
|
||||
const origLog = console.log;
|
||||
console.log = (msg: string) => {
|
||||
logs.push(msg);
|
||||
};
|
||||
(c as any).makeDeps = (spec: { typeName: string }) => ({
|
||||
listNotes: async () => {
|
||||
if (spec.typeName === 'book') throw new Error('boom');
|
||||
return [];
|
||||
},
|
||||
readNote: async () => '',
|
||||
writeNote: async () => {},
|
||||
sleep: async () => {},
|
||||
log: () => {},
|
||||
specDeps: fakeSpecDeps(),
|
||||
});
|
||||
|
||||
try {
|
||||
await c.syncAll(false, true);
|
||||
} finally {
|
||||
console.log = origLog;
|
||||
}
|
||||
|
||||
expect(notices.some(m => m.includes('Library sync failed for book'))).toBe(false);
|
||||
expect(logs.some(m => m.includes('Library sync failed for book'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('dry-run notify suppression (minor)', () => {
|
||||
test('makeSpecDeps(dryRun=true) -> notify logs instead of raising a Notice', () => {
|
||||
const c = new LibraryController(fakePlugin());
|
||||
const notices: string[] = [];
|
||||
(c as any).notify = (msg: string) => notices.push(msg);
|
||||
const logs: string[] = [];
|
||||
const origLog = console.log;
|
||||
console.log = (msg: string) => {
|
||||
logs.push(msg);
|
||||
};
|
||||
try {
|
||||
const specDeps = (c as any).makeSpecDeps(true);
|
||||
specDeps.notify('«Test Manga» ch. 5 out');
|
||||
} finally {
|
||||
console.log = origLog;
|
||||
}
|
||||
expect(notices).toEqual([]);
|
||||
expect(logs.some(m => m.includes('«Test Manga» ch. 5 out'))).toBe(true);
|
||||
});
|
||||
|
||||
test('makeSpecDeps(dryRun=false) -> notify raises a Notice normally', () => {
|
||||
const c = new LibraryController(fakePlugin());
|
||||
const notices: string[] = [];
|
||||
(c as any).notify = (msg: string) => notices.push(msg);
|
||||
const specDeps = (c as any).makeSpecDeps(false);
|
||||
specDeps.notify('«Test Manga» ch. 5 out');
|
||||
expect(notices).toEqual(['«Test Manga» ch. 5 out']);
|
||||
});
|
||||
|
||||
test('dry-run manga flip scenario end-to-end: syncType(dryRun=true) -> no Notice, log entry present', async () => {
|
||||
const c = new LibraryController(fakePlugin());
|
||||
const notices: string[] = [];
|
||||
(c as any).notify = (msg: string) => notices.push(msg);
|
||||
const logs: string[] = [];
|
||||
const origLog = console.log;
|
||||
console.log = (msg: string) => {
|
||||
logs.push(msg);
|
||||
};
|
||||
|
||||
const flipSpec = {
|
||||
typeName: 'manga' as const,
|
||||
itemType: 'manga_item',
|
||||
folderSettingKey: 'libraryMangaFolder',
|
||||
enabledSettingKey: 'libraryMangaEnabled',
|
||||
throttleMs: 0,
|
||||
hasId: () => true,
|
||||
isActive: () => true,
|
||||
resolve: async () => null,
|
||||
sync: async (_ctx: unknown, deps: { notify: (msg: string) => void }) => {
|
||||
deps.notify('«Test Manga» ch. 5 out');
|
||||
return { content: 'updated content', flipped: true };
|
||||
},
|
||||
};
|
||||
|
||||
(c as any).makeDeps = (spec: unknown, dryRun: boolean) => ({
|
||||
listNotes: async () => [{ path: 'Mangas/Test Manga.md' }],
|
||||
readNote: async () => 'orig content',
|
||||
writeNote: async () => {},
|
||||
sleep: async () => {},
|
||||
log: () => {},
|
||||
specDeps: (c as any).makeSpecDeps(dryRun),
|
||||
});
|
||||
|
||||
try {
|
||||
await c.syncType(flipSpec as any, false, true); // dryRun = true
|
||||
} finally {
|
||||
console.log = origLog;
|
||||
}
|
||||
|
||||
// the run-summary Notice (dry-run or not) is unrelated existing behavior and still fires;
|
||||
// what must NOT happen is the spec's flip-notify reaching a real Notice
|
||||
expect(notices.some(m => m.includes('«Test Manga» ch. 5 out'))).toBe(false);
|
||||
expect(logs.some(m => m.includes('«Test Manga» ch. 5 out'))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue