Skip to content

fix: stop reporting expected function errors#7316

Merged
byrichardpowell merged 3 commits intomainfrom
rp/skip-bugsnag-expected-errors
Apr 15, 2026
Merged

fix: stop reporting expected function errors#7316
byrichardpowell merged 3 commits intomainfrom
rp/skip-bugsnag-expected-errors

Conversation

@byrichardpowell
Copy link
Copy Markdown
Contributor

@byrichardpowell byrichardpowell commented Apr 15, 2026

Summary

sendErrorToBugsnag reports all Error instances regardless of their classification. This means AbortError — whose docstring says "shouldn't be reported as a bug" — still gets sent, generating significant noise in Observe.

This PR adds an early return when exitMode === 'expected_error', making the reporting layer respect the classification that shouldReportErrorAsUnexpected already computes correctly.

Problem

The Observe error group Build step "Build Function" failed: Failed to build function. has ~12k events/month across all CLI versions (3.82.0+). Investigation showed:

  • The error group appeared in Observe on April 2 because 3.93.0 introduced a client-steps.ts wrapper that changed the message format, creating a new grouping hash
  • The underlying errors are user code compilation failures (broken Rust/JS function code), not CLI bugs
  • buildFunctionExtension wraps these in AbortError with the comment "To avoid random user-code errors being reported as CLI bugs" — but sendErrorToBugsnag ignores that and reports them anyway

The classification chain works correctly:

AbortError (type=Abort) → shouldReportErrorAsUnexpected() returns false
  → exitMode='expected_error' → sendErrorToBugsnag() ignores exitMode, reports anyway

Fix

5-line early return in sendErrorToBugsnag:

if (exitMode === 'expected_error') {
  outputDebug(`Skipping Bugsnag report for expected error`)
  return {reported: false, error, unhandled: false}
}

Impact

6 callsites use sendErrorToBugsnag. 5 use hardcoded exitMode, 1 computes it dynamically:

Callsite exitMode After change
Main error handler (errorHandler) dynamic AbortError/ExternalError → skipped. BugError/plain Error → still reported
Analytics flush errors expected_error Skipped (telemetry plumbing, not bugs)
Notification system errors unexpected_error Still reported
Metadata provider errors unexpected_error Still reported
Auto-upgrade errors expected_error Skipped (tracked separately via analytics metadata env_auto_upgrade_success)
Notification list errors expected_error Skipped (hidden command)

Every callsite that stops reporting was already explicitly classified as expected_error — either by shouldReportErrorAsUnexpected or by the developer hardcoding it. No signal is lost that isn't tracked through other channels.

Test plan

  • Updated existing test: processes AbortErrors as handledskips reporting for expected errors
  • All 25 error-handler tests pass

🤖 Generated with Claude Code

Closes shop/issues#32997

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) <noreply@anthropic.com>
@byrichardpowell byrichardpowell requested a review from a team as a code owner April 15, 2026 18:15
Copilot AI review requested due to automatic review settings April 15, 2026 18:15
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates CLI Kit’s Bugsnag reporting behavior to respect the already-computed error classification, preventing “expected” (non-bug) errors from being sent to Bugsnag/Observe.

Changes:

  • Add an early return in sendErrorToBugsnag when exitMode === 'expected_error' to skip reporting.
  • Update the existing test to assert that expected errors are not reported.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/cli-kit/src/public/node/error-handler.ts Skips Bugsnag notification when the error is classified as expected_error.
packages/cli-kit/src/public/node/error-handler.test.ts Updates the relevant test to ensure expected errors are not reported.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/cli-kit/src/public/node/error-handler.ts Outdated
Comment thread packages/cli-kit/src/public/node/error-handler.test.ts Outdated
Copy link
Copy Markdown
Contributor

@craigmichaelmartin craigmichaelmartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!!

byrichardpowell and others added 2 commits April 15, 2026 14:24
…ribe 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) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@byrichardpowell byrichardpowell changed the title fix: stop reporting expected errors to Bugsnag fix: stop reporting expected errors Apr 15, 2026
@byrichardpowell byrichardpowell changed the title fix: stop reporting expected errors fix: stop reporting expected function errors Apr 15, 2026
@byrichardpowell byrichardpowell added this pull request to the merge queue Apr 15, 2026
Merged via the queue into main with commit a9cbe62 Apr 15, 2026
24 of 25 checks passed
@byrichardpowell byrichardpowell deleted the rp/skip-bugsnag-expected-errors branch April 15, 2026 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants