diff --git a/src/providers/index.ts b/src/providers/index.ts index 501777d7..b61121a0 100644 --- a/src/providers/index.ts +++ b/src/providers/index.ts @@ -226,14 +226,40 @@ export async function getAllProviders(): Promise { export const providers = coreProviders -export async function discoverAllSessions(providerFilter?: string): Promise { - const allProviders = await getAllProviders() +// Isolate one provider's discovery. A provider that throws (a crafted/corrupt +// file reaching a string op, an unexpected on-disk shape) must never take down +// the whole scan and blank every other provider's usage. Warn once per +// provider per run, then skip it. Mirrors the parse-failure isolation already +// used per-file in parser.ts. +const warnedDiscoveryFailures = new Set() +export async function safeDiscoverSessions(provider: Provider): Promise { + try { + return await provider.discoverSessions() + } catch (err) { + if (!warnedDiscoveryFailures.has(provider.name)) { + warnedDiscoveryFailures.add(provider.name) + const msg = err instanceof Error ? err.message : String(err) + process.stderr.write( + `codeburn: skipped ${provider.name} discovery after an error: ${msg}\n` + ) + } + return [] + } +} + +export async function discoverAllSessions( + providerFilter?: string, + // Injectable for tests so the isolation loop itself is exercised, not just + // the helper. Defaults to the real registry. + providerList?: Provider[], +): Promise { + const allProviders = providerList ?? await getAllProviders() const filtered = providerFilter && providerFilter !== 'all' ? allProviders.filter(p => p.name === providerFilter) : allProviders const all: SessionSource[] = [] for (const provider of filtered) { - const sessions = await provider.discoverSessions() + const sessions = await safeDiscoverSessions(provider) all.push(...sessions) } return all diff --git a/src/usage-aggregator.ts b/src/usage-aggregator.ts index 6fceb7af..38ed0732 100644 --- a/src/usage-aggregator.ts +++ b/src/usage-aggregator.ts @@ -3,7 +3,7 @@ import { CATEGORY_LABELS, type ProjectSummary, type TaskCategory, type DateRange import { type PeriodData, type ProviderCost, type BreakdownArrays, type MenubarPayload, buildMenubarPayload } from './menubar-json.js' import { parseAllSessions, filterProjectsByName, filterProjectsByDays } from './parser.js' import { findUnpricedModels, getLocalModelSavingsConfigHash, getPriceOverridesConfigHash, getShortModelName } from './models.js' -import { getAllProviders } from './providers/index.js' +import { getAllProviders, safeDiscoverSessions } from './providers/index.js' import { aggregateProjectsIntoDays, buildPeriodDataFromDays } from './day-aggregator.js' import { aggregateModelEfficiency } from './model-efficiency.js' import { aggregateModels } from './models-report.js' @@ -200,7 +200,7 @@ export async function buildMenubarPayloadForRange(periodInfo: PeriodInfo, opts: } for (const p of allProviders) { if (providers.some(pc => pc.name === p.displayName)) continue - const sources = await p.discoverSessions() + const sources = await safeDiscoverSessions(p) if (sources.length > 0) providers.push({ name: p.displayName, cost: 0 }) } } else { diff --git a/tests/provider-registry.test.ts b/tests/provider-registry.test.ts index 004dc02c..1c346dd8 100644 --- a/tests/provider-registry.test.ts +++ b/tests/provider-registry.test.ts @@ -1,5 +1,16 @@ -import { describe, it, expect } from 'vitest' -import { providers, getAllProviders, getProvider } from '../src/providers/index.js' +import { describe, it, expect, vi } from 'vitest' +import { providers, getAllProviders, getProvider, safeDiscoverSessions, discoverAllSessions } from '../src/providers/index.js' +import type { Provider } from '../src/providers/types.js' + +function fakeProvider(name: string, discover: Provider['discoverSessions']): Provider { + return { + name, + displayName: name, + modelDisplayName: (m: string) => m, + toolDisplayName: (t: string) => t, + discoverSessions: discover, + } as unknown as Provider +} describe('provider registry', () => { it('has core providers registered synchronously', () => { @@ -117,4 +128,44 @@ describe('provider registry', () => { expect(cursor.modelDisplayName('grok-code-fast-1')).toBe('Grok Code Fast') expect(cursor.modelDisplayName('unknown-model')).toBe('unknown-model') }) + + describe('provider-discovery isolation', () => { + it('safeDiscoverSessions returns [] and warns once instead of propagating', async () => { + const warn = vi.spyOn(process.stderr, 'write').mockReturnValue(true) + const boom = fakeProvider('boom-helper', async () => { throw new Error('crafted file blew up') }) + try { + await expect(safeDiscoverSessions(boom)).resolves.toEqual([]) + expect(warn.mock.calls.length).toBeGreaterThanOrEqual(1) + expect(String(warn.mock.calls[0]![0])).toContain('boom-helper') + // Deduped on repeat within the same run: no additional warning. + const afterFirst = warn.mock.calls.length + await safeDiscoverSessions(boom) + expect(warn.mock.calls.length).toBe(afterFirst) + } finally { + warn.mockRestore() + } + }) + + it('discoverAllSessions drops a throwing provider but keeps the healthy ones', async () => { + const warn = vi.spyOn(process.stderr, 'write').mockReturnValue(true) + const boom = fakeProvider('boom-loop', async () => { throw new Error('kaboom') }) + const ok1 = fakeProvider('ok1', async () => [{ path: '/a.jsonl', project: 'p1', provider: 'ok1' }]) + const ok2 = fakeProvider('ok2', async () => [{ path: '/b.jsonl', project: 'p2', provider: 'ok2' }]) + try { + // A throwing provider in the middle must not abort the loop. + const sources = await discoverAllSessions('all', [ok1, boom, ok2]) + expect(sources.map(s => s.path)).toEqual(['/a.jsonl', '/b.jsonl']) + expect(warn.mock.calls.some(c => String(c[0]).includes('boom-loop'))).toBe(true) + } finally { + warn.mockRestore() + } + }) + + it('discoverAllSessions honors the provider filter', async () => { + const ok1 = fakeProvider('keep', async () => [{ path: '/keep.jsonl', project: 'k', provider: 'keep' }]) + const ok2 = fakeProvider('drop', async () => [{ path: '/drop.jsonl', project: 'd', provider: 'drop' }]) + const sources = await discoverAllSessions('keep', [ok1, ok2]) + expect(sources.map(s => s.path)).toEqual(['/keep.jsonl']) + }) + }) })