Skip to content

refactor: simplify error handling and reporting#226

Open
angeloashmore wants to merge 2 commits into
aa/refactorfrom
aa/error-handling
Open

refactor: simplify error handling and reporting#226
angeloashmore wants to merge 2 commits into
aa/refactorfrom
aa/error-handling

Conversation

@angeloashmore

@angeloashmore angeloashmore commented Jul 15, 2026

Copy link
Copy Markdown
Member

Resolves:

Description

Simplifies the error handling and reporting in src/index.ts. The catch block was 17 near-identical instanceof branches, each repeating the same tracking boilerplate and differing only in the message printed.

  • A pure formatError maps a known error to its user-facing message (or undefined for an unexpected one).
  • Tracking fires once. Segment now records the error message on every failed command; Sentry stays scoped to unexpected errors only.
  • Reworded the Type Builder requirement message and linked to the repository's settings page.

Also refactors src/lib/sentry.ts to drop the @sentry/node-core SDK. It now builds and POSTs the Sentry envelope directly via fetch, removing the dependency.

Checklist

  • A comprehensive Linear ticket, providing sufficient context and details to facilitate the review of the PR, is linked to the PR.
  • If my changes require tests, I added them.
  • If my changes affect backward compatibility, it has been discussed.
  • If my changes require an update to the CONTRIBUTING.md guide, I updated it.

Preview

How to QA 1

🤖 Generated with Claude Code


Note

Medium Risk
Replacing the Sentry SDK with a hand-rolled ingest path and changing when errors are reported vs tracked could alter production error visibility; behavior is limited to CLI telemetry and user messaging.

Overview
Consolidates CLI failure handling in src/index.ts: a new formatError maps known errors to user-facing text, trackCommandEnd runs once per failed command (with the formatted message when available), and Sentry is only invoked for unexpected errors that are rethrown.

src/lib/sentry.ts no longer uses @sentry/node-core; it builds a minimal Sentry envelope and POSTs it with fetch, with setupSentry taking explicit DSN/app metadata from the entrypoint. The @sentry/node-core devDependency and related lockfile entries are removed.

TypeBuilderRequiredError now carries repo and host so the message can link to https://{repo}.{host}/settings/repository/; init passes those values when Type Builder is not enabled.

Reviewed by Cursor Bugbot for commit d0301ab. Bugbot is set up for automated code reviews on this repo. Configure here.

Footnotes

  1. Please use these labels when submitting a review:
    ❓ #ask: Ask a question.
    💡 #idea: Suggest an idea.
    ⚠️ #issue: Strongly suggest a change.
    🎉 #nice: Share a compliment.

Collapse the 17-branch `catch` dispatcher in `src/index.ts` into a single
flow: a pure `formatError` maps a known error to its user-facing message,
tracking fires once, and only unknown errors reach Sentry. Segment now
records the error message on every failed command.

Reword the Type Builder requirement message and link to the repository's
settings page.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@angeloashmore angeloashmore marked this pull request as ready for review July 15, 2026 01:50
Comment thread src/index.ts
Comment thread src/lib/sentry.ts
},
body,
signal: AbortSignal.timeout(2_000),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sentry ingest failures ignored silently

Medium Severity

sentryCaptureError awaits the envelope fetch but never checks whether the response succeeded. Non-2xx or malformed-ingest replies are swallowed by the outer try/catch, so unexpected CLI errors may never reach Sentry despite the refactor’s goal of reporting them.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 682e15d. Configure here.

Comment thread src/lib/sentry.ts
JSON.stringify({ event_id: eventId, sent_at: new Date().toISOString() }),
JSON.stringify({ type: "event" }),
JSON.stringify(event),
].join("\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sentry envelope missing item length

High Severity

The hand-built Sentry envelope posts an event item header with only type, but Relay expects each item header to include the payload byte length. Ingest can reject the body, and failures are swallowed, so unexpected CLI errors may never reach Sentry after removing the SDK.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 682e15d. Configure here.

Comment thread src/index.ts Outdated
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

There are 3 total unresolved issues (including 2 from previous reviews).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit d0301ab. Configure here.

Comment thread src/index.ts
This command requires the Type Builder in your repository.

Enable it by turning off Legacy Builder in your repository settings:
https://${error.repo}.${error.host}/settings/repository/

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Wrong repository settings URL

Medium Severity

The Type Builder error links to https://{repo}.{host}/settings/repository/, but repository settings elsewhere use app/settings/repository on the same host (e.g. wroom.ts, integration tests). Users following the link may hit a missing or wrong page instead of repository settings.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d0301ab. Configure here.

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.

1 participant