From 54cc44d28ac2b1e253d2d30778af9589c3c33be1 Mon Sep 17 00:00:00 2001 From: Richard Powell Date: Wed, 15 Apr 2026 14:14:31 -0400 Subject: [PATCH 1/3] fix: stop reporting expected errors to Bugsnag AbortError's docstring says "shouldn't be reported as a bug" and shouldReportErrorAsUnexpected correctly returns false for it, setting exitMode to 'expected_error'. But sendErrorToBugsnag ignored that classification and reported all Error instances regardless. This adds an early return in sendErrorToBugsnag when exitMode is 'expected_error', making the reporting layer respect the classification that already exists. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/cli-kit/src/public/node/error-handler.test.ts | 6 +++--- packages/cli-kit/src/public/node/error-handler.ts | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/cli-kit/src/public/node/error-handler.test.ts b/packages/cli-kit/src/public/node/error-handler.test.ts index c17d7f0472c..205ce821c9b 100644 --- a/packages/cli-kit/src/public/node/error-handler.test.ts +++ b/packages/cli-kit/src/public/node/error-handler.test.ts @@ -200,11 +200,11 @@ describe('sends errors to Bugsnag', () => { expect(onNotify).toHaveBeenCalledWith(res.error) }) - test('processes AbortErrors as handled', async () => { + test('skips reporting for expected errors', async () => { const res = await sendErrorToBugsnag(new error.AbortError('In test'), 'expected_error') - expect(res.reported).toEqual(true) + expect(res.reported).toEqual(false) expect(res.unhandled).toEqual(false) - expect(onNotify).toHaveBeenCalledWith(res.error) + expect(onNotify).not.toHaveBeenCalled() }) test.each([null, undefined, {}, {message: 'nope'}])('deals with strange things to throw %s', async (throwable) => { diff --git a/packages/cli-kit/src/public/node/error-handler.ts b/packages/cli-kit/src/public/node/error-handler.ts index 9b764be5c78..484c1f294aa 100644 --- a/packages/cli-kit/src/public/node/error-handler.ts +++ b/packages/cli-kit/src/public/node/error-handler.ts @@ -76,6 +76,11 @@ export async function sendErrorToBugsnag( return {reported: false, error, unhandled: undefined} } + if (exitMode === 'expected_error') { + outputDebug(`Skipping Bugsnag report for expected error`) + return {reported: false, error, unhandled: false} + } + // If the error was unexpected, we flag it as "unhandled" in Bugsnag. This is a helpful distinction. const unhandled = exitMode === 'unexpected_error' From f4a174178a5ff6c55b4bc9c725daff22a758c349 Mon Sep 17 00:00:00 2001 From: Richard Powell Date: Wed, 15 Apr 2026 14:24:18 -0400 Subject: [PATCH 2/3] Address review: consistent return shape and move test to correct describe block - Return `unhandled: undefined` instead of `false` for consistency with other `reported: false` early returns - Move test from "sends errors to Bugsnag" to "skips sending errors to Bugsnag" describe block where it belongs - Assert on the debug log message to match existing test patterns Co-Authored-By: Claude Opus 4.6 (1M context) --- .../cli-kit/src/public/node/error-handler.test.ts | 15 +++++++++------ packages/cli-kit/src/public/node/error-handler.ts | 2 +- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/cli-kit/src/public/node/error-handler.test.ts b/packages/cli-kit/src/public/node/error-handler.test.ts index 205ce821c9b..cba3c1a045d 100644 --- a/packages/cli-kit/src/public/node/error-handler.test.ts +++ b/packages/cli-kit/src/public/node/error-handler.test.ts @@ -174,6 +174,15 @@ describe('skips sending errors to Bugsnag', () => { expect(onNotify).not.toHaveBeenCalled() expect(mockOutput.debug()).toMatch('Skipping Bugsnag report') }) + + test('when error is expected', async () => { + const mockOutput = mockAndCaptureOutput() + const res = await sendErrorToBugsnag(new error.AbortError('In test'), 'expected_error') + expect(res.reported).toEqual(false) + expect(res.unhandled).toBeUndefined() + expect(onNotify).not.toHaveBeenCalled() + expect(mockOutput.debug()).toMatch('Skipping Bugsnag report for expected error') + }) }) describe('sends errors to Bugsnag', () => { @@ -200,12 +209,6 @@ describe('sends errors to Bugsnag', () => { expect(onNotify).toHaveBeenCalledWith(res.error) }) - test('skips reporting for expected errors', async () => { - const res = await sendErrorToBugsnag(new error.AbortError('In test'), 'expected_error') - expect(res.reported).toEqual(false) - expect(res.unhandled).toEqual(false) - expect(onNotify).not.toHaveBeenCalled() - }) test.each([null, undefined, {}, {message: 'nope'}])('deals with strange things to throw %s', async (throwable) => { const res = await sendErrorToBugsnag(throwable, 'unexpected_error') diff --git a/packages/cli-kit/src/public/node/error-handler.ts b/packages/cli-kit/src/public/node/error-handler.ts index 484c1f294aa..5735d2b823a 100644 --- a/packages/cli-kit/src/public/node/error-handler.ts +++ b/packages/cli-kit/src/public/node/error-handler.ts @@ -78,7 +78,7 @@ export async function sendErrorToBugsnag( if (exitMode === 'expected_error') { outputDebug(`Skipping Bugsnag report for expected error`) - return {reported: false, error, unhandled: false} + return {reported: false, error, unhandled: undefined} } // If the error was unexpected, we flag it as "unhandled" in Bugsnag. This is a helpful distinction. From 39c7b05115cbdb144062e0cedff09289745cb5dc Mon Sep 17 00:00:00 2001 From: Richard Powell Date: Wed, 15 Apr 2026 14:28:02 -0400 Subject: [PATCH 3/3] fix: remove extra blank line (prettier) Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/cli-kit/src/public/node/error-handler.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/cli-kit/src/public/node/error-handler.test.ts b/packages/cli-kit/src/public/node/error-handler.test.ts index cba3c1a045d..8f101ab55d4 100644 --- a/packages/cli-kit/src/public/node/error-handler.test.ts +++ b/packages/cli-kit/src/public/node/error-handler.test.ts @@ -209,7 +209,6 @@ describe('sends errors to Bugsnag', () => { expect(onNotify).toHaveBeenCalledWith(res.error) }) - test.each([null, undefined, {}, {message: 'nope'}])('deals with strange things to throw %s', async (throwable) => { const res = await sendErrorToBugsnag(throwable, 'unexpected_error') expect(res.reported).toEqual(false)