diff --git a/packages/obsidian/src/library/LibraryController.ts b/packages/obsidian/src/library/LibraryController.ts index 1e73d21..9162eda 100644 --- a/packages/obsidian/src/library/LibraryController.ts +++ b/packages/obsidian/src/library/LibraryController.ts @@ -146,17 +146,25 @@ export class LibraryController { } } + /** Surfaces every counter, not just synced/written/flipped -- a run where fetches are + * silently failing (e.g. Comic Vine error envelope) must not read as an empty-but-clean + * success just because skippedNoData/errors were left out of the message. */ + private buildSyncSummary(spec: MediaTypeSpec, mode: string, report: LibraryReport): string { + let msg = `Library ${mode}sync (${spec.typeName}): ${report.scanned} scanned, ${report.synced} ok, ${report.written} updated, ${report.flipped.length} flipped`; + if (report.skippedStatic) msg += `, ${report.skippedStatic} static`; + if (report.skippedNoId) msg += `, ${report.skippedNoId} no-id`; + if (report.skippedNoData) msg += `, ${report.skippedNoData} no-data (see console)`; + if (report.errors.length) msg += `, ${report.errors.length} errors (see console)`; + return msg; + } + private async runSync(spec: MediaTypeSpec, full: boolean, dryRun: boolean, quiet: boolean): Promise { this.warnMissingKey(spec, quiet); 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)) { - this.notify( - `Library ${mode}sync (${spec.typeName}): ${report.synced} checked, ${report.written} updated, ${report.flipped.length} flipped` + - (report.errors.length ? `, ${report.errors.length} errors (see console)` : ''), - 0, - ); + this.notify(this.buildSyncSummary(spec, mode, report), 0); } return report; } diff --git a/packages/obsidian/src/library/comic.ts b/packages/obsidian/src/library/comic.ts index 18fdd2c..ab27433 100644 --- a/packages/obsidian/src/library/comic.ts +++ b/packages/obsidian/src/library/comic.ts @@ -139,9 +139,27 @@ export function renderComic(r: ComicRecord, myNotes: string, customSections: Cus return fm.join('\n') + '\n' + b.join('\n'); } -async function fetchVolumeResults(query: string, key: string, deps: SpecDeps): Promise { +/** + * Comic Vine returns HTTP 200 even on auth/request failures -- the real signal is the + * envelope's `status_code` (1 = OK; anything else carries a human-readable `error`, e.g. + * "Invalid API Key"). Left un-checked, callers see empty `results` and treat it as + * "nothing found" instead of a real failure. `status_code` absent entirely (as in older + * hand-built test fixtures) is treated as OK -- only an explicit non-1 code trips this. + */ +function checkEnvelope(data: any, deps: SpecDeps): boolean { + if (data && data.status_code !== undefined && data.status_code !== 1) { + const msg = data.error || 'Unknown error'; + deps.log(`Comic Vine error: ${msg}`); + deps.notify(`Comic Vine: ${msg}`); + return false; + } + return true; +} + +async function fetchVolumeResults(query: string, key: string, deps: SpecDeps): Promise { const qs = new URLSearchParams({ api_key: key, format: 'json', filter: `name:${query}`, limit: '10' }); const res = await deps.http(`${COMICVINE_BASE}/volumes/?${qs.toString()}`, {}); + if (!checkEnvelope(res, deps)) return null; return Array.isArray(res?.results) ? res.results : []; } @@ -183,6 +201,7 @@ export const comicSpec: MediaTypeSpec = { if (!query) return null; try { const results = await fetchVolumeResults(query, key, deps); + if (results === null) return null; // envelope error already logged/notified if (results.length === 0) return null; const q = query.toLowerCase(); const exacts = results.filter(r => String(r.name ?? '').toLowerCase() === q); @@ -218,6 +237,8 @@ export const comicSpec: MediaTypeSpec = { return null; } + if (!checkEnvelope(json, deps)) return null; + const result = json?.results; if (!result || !result.name || (result.id != null && String(result.id) !== id)) { // identity-guard: never silently swap to a different volume's data; leave the note diff --git a/tests/fixtures/comicvine-volume.json b/tests/fixtures/comicvine-volume.json index 949ea88..c2d3d7d 100644 --- a/tests/fixtures/comicvine-volume.json +++ b/tests/fixtures/comicvine-volume.json @@ -1,4 +1,6 @@ { + "error": "OK", + "status_code": 1, "results": { "id": 195824, "name": "Absolute Batman", diff --git a/tests/library-comic.test.ts b/tests/library-comic.test.ts index 7213222..2d6e2aa 100644 --- a/tests/library-comic.test.ts +++ b/tests/library-comic.test.ts @@ -329,6 +329,16 @@ describe('comicSpec.resolve', () => { const result = await comicSpec.resolve(ctx, deps); expect(result).toBeNull(); }); + + test('Comic Vine error envelope (invalid key) -> null, logged', async () => { + const deps = makeDeps({ + http: async () => ({ error: 'Invalid API Key', status_code: 100, results: [] }), + getKey: () => 'bad-key', + }); + const result = await comicSpec.resolve(ctxFor({ title: 'Absolute Batman' }), deps); + expect(result).toBeNull(); + expect(deps.logCalls.some(m => m.includes('Invalid API Key'))).toBe(true); + }); }); describe('comicSpec.sync — no id / no key', () => { @@ -385,6 +395,17 @@ describe('comicSpec.sync — comicvine enrich', () => { expect(deps.notifyCalls).toEqual([]); }); + test('Comic Vine error envelope (invalid key) -> null, notify + log', async () => { + const deps = makeDeps({ + http: async () => ({ error: 'Invalid API Key', status_code: 100, results: [] }), + getKey: () => 'bad-key', + }); + const result = await comicSpec.sync(ctxFor({ comicvine_id: '195824' }), deps); + expect(result).toBeNull(); + expect(deps.notifyCalls.some(m => m.includes('Invalid API Key'))).toBe(true); + expect(deps.logCalls.some(m => m.includes('Invalid API Key'))).toBe(true); + }); + test('fetches from GET /volume/4050-{id}/ with api_key + format=json', async () => { let calledUrl = ''; const deps = makeDeps({ diff --git a/tests/library-controller.test.ts b/tests/library-controller.test.ts index 4452c1c..7ca14fb 100644 --- a/tests/library-controller.test.ts +++ b/tests/library-controller.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from 'bun:test'; +import type { MediaTypeSpec } from 'packages/obsidian/src/library/types'; import { bookSpec } from 'packages/obsidian/src/library/book'; import { gameSpec } from 'packages/obsidian/src/library/game'; import { LibraryController } from 'packages/obsidian/src/library/LibraryController'; @@ -421,3 +422,42 @@ describe('dry-run notify suppression (minor)', () => { expect(logs.some(m => m.includes('«Test Manga» ch. 5 out'))).toBe(true); }); }); + +describe('sync summary transparency', () => { + test('scanned + no-data counts surface in summary notice, not hidden behind a silent-looking 0/0', async () => { + const c = new LibraryController(fakePlugin()); + const notices: string[] = []; + (c as any).notify = (msg: string) => notices.push(msg); + (c as any).makeDeps = () => ({ + listNotes: async () => [{ path: 'Mangas/Test.md' }], + readNote: async () => 'content', + writeNote: async () => {}, + sleep: async () => {}, + log: () => {}, + specDeps: fakeSpecDeps(), + }); + + // simulates e.g. a Comic Vine error-envelope: fetch "succeeds" (no throw) but spec.sync + // has no usable data -> counted as skippedNoData rather than a written/errors change + const noDataSpec: MediaTypeSpec = { + typeName: 'manga', + itemType: 'manga_item', + folderSettingKey: 'libraryMangaFolder', + enabledSettingKey: 'libraryMangaEnabled', + throttleMs: 0, + hasId: () => true, + isActive: () => true, + resolve: async () => null, + sync: async () => null, + }; + + const report = await c.syncType(noDataSpec, false); + + expect(report.scanned).toBe(1); + expect(report.skippedNoData).toBe(1); + expect(notices.length).toBe(1); + expect(notices[0]).toContain('1 scanned'); + expect(notices[0]).toContain('0 ok'); + expect(notices[0]).toContain('1 no-data (see console)'); + }); +});