fix(cli): show all API errors and surface plain error/message bodies#351
fix(cli): show all API errors and surface plain error/message bodies#351wyattjoh wants to merge 1 commit into
Conversation
Parse the raw response body in formatApiBody so multi-error responses are all shown (joined by newlines) instead of only the first error, and bodies that carry a plain error or message field are surfaced directly rather than as raw JSON. The error classes keep their structured fields unchanged.
|
Stack: wyattjoh/errors-eval Part of a stacked-prs chain. Do not merge manually. |
🦋 Changeset detectedLatest commit: b2eb2c8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughWalkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli-core/src/cli-program.test.ts (1)
281-291: ⚡ Quick winAdd a regression case for non-string fallback fields.
Current fallback tests cover string
error/messageonly. Add a case like{ error: { detail: "x" } }(and/or non-stringmessage) to lock in expected fallback behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli-core/src/cli-program.test.ts` around lines 281 - 291, The existing test cases for formatApiBody only verify fallback behavior with string values for the error and message fields. Add new regression test cases that verify the fallback behavior when error and/or message fields contain non-string values (such as objects with a detail property). Create test cases similar to the existing ones but pass objects instead of strings as the error or message field values to the formatApiBody function to ensure the fallback logic handles non-string types correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli-core/src/cli-program.ts`:
- Around line 163-164: The code at lines 163-164 returns parsed.error and
parsed.message without verifying they are strings, causing non-string values to
display as [object Object] in CLI output. Add type guards using typeof checks to
ensure both parsed.error and parsed.message are strings before returning them.
If either field exists but is not a string type, skip that return statement and
continue to the next fallback check or provide an appropriate string
representation.
---
Nitpick comments:
In `@packages/cli-core/src/cli-program.test.ts`:
- Around line 281-291: The existing test cases for formatApiBody only verify
fallback behavior with string values for the error and message fields. Add new
regression test cases that verify the fallback behavior when error and/or
message fields contain non-string values (such as objects with a detail
property). Create test cases similar to the existing ones but pass objects
instead of strings as the error or message field values to the formatApiBody
function to ensure the fallback logic handles non-string types correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dc35e48e-589b-44a7-9a5e-54d0b1d93dfe
📒 Files selected for processing (3)
.changeset/api-error-formatting.mdpackages/cli-core/src/cli-program.test.tspackages/cli-core/src/cli-program.ts
| if (parsed.error) return parsed.error; | ||
| if (parsed.message) return parsed.message; |
There was a problem hiding this comment.
Guard fallback fields to strings only.
Line 163 and Line 164 return parsed values without type checks. If error/message is non-string JSON, CLI output degrades to [object Object] instead of a useful fallback.
Suggested fix
- if (parsed.error) return parsed.error;
- if (parsed.message) return parsed.message;
+ if (typeof parsed.error === "string") return parsed.error;
+ if (typeof parsed.message === "string") return parsed.message;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (parsed.error) return parsed.error; | |
| if (parsed.message) return parsed.message; | |
| if (typeof parsed.error === "string") return parsed.error; | |
| if (typeof parsed.message === "string") return parsed.message; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli-core/src/cli-program.ts` around lines 163 - 164, The code at
lines 163-164 returns parsed.error and parsed.message without verifying they are
strings, causing non-string values to display as [object Object] in CLI output.
Add type guards using typeof checks to ensure both parsed.error and
parsed.message are strings before returning them. If either field exists but is
not a string type, skip that return statement and continue to the next fallback
check or provide an appropriate string representation.
Summary
When an API request fails, the CLI now parses the response body so every error in a multi-error response is shown, joined by newlines, instead of only the first one. Response bodies that carry a plain
errorormessagefield instead of a Clerkerrorsarray are surfaced directly rather than printed as raw JSON. The error classes and their structured fields are unchanged, so existing consumers that readcodeandmeta(such as the deploy command) keep working.Test plan
bun run typecheckbun run lintbun run format:checkbun run test(1679 pass)