Skip to content

refactor(error): route uncaught CLI errors through typed-error display#1238

Merged
John-David Dalton (jdalton) merged 6 commits intomainfrom
feat/wire-up-typed-errors
Apr 21, 2026
Merged

refactor(error): route uncaught CLI errors through typed-error display#1238
John-David Dalton (jdalton) merged 6 commits intomainfrom
feat/wire-up-typed-errors

Conversation

@jdalton
Copy link
Copy Markdown
Contributor

@jdalton John-David Dalton (jdalton) commented Apr 18, 2026

Summary

  • The typed-error classes in utils/error/errors.mts (AuthError, InputError, ConfigError, NetworkError, FileSystemError, RateLimitError) plus formatErrorForDisplay / formatErrorForTerminal / formatErrorForJson were only exercised by tests — cli-entry.mts used its own inline if/else dispatch that handled only AuthError / InputError / Error.
  • Replace the inline dispatch in cli-entry.mts with formatErrorForTerminal / formatErrorForJson, so any typed error thrown in production now gets consistent titles, recovery suggestions, and JSON/text formatting (including Caused by [N]: chain walking and Suggested actions: blocks).
  • Convert queryApi's "Socket API base URL is not configured" throw from a generic Error to a ConfigError keyed on CONFIG_KEY_API_BASE_URL. Users now see recovery suggestions directing them to socket config set instead of a bare stack trace.

The remaining typed classes (NetworkError, FileSystemError, RateLimitError) stay parked — forcing them into current CResult-returning sites would be a larger refactor and isn't needed for the display wiring itself to work. They slot in automatically the moment any throw site adopts them.

Test plan

  • pnpm --filter @socketsecurity/cli run test:unit test/unit/utils/error/ — 144 passed
  • pnpm --filter @socketsecurity/cli run test:unit test/unit/utils/socket/ — 189 passed
  • pnpm --filter @socketsecurity/cli run test:unit — 5104 passed
  • pnpm run type — passes
  • pnpm run lint — passes

Note

Medium Risk
Changes the CLI’s top-level exception handler and JSON/terminal error payloads, which may affect automation that parses output and alters user-facing diagnostics. Risk is limited to error-reporting paths (no auth/data-flow changes).

Overview
Uncaught CLI errors are now formatted via the shared typed-error display layer instead of an inline if/else in cli-entry.mts, producing consistent terminal output and --json payloads across error types.

Spinner/error output interaction is fixed by stopping any active default spinner before printing top-level error text.

Error diagnostics are improved by including an Error.cause chain in the non-verbose message (depth-limited), with new unit tests covering cause preservation and truncation.

Misconfiguration handling is made actionable by changing queryApi to throw ConfigError(CONFIG_KEY_API_BASE_URL) when the API base URL is missing, enabling recovery suggestions.

Reviewed by Cursor Bugbot for commit e1525c5. Configure here.

The typed-error classes in `utils/error/errors.mts` (AuthError, InputError,
ConfigError, NetworkError, FileSystemError, RateLimitError) plus the
matching `formatErrorForDisplay`/`formatErrorForTerminal`/`formatErrorForJson`
helpers were only ever exercised by tests — the CLI entry point used its
own inline `if/else` dispatch that handled AuthError/InputError/Error.

- Replace the inline dispatch in `cli-entry.mts` with calls to
  `formatErrorForTerminal` and `formatErrorForJson`, so any typed error
  now thrown in production gets consistent titles, recovery suggestions,
  and JSON/text formatting.
- Convert `queryApi`'s "Socket API base URL is not configured" throw from
  a generic Error to a ConfigError keyed on CONFIG_KEY_API_BASE_URL. Users
  now see recovery suggestions directing them to `socket config set`
  instead of a bare stack trace.

The remaining typed classes (NetworkError, FileSystemError, RateLimitError)
stay parked. Forcing them into current CResult-returning sites would be a
larger refactor and isn't needed for the display wiring itself to work —
they slot in automatically the moment any throw site adopts them.
Comment thread packages/cli/src/cli-entry.mts
Addresses Cursor bugbot feedback on PR #1238. The previous code walked
the cause chain only when `showStack`/verbose was set, so non-debug users
lost the wrapped-error diagnostic context that `messageWithCauses` used
to concatenate automatically.

- Always fold up to 5 `Error.cause` messages into `message` for generic
  Error instances, so the outer message reads like
  `outer: middle: root` — matching `pony-cause`'s behavior.
- The verbose/debug path still renders the richer `Caused by [N]:`
  formatted body with stack traces.
- Add regression tests covering both the cause-chain preservation and
  the 5-level truncation cap.
@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit edf7906. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Comment thread packages/cli/src/cli-entry.mts
@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e1525c5. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e1525c5. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e1525c5. Configure here.

@jdalton
Copy link
Copy Markdown
Contributor Author

Cursor (@cursor) review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit e1525c5. Configure here.

Copy link
Copy Markdown

@m74278803-cmyk MAN (m74278803-cmyk) left a comment

Choose a reason for hiding this comment

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

this.container.appendChild (this.el);
this.ascend ( );

The heatmap you see visualizes the correlation matrix for your housing data. Here's a breakdown of what it tells us:

What is Correlation 

Correlation measures the linear relationship between two variables.

A correlation coefficient ranges from
-1 to +1:
+1 (Dark Red): Indicates a perfect positive linear relationship.

As one variable increases, the other increases proportionally.

    -1 (Dark Blue): Indicates a perfect negative linear relationship. 

As one variable increases,
the other decreases proportionally.
0 (White/Gray): Indicates no linear relationship between the variables.

Interpreting the Heatmap:
    Colors (cmap='coolwarm'):

The coolwarm colormap means warmer colors(reds) represent positive correlations, while cooler colors(blues) represent negative correlations.

The intensity of the color indicates the strength of the correlation.
Values(annot=True, fmt=".2f"):
The numbers displayed in each cell are the Pearson correlation coefficients,
rounded to two decimal places.

    Diagonal: The diagonal elements are always 1 (dark red) because each variable is perfectly correlated with itself.
    Symmetry: The matrix is symmetric, meaning the correlation between 'A' and 'B' is the same as between 'B' and 'A'.

Key Insights from your df_housing heatmap:
You can observe strong positive correlations between total_rooms, total_bedrooms, population, and households. This is expected, as more rooms usually mean more bedrooms, and larger populations would lead to more households.
median_income shows a noticeable positive correlation with median_house_value, which is a common finding: higher income areas tend to have higher house values.
longitude and latitude often show some correlation with other features due to geographical patterns in housing prices, but the relationships might not be strictly linear.

Extract the .cause walk into `appendCauseChain` and use it from every
Error-instanceof branch, not just the generic fallback. AuthError /
NetworkError / FileSystemError / ConfigError / InputError / RateLimitError
constructors don't currently forward `{ cause }`, so the helper is a
no-op for them today — but it future-proofs the display layer so the
day a typed error does carry a cause, it surfaces without another edit.
…ib/errors

Replace the hand-rolled cause-chain walker with the socket-lib
re-export (originally pony-cause). Benefits:

- Single source of truth for cause formatting across CLI + lib.
- Proper cycle detection via a `seen` Set instead of a depth-5 cap
  that could silently truncate legitimate 5+ level chains.
- api.mts drops its direct `pony-cause` import for the same re-export.
- Removes `pony-cause` from root and cli devDependencies (it was
  imported in runtime code, a latent classification bug — socket-lib
  bundles it internally so direct dependence was never needed).

Test updated to assert cycle termination instead of depth cap.
@jdalton John-David Dalton (jdalton) merged commit 1bef659 into main Apr 21, 2026
13 checks passed
@jdalton John-David Dalton (jdalton) deleted the feat/wire-up-typed-errors branch April 21, 2026 14:51
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