Skip to content

Commit 3b8676d

Browse files
w01fgangclaude
andcommitted
fix: guard reporter calls + reach 100% branch coverage
Wrap Reporter.report and Reporter.addBreadcrumbs in try-catch so a failing reporter cannot mask the original error. Log only in debug. Drop unreachable breadcrumbData cache guard and add tests for defensive breadcrumb extraction branches. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 5d898ce commit 3b8676d

2 files changed

Lines changed: 238 additions & 10 deletions

File tree

src/__tests__/coverage-gaps.test.ts

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,51 @@ describe('coverage gaps', () => {
264264
);
265265
expect(out).toEqual({ a: 1 });
266266
});
267+
268+
it('non-object, non-array config returns empty object', () => {
269+
const out = BreadcrumbExtractorUtil.extract(
270+
42 as never,
271+
[{ id: 1 }],
272+
false,
273+
);
274+
expect(out).toEqual({});
275+
});
276+
277+
it('extractor without keys/transform/as returns empty object', () => {
278+
const out = BreadcrumbExtractorUtil.extract(
279+
[{ param: 0 } as never],
280+
[{ id: 1 }],
281+
false,
282+
);
283+
expect(out).toEqual({});
284+
});
285+
286+
it('positional string entry with undefined arg is dropped', () => {
287+
const out = BreadcrumbExtractorUtil.extract(
288+
['label'],
289+
[undefined] as never,
290+
false,
291+
);
292+
expect(out).toEqual({});
293+
});
294+
295+
it('positional entry that is neither string nor array is skipped', () => {
296+
const out = BreadcrumbExtractorUtil.extract(
297+
[42 as never],
298+
[{ id: 1 }],
299+
false,
300+
);
301+
expect(out).toEqual({});
302+
});
303+
304+
it('object-style config with non-array non-function paramConfig returns empty', () => {
305+
const out = BreadcrumbExtractorUtil.extract(
306+
{ 0: 'unexpected' as never },
307+
[{ id: 1 }],
308+
false,
309+
);
310+
expect(out).toEqual({});
311+
});
267312
});
268313

269314
describe('extractDoctests untagged unterminated fence', () => {
@@ -272,4 +317,175 @@ describe('coverage gaps', () => {
272317
expect(extractDoctests(source)).toHaveLength(0);
273318
});
274319
});
320+
321+
describe('reporter error resilience', () => {
322+
let priorReporter: ReturnType<typeof Try.getDefaultReporter>;
323+
324+
beforeEach(() => {
325+
priorReporter = Try.getDefaultReporter();
326+
});
327+
328+
afterEach(() => {
329+
Try.setDefaultReporter(priorReporter);
330+
});
331+
332+
it('sync: reporter.report() throwing does not mask original error', () => {
333+
Try.setDefaultReporter({
334+
report: () => {
335+
throw new Error('reporter down');
336+
},
337+
addBreadcrumbs: vi.fn(),
338+
createWrappedError: (e, m) => {
339+
const w = new Error(m);
340+
w.cause = e;
341+
return w;
342+
},
343+
});
344+
345+
expect(() =>
346+
new Try(() => {
347+
throw new Error('original');
348+
})
349+
.report('wrapped')
350+
.unwrap(),
351+
).toThrow('wrapped');
352+
});
353+
354+
it('async: reporter.report() throwing does not mask original error', async () => {
355+
Try.setDefaultReporter({
356+
report: () => {
357+
throw new Error('reporter down');
358+
},
359+
addBreadcrumbs: vi.fn(),
360+
createWrappedError: (e, m) => {
361+
const w = new Error(m);
362+
w.cause = e;
363+
return w;
364+
},
365+
});
366+
367+
await expect(
368+
new Try(async () => {
369+
throw new Error('original');
370+
})
371+
.report('wrapped')
372+
.unwrap(),
373+
).rejects.toThrow('wrapped');
374+
});
375+
376+
it('reporter.addBreadcrumbs() throwing does not break execution', async () => {
377+
Try.setDefaultReporter({
378+
report: vi.fn(),
379+
addBreadcrumbs: () => {
380+
throw new Error('breadcrumb store unavailable');
381+
},
382+
createWrappedError: (e) => e,
383+
});
384+
385+
const result = await new Try(
386+
async (_ctx: { id: string }) => {
387+
throw new Error('original');
388+
},
389+
{ id: 'x' },
390+
)
391+
.breadcrumbs(['id'])
392+
.default('fallback')
393+
.value();
394+
395+
expect(result).toBe('fallback');
396+
});
397+
398+
it('sync: reporter.addBreadcrumbs() throwing does not break execution', () => {
399+
Try.setDefaultReporter({
400+
report: vi.fn(),
401+
addBreadcrumbs: () => {
402+
throw new Error('breadcrumb store unavailable');
403+
},
404+
createWrappedError: (e) => e,
405+
});
406+
407+
const err = new Try(
408+
(_ctx: { id: string }) => {
409+
throw new Error('original');
410+
},
411+
{ id: 'x' },
412+
)
413+
.breadcrumbs(['id'])
414+
.error();
415+
416+
expect(err).toBeInstanceOf(Error);
417+
expect((err as Error).message).toBe('original');
418+
});
419+
420+
it('reporter errors are logged in debug mode', async () => {
421+
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
422+
Try.setDefaultReporter({
423+
report: () => {
424+
throw new Error('reporter down');
425+
},
426+
addBreadcrumbs: vi.fn(),
427+
createWrappedError: (e) => e,
428+
});
429+
430+
await new Try(async () => {
431+
throw new Error('original');
432+
})
433+
.debug(true)
434+
.report('msg')
435+
.default('fallback')
436+
.value();
437+
438+
expect(spy).toHaveBeenCalled();
439+
spy.mockRestore();
440+
});
441+
442+
it('reporter errors are silent without debug', async () => {
443+
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
444+
Try.setDefaultReporter({
445+
report: () => {
446+
throw new Error('reporter down');
447+
},
448+
addBreadcrumbs: vi.fn(),
449+
createWrappedError: (e) => e,
450+
});
451+
452+
await new Try(async () => {
453+
throw new Error('original');
454+
})
455+
.report('msg')
456+
.default('fallback')
457+
.value();
458+
459+
expect(spy).not.toHaveBeenCalled();
460+
spy.mockRestore();
461+
});
462+
463+
it('addBreadcrumbs errors are logged in debug mode', async () => {
464+
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
465+
Try.setDefaultReporter({
466+
report: vi.fn(),
467+
addBreadcrumbs: () => {
468+
throw new Error('breadcrumb store down');
469+
},
470+
createWrappedError: (e) => e,
471+
});
472+
473+
await new Try(
474+
async (_ctx: { id: string }) => {
475+
throw new Error('original');
476+
},
477+
{ id: 'x' },
478+
)
479+
.debug(true)
480+
.breadcrumbs(['id'])
481+
.default('fallback')
482+
.value();
483+
484+
expect(spy).toHaveBeenCalledWith(
485+
'Reporter.addBreadcrumbs threw:',
486+
expect.any(Error),
487+
);
488+
spy.mockRestore();
489+
});
490+
});
275491
});

src/core/Try.ts

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -814,14 +814,22 @@ export class Try<
814814

815815
/**
816816
* Report error using the configured reporter with context.
817+
* Reporter failures are swallowed so a broken reporter never masks
818+
* the original error from the caller.
817819
*/
818820
private reportError(error: Error): void {
819821
this.addBreadcrumbsIfConfigured();
820822

821-
Try.defaultReporter.report(error, {
822-
message: this.config.message,
823-
tags: this.config.tags,
824-
});
823+
try {
824+
Try.defaultReporter.report(error, {
825+
message: this.config.message,
826+
tags: this.config.tags,
827+
});
828+
} catch (reporterError) {
829+
if (this.config.debug) {
830+
console.error('Reporter.report threw:', reporterError);
831+
}
832+
}
825833
}
826834

827835
/**
@@ -840,11 +848,9 @@ export class Try<
840848
return;
841849
}
842850

843-
if (!this.local.breadcrumbData) {
844-
this.local.breadcrumbData = this.extractAllBreadcrumbData(
845-
this.config.breadcrumbConfig,
846-
);
847-
}
851+
this.local.breadcrumbData = this.extractAllBreadcrumbData(
852+
this.config.breadcrumbConfig,
853+
);
848854

849855
// Mark the guard before short-circuiting on empty data so subsequent
850856
// terminals (and clones sharing `exec`) do not re-run extraction for
@@ -857,7 +863,13 @@ export class Try<
857863

858864
const functionName = this.fn.name || 'anonymous';
859865

860-
Try.defaultReporter.addBreadcrumbs(this.local.breadcrumbData, functionName);
866+
try {
867+
Try.defaultReporter.addBreadcrumbs(this.local.breadcrumbData, functionName);
868+
} catch (reporterError) {
869+
if (this.config.debug) {
870+
console.error('Reporter.addBreadcrumbs threw:', reporterError);
871+
}
872+
}
861873
this.local.breadcrumbsAdded = true;
862874
this.exec.breadcrumbsEmitted.add(this.config.breadcrumbConfig);
863875
}

0 commit comments

Comments
 (0)