Repository navigation
feat(cli): print the API's client error message - #1214
Conversation
A 4xx from the Trunk API can carry {"code": "<CODE>"}. The CLI maps
known codes to its own explanation and prints it in the error report;
the first is TEST_COLLECTION_REQUIRED.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
😎 Merged successfully - details. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1214 +/- ##
==========================================
+ Coverage 83.74% 84.08% +0.33%
==========================================
Files 74 74
Lines 17745 17884 +139
==========================================
+ Hits 14860 15037 +177
+ Misses 2885 2847 -38 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The API now sends a message with each client error code, so the CLI keeps no wording of its own and a code added after this release still reaches its users. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
| if message.is_empty() || message.len() > MAX_CLIENT_ERROR_MESSAGE_LEN { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
I feel like we shouldn't return None for these reasons. I think code without message is still good signal? And also, maybe we should just slice the message to the max len?
There was a problem hiding this comment.
Agreed, done in 8c35f9c. code and message are both optional now, and the body is ignored only when it has neither. A code without a message prints "The Trunk API rejected this request with error code ." A message over 2,048 characters is truncated on a character boundary with a trailing "…" instead of being dropped. New tests cover code-only, whitespace-only message, and truncation.
| }) | ||
| } | ||
|
|
||
| pub(crate) async fn status_code_help<T: FnMut(&Response) -> String>( |
There was a problem hiding this comment.
can we update this fn to accept a struct as an agrument? This is too many args IMO
There was a problem hiding this comment.
Done in 8c35f9c: status_code_help(response, StatusCodeHelp { check_unauthorized, check_not_found, create_error_message, api_host, org_url_slug }). The response stays positional, and all four callers (three in client.rs, one test) use the struct.
| r#"{"code":"SOME_CODE"}"#, | ||
| r#"{"code":"SOME_CODE","message":" "}"#, |
There was a problem hiding this comment.
nit, optional: use the serde_json::json macro for these and the json string above
There was a problem hiding this comment.
Done in 8c35f9c: the test bodies are built with json!.
status_code_help takes its options as a StatusCodeHelp struct. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Overview
A 4xx from the Trunk API can now carry
{"code": "<CODE>", "message": "<text>"}(trunk-io/trunk#34193). The CLI printsmessagein the error report as given, beneath the generic "There was an unexpected error…" line.The server owns the wording, and the CLI keeps no per-code text. So a code added after this release still reaches the users of this release. That's the property a codes-only design can't have: an "upgrade your CLI" code would never reach the old clients it's for.
The first codes are:
TEST_COLLECTION_REQUIRED: an organization on test collections uploaded without--test-collection-id.TEST_COLLECTION_NOT_FOUND: the id matches no collection in the organization. Today that's a 404, which the CLI reports as a bad token.UNSUPPORTED_CLI_VERSION: declared on the server; nothing sends it yet.Details
message.rs:ClientErrorBody { code: Option<String>, message: Option<String> }.client.rs:status_code_helpbecomes async and takes its options as aStatusCodeHelpstruct. On a 4xx it reads the body. If the body has a non-blankcodeormessage, it wraps the reqwest error in aClientErrorExplanation { code, message }.."error_report.rs:ErrorReport::outputprints the message beneath the base message.ApiErrorEndpointcontext on top, and the report prints only the base message, so plain text would sit in the chain unseen.downcast_ref, which reaches context values at any depth.chain()yields anyhow's context wrapper instead of the value.codeis kept on the error, so a later change could branch on it, for example to make a code fatal. Nothing branches on it yet.Considerations
AbortableRetry) andis_unauthorizedboth find it throughchain()/root_cause(), so retries and exit codes are unchanged. A 400 still isn't retried, and still falls back to the test exit code.CLIENT_ERROR_MESSAGESin trunk1), never gRPCdetails. The truncation is a guard against a malformed body, not a trust boundary.status_code_helpalready builds are still never shown. Those areUNAUTHORIZED_CONTEXT,NOT_FOUND_CONTEXTand the per-call messages. On any 4xx the function returns reqwest's error instead. That's a separate fix.Testing
call_api.rstests:400reqwest error as the root cause, and isn't retried.Bad Requestbody and an empty body each leave the error unchanged.error_report.rstest: the message is printed beneath the base message even under an endpoint context.cargo test -p api -p trunk-analytics-cli: all pass except threeswift_test_xunitupload tests. Those fail identically onmainin my environment (no Swift toolchain).cargo clippy --all-targets --all-features --no-deps: no warnings on changed lines.cargo fmtis clean.🤖 Generated with Claude Code