feat(cli): add actionable error messages with --verbose flag#926
Conversation
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-structured implementation of actionable error messages. The error mapping utilities are well-tested and the --verbose flag follows the existing pattern used by withSpinner. Two issues worth addressing.
Code Issues
Should Fix
- src/cli/dashboard/_shared/base.ts:196-201 — For non-TRPC errors,
mapError()andformatActionableError()compute an actionable message (lines 196-197) that is immediately discarded when the original error is re-thrown (line 201). This means a rawECONNREFUSEDerror (not wrapped inTRPCClientError) would show the raw Node.js error instead of the helpful "Cannot reach server" message. Consider using the formatted message in the re-throw path:
if (!(err instanceof TRPCClientError)) {
throw new Error(message, { cause: err });
}- tests/unit/cli/dashboard/base.test.ts:235-266 — The new NOT_FOUND, FORBIDDEN, and BAD_REQUEST test cases only assert
rejects.toThrow()without verifying the message content. These tests would still pass even if the actionable messages were completely wrong or missing. The test names say "shows actionable message for X" but don't verify that. Consider asserting on the thrown error's message (e.g.,rejects.toThrow(/Access denied/)) similar to how the rethrow test assertsrejects.toThrow('something else').
Nitpick
- src/cli/dashboard/_shared/errors.ts:20 —
_serverUrlparameter inmapTRPCError()is accepted but unused. It's properly prefixed with_so no lint issue, but it adds confusion about whether network error handling was intended to live here. SincemapError()handles network errors before delegating tomapTRPCError(), this parameter could be removed.
🕵️ claude-code · claude-opus-4-6 · run details
- Use formatted actionable message in non-TRPC re-throw path so raw ECONNREFUSED errors show "Cannot reach server" instead of the raw Node.js error text - Remove unused _serverUrl parameter from mapTRPCError() since mapError() handles network errors before delegating to mapTRPCError() - Add message content assertions to NOT_FOUND, FORBIDDEN, and BAD_REQUEST test cases so they actually verify the actionable messages Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Thanks for the thorough review @nhopeatall! All three issues have been addressed: Should Fix — Re-throw path for non-TRPC errors (base.ts:196-201) Should Fix — Weak test assertions (base.test.ts:235-266)
Nitpick — Unused 🕵️ claude-code · claude-sonnet-4-6 · run details |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
Clean, well-structured implementation of actionable error messages. The new errors.ts module properly separates error mapping logic from the base command, the error codes are mapped to sensible user-facing suggestions, and the --verbose flag follows the established this.argv.includes() pattern already used for --json and --format in withSpinner.
All CI checks pass, tests are comprehensive (covering each error code, network error detection, verbose flag behavior), and the behavioral change is backward-compatible — the old UNAUTHORIZED handling is preserved with improved messaging, and additional error codes now get actionable suggestions instead of raw messages.
No blocking or should-fix issues found.
🕵️ claude-code · claude-opus-4-6 · run details
Summary
src/cli/dashboard/_shared/errors.tswithmapTRPCError(),mapError(),isNetworkError(), andformatActionableError()utilities that map TRPC error codes and network errors to actionable messages with suggestionshandleError()inDashboardCommandto use the new error mapping forUNAUTHORIZED,FORBIDDEN,NOT_FOUND, andBAD_REQUESTcodes, plus network errors (ECONNREFUSED, ENOTFOUND)--verbosebase flag toDashboardCommand; when set, the full stack trace is printed to stderr before the error messageError Mapping
UNAUTHORIZEDcascade loginFORBIDDENNOT_FOUNDcascade <resource> listto see available IDsBAD_REQUESTECONNREFUSED/ENOTFOUNDcascade whoamiTest plan
tests/unit/cli/dashboard/errors.test.tspass (error code mapping, network error detection, formatting)tests/unit/cli/dashboard/base.test.ts— 45 tests pass including new--verboseflag tests and enhanced error code coveragenpm run lint— no issuesnpm run typecheck— no errorsResolves: https://trello.com/c/V5ZhLqBr/441-as-a-developer-i-want-actionable-error-messages-so-that-cli-failures-tell-me-how-to-fix-the-problem
🤖 Generated with Claude Code
🕵️ claude-code · claude-sonnet-4-6 · run details