docs(claude): align Error Messages with fleet doctrine#1261
Closed
John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
Closed
docs(claude): align Error Messages with fleet doctrine#1261John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
John-David Dalton (jdalton) wants to merge 5 commits intomainfrom
Conversation
…s doc Restructure the CLI-specific Error Messages section to match the updated fleet doctrine from socket-repo-template: - Keep the four ingredients (What / Where / Saw vs. wanted / Fix). - Add audience-based length guidance (library API terse / CLI verbose / programmatic rule-only). `InputError`/`AuthError` usages are verbose-tier. - Tighten baseline rules to one-liners; drop narrative phrasing. - Preserve the CLI-specific examples (--pull-request, socket init, AuthError) — they earn their keep as real anti-pattern fodder. Add `docs/references/error-messages.md` with fleet-wide worked examples so CLAUDE.md stays tight and the rich anti-patterns live once.
Point readers at @socketsecurity/lib/arrays' list-formatting helpers from CLAUDE.md (one-line rule) and the worked-examples references doc (new "Formatting lists of values" section).
Fleet-wide migration to the caught-value helpers in
@socketsecurity/lib/errors.
- pnpm-workspace.yaml: catalog bump 5.21.0 → 5.24.0.
- 18 src files under packages/cli/src: replace every
`e instanceof Error ? e.message : String(e)` and `UNKNOWN_ERROR`
fallback with `errorMessage(e)`; replace bare `x instanceof Error`
boolean checks with `isError(x)`; replace
`e instanceof Error ? e.stack : undefined` with `errorStack(e)`.
- packages/cli/src/utils/error/errors.mts: drop the locally-defined
`isErrnoException` (which accepted any `code !== undefined`) and
re-export the library's stricter string-code variant.
- packages/cli/src/commands/manifest/convert-{gradle,sbt}-to-maven.mts:
rename a local `const errorMessage` → `summary` to free the
identifier for the imported helper.
- packages/cli/src/utils/telemetry/service.mts: rename two local
`const errorMessage = …` variables to `errMsg` inside catch
blocks for the same reason.
- docs/references/error-messages.md: pick up the new "Working with
caught values" section from socket-repo-template.
Out of scope (intentionally left):
- Exit-code checks on child-process results (`'code' in e` where
`code` is a number, e.g. display.mts:257). `isErrnoException`
requires a string code and would wrongly return false.
- The local `getErrorMessage` / `getErrorMessageOr` helpers in
errors.mts — callers outside this file still use them; a
broader refactor to the library `errorMessage` is follow-up.
Pre-commit tests skipped (DISABLE_PRECOMMIT_TEST); `pnpm run type`
and `pnpm run lint` pass.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
Author
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Cursor bugbot flagged the while(currentCause) loop in displayError: it walks the .cause chain manually but was calling errorMessage() on each level, which itself walks the entire remaining chain via messageWithCauses. For A → B → C, that printed "B msg: C msg" at depth 1, then "C msg" at depth 2, showing C's message twice. Switch to reading `.message` directly (matching the pre-PR behavior the bot pointed to) so each iteration emits only that level's text. Fall back to `String(currentCause)` for non-Error nodes in the chain. Drop the now-unused `errorMessage` import. Reported on PR #1261.
…ntics
The debugApiResponse test expected errorMessage('String error') to
return the 'Unknown error' sentinel, matching the old local shim's
behavior that treated any non-Error caught value as unusable. The
catalog bump to @socketsecurity/lib 5.24 switched debug.mts to the
upstream errorMessage, which preserves non-empty primitives as-is —
only empty strings, null, undefined, and plain objects coerce to the
sentinel. Assert on 'String error' to pin the current contract.
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ 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 178568c. Configure here.
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 24, 2026
Fold the fleet-doctrine error-message rewrite + @socketsecurity/lib/errors
migration into this branch so the whole error-messages initiative lands
as one PR:
- CLAUDE.md Error Messages section aligned with fleet doctrine
- New docs/references/error-messages.md (worked examples, anti-patterns)
- packages/cli/src/utils/error/{errors,display}.mts + callers migrated
to @socketsecurity/lib/errors helpers (isError, errorMessage,
errorStack, isErrnoException). CLI-local isErrnoException removed.
- @socketsecurity/lib bump in pnpm-workspace.yaml
Supersedes #1261.
4 tasks
Contributor
Author
|
Superseded by #1257 — the entire content of this PR (CLAUDE.md doctrine rewrite, Verified on the merged branch locally:
Closing here; review continues on #1257. |
This was referenced Apr 24, 2026
John-David Dalton (jdalton)
added a commit
that referenced
this pull request
Apr 24, 2026
…rary migration (#1257) * fix(cli): align utils/update/ + utils/command/ error messages with 4-ingredient strategy Rewrites error messages across packages/cli/src/utils/update/ and packages/cli/src/utils/command/ to follow the What / Where / Saw vs. wanted / Fix strategy from CLAUDE.md. Sources: - utils/update/checker.mts: 8 messages (URL validation, package name / registry URL validation, version-response validation). Each now names the function, the received type, and what a valid value looks like. - utils/update/manager.mts: 3 messages (mirror guards for name / version / ttl). Still warn-and-return-false, but the text now tells the caller exactly which option was wrong. - utils/command/registry-core.mts: 6 messages (command / alias registration conflicts, middleware next() misuse, flag parsing failures). Each now names the offending command, flag name, or index so debuggers don't need to read source. Tests updated: - test/unit/utils/update/checker.test.mts: 6 assertions (switched to regex) - test/unit/utils/update/manager.test.mts: 3 assertions (switched to expect.stringContaining) - test/unit/utils/command/registry-core.test.mts: 5 assertions All 153 tests in the affected suites pass. Follows strategy from #1254. Part of the multi-PR series started by #1255 (commands/) and continued in #1256 (utils/dlx/). * fix(cli): address Cursor bugbot findings on error messages Two issues flagged by Cursor bugbot on #1257: 1. (Medium) registry-core.mts middleware error reported the wrong offending middleware index. `index` tracks the highest dispatched position, not the caller; when dispatch(i) is re-entered after a double-next(), the offender held middleware[i - 1]'s next closure. Fixed to use `i - 1` with a comment explaining why. 2. (Low) checker.mts error referenced a non-existent `UpdateChecker.fetch()` — the object is actually exported as `NetworkUtils`. Renamed in both the error and its test regex. Caught by #1257 bugbot review. * docs(claude): align Error Messages with fleet doctrine, add references doc Restructure the CLI-specific Error Messages section to match the updated fleet doctrine from socket-repo-template: - Keep the four ingredients (What / Where / Saw vs. wanted / Fix). - Add audience-based length guidance (library API terse / CLI verbose / programmatic rule-only). `InputError`/`AuthError` usages are verbose-tier. - Tighten baseline rules to one-liners; drop narrative phrasing. - Preserve the CLI-specific examples (--pull-request, socket init, AuthError) — they earn their keep as real anti-pattern fodder. Add `docs/references/error-messages.md` with fleet-wide worked examples so CLAUDE.md stays tight and the rich anti-patterns live once. * docs(claude): reference joinAnd / joinOr helpers in Error Messages Point readers at @socketsecurity/lib/arrays' list-formatting helpers from CLAUDE.md (one-line rule) and the worked-examples references doc (new "Formatting lists of values" section). * chore: bump @socketsecurity/lib to 5.24.0 and adopt error helpers Fleet-wide migration to the caught-value helpers in @socketsecurity/lib/errors. - pnpm-workspace.yaml: catalog bump 5.21.0 → 5.24.0. - 18 src files under packages/cli/src: replace every `e instanceof Error ? e.message : String(e)` and `UNKNOWN_ERROR` fallback with `errorMessage(e)`; replace bare `x instanceof Error` boolean checks with `isError(x)`; replace `e instanceof Error ? e.stack : undefined` with `errorStack(e)`. - packages/cli/src/utils/error/errors.mts: drop the locally-defined `isErrnoException` (which accepted any `code !== undefined`) and re-export the library's stricter string-code variant. - packages/cli/src/commands/manifest/convert-{gradle,sbt}-to-maven.mts: rename a local `const errorMessage` → `summary` to free the identifier for the imported helper. - packages/cli/src/utils/telemetry/service.mts: rename two local `const errorMessage = …` variables to `errMsg` inside catch blocks for the same reason. - docs/references/error-messages.md: pick up the new "Working with caught values" section from socket-repo-template. Out of scope (intentionally left): - Exit-code checks on child-process results (`'code' in e` where `code` is a number, e.g. display.mts:257). `isErrnoException` requires a string code and would wrongly return false. - The local `getErrorMessage` / `getErrorMessageOr` helpers in errors.mts — callers outside this file still use them; a broader refactor to the library `errorMessage` is follow-up. Pre-commit tests skipped (DISABLE_PRECOMMIT_TEST); `pnpm run type` and `pnpm run lint` pass. * fix(cli): stop duplicating cause messages in display.mts loop Cursor bugbot flagged the while(currentCause) loop in displayError: it walks the .cause chain manually but was calling errorMessage() on each level, which itself walks the entire remaining chain via messageWithCauses. For A → B → C, that printed "B msg: C msg" at depth 1, then "C msg" at depth 2, showing C's message twice. Switch to reading `.message` directly (matching the pre-PR behavior the bot pointed to) so each iteration emits only that level's text. Fall back to `String(currentCause)` for non-Error nodes in the chain. Drop the now-unused `errorMessage` import. Reported on PR #1261. * test(cli): update debug.test for @socketsecurity/lib/errors 5.24 semantics The debugApiResponse test expected errorMessage('String error') to return the 'Unknown error' sentinel, matching the old local shim's behavior that treated any non-Error caught value as unusable. The catalog bump to @socketsecurity/lib 5.24 switched debug.mts to the upstream errorMessage, which preserves non-empty primitives as-is — only empty strings, null, undefined, and plain objects coerce to the sentinel. Assert on 'String error' to pin the current contract.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CLAUDE.md's Error Messages section to match the updated fleet doctrine fromsocket-repo-template: four ingredients stay, audience-based length guidance (library API terse / CLI verbose / programmatic rule-only) added, baseline rules tightened.InputError/AuthErrorexamples — they're better training material than generic ones.docs/references/error-messages.mdwith cross-fleet worked examples and anti-patterns so CLAUDE.md stays tight (CLAUDE.md is always loaded; we want it under the 40 KB ceiling and short enough to re-read).Test plan
CLAUDE.mdis still < 40 KB (12.5 KB).Note
Medium Risk
Touches many CLI error paths and bumps
@socketsecurity/libto5.24.0, which could subtly change surfaced error/cause strings and telemetry/debug output. No core business logic changes, but behavior changes are concentrated around failure handling.Overview
Updates
CLAUDE.md’s error-message guidance (audience-based length + tightened rules) and addsdocs/references/error-messages.mdwith worked examples/anti-patterns.Across the CLI, standardizes caught-error handling by adopting
@socketsecurity/lib/errorshelpers (isError,errorMessage,errorStack,isErrnoException) instead of ad-hocinstanceof Error ? … : String(…)patterns, including command execution results, renderers, update checks, GitHub/GitLab providers, and debug/telemetry plumbing. This removes the CLI’s localisErrnoExceptionimplementation and updates the related unit test expectation.Reviewed by Cursor Bugbot for commit 178568c. Configure here.