afiqzudinhadi/obsidian-media-db-sync
1
0
Fork 0
Code Issues Pull requests Projects Releases 1 Packages Wiki Activity Actions

fix(library): proper picker-loop isolation with real regression test

Browse source 
Supersedes 4249952's half-fix: try now wraps pickCandidate too; restores
the 19 controller tests that commit deleted; replaces its source-grep
pseudo-tests with a behavioral two-entry isolation test.
This commit is contained in:
afiqzudinhadi 2026-08-05 15:37:19 +08:00
parent 4249952f0a
commit 2686c3fa64
3 changed files with 40 additions and 36 deletions
Download patch file Download diff file Expand all files Collapse all files

15
packages/obsidian/src/library/LibraryController.ts
View file

@ -227,20 +227,19 @@ export class LibraryController {
let skipped = 0;
if (!dryRun) {
for (const entry of report.needsChoice) {
const idx = await this.pickCandidate(entry.filename, entry.candidates);
const candidate = idx != null ? entry.candidates[idx] : undefined;
if (!candidate) {
skipped++;
continue;
}
try {
const idx = await this.pickCandidate(entry.filename, entry.candidates);
const candidate = idx != null ? entry.candidates[idx] : undefined;
if (!candidate) {
skipped++;
continue;
}
const content = await deps.readNote(entry.path);
await deps.writeNote(entry.path, patchFrontmatter(content, candidate.patches, { defaultType: spec.itemType }));
report.resolved.push(entry.path);
picked++;
} catch (e) {
const msg = e instanceof Error ? e.message : String(e);
console.log(`[media-db-library] picker failed for ${entry.filename}: ${msg}`);
deps.log(`picker failed for ${entry.filename}: ${e instanceof Error ? e.message : String(e)}`);
skipped++;
}
}

28
tests/library-controller-isolation.test.ts
View file

@ -1,28 +0,0 @@
import { describe, expect, test } from 'bun:test';
// Test that verifies the try/catch wrapping is syntactically correct
// and the error handling path exists in LibraryController.resolveType
describe('LibraryController per-entry error isolation', () => {
test('try/catch present in resolveType for per-entry failures', async () => {
// Read the source file and verify try/catch is present
const srcText = (await Bun.file('packages/obsidian/src/library/LibraryController.ts').text());
// Verify the fix is in place: try block wrapping readNote/writeNote
expect(srcText).toContain('try {');
expect(srcText).toContain('const content = await deps.readNote(entry.path);');
expect(srcText).toContain('await deps.writeNote(entry.path');
expect(srcText).toContain('} catch (e) {');
expect(srcText).toContain('picker failed for');
expect(srcText).toContain('skipped++;');
});
test('error logging includes filename and message', async () => {
const srcText = await Bun.file('packages/obsidian/src/library/LibraryController.ts').text();
// Verify error log includes entry filename
expect(srcText).toContain('picker failed for ${entry.filename}');
// Verify error message is captured
expect(srcText).toContain('e instanceof Error ? e.message : String(e)');
});
});

33
tests/library-controller.test.ts
View file

@ -554,6 +554,39 @@ describe('resolveType: interactive candidate picker (needsChoice)', () => {
expect(notices[0]).toContain('0 picked, 0 skipped'); // dry-run never picks/skips; needsChoice.length is still visible on the report
});
test('per-entry failure isolated: first entry readNote throws in picker phase, second still picked', async () => {
const c = new LibraryController(fakePlugin());
const notices: string[] = [];
(c as any).notify = (msg: string) => notices.push(msg);
const logs: string[] = [];
const writes: { path: string; content: string }[] = [];
const readCounts: Record<string, number> = {};
(c as any).makeDeps = () => ({
listNotes: async () => [{ path: 'Mangas/Bad.md' }, { path: 'Mangas/Good.md' }],
readNote: async (path: string) => {
readCounts[path] = (readCounts[path] ?? 0) + 1;
// engine resolve pass reads once; picker phase read is the second call
if (path === 'Mangas/Bad.md' && readCounts[path] > 1) throw new Error('note vanished');
return '---\ntype: manga_item\ntitle: X\n---\n\nbody\n';
},
writeNote: async (path: string, content: string) => {
writes.push({ path, content });
},
sleep: async () => {},
log: (m: string) => logs.push(m),
specDeps: fakeSpecDeps(),
});
(c as any).pickCandidate = async () => 0;
const report = await c.resolveType(candidateSpec);
expect(writes.length).toBe(1);
expect(writes[0].path).toBe('Mangas/Good.md');
expect(report.resolved).toEqual(['Mangas/Good.md']);
expect(logs.some(l => l.includes('picker failed for Bad.md') && l.includes('note vanished'))).toBe(true);
expect(notices[0]).toContain('1 picked, 1 skipped');
});
test('no needsChoice entries -> summary omits the picked/skipped clause entirely', async () => {
const c = new LibraryController(fakePlugin());
const notices: string[] = [];