diff --git a/docs-yml.schema.json b/docs-yml.schema.json index 9d86fee3f48f..347e200d6356 100644 --- a/docs-yml.schema.json +++ b/docs-yml.schema.json @@ -5991,6 +5991,13 @@ "classic" ] }, + "docs.ApiReferenceLayout": { + "type": "string", + "enum": [ + "indented", + "cards" + ] + }, "docs.LayoutConfig": { "type": "object", "properties": { @@ -6148,6 +6155,17 @@ ], "description": "Sets which layout to use for changelog pages.\n\n@default: `timeline`\n\n- `timeline` renders the searchable timeline-of-cards layout.\n- `classic` renders the legacy stacked-entries layout with a per-page table of contents." }, + "api-reference-layout": { + "oneOf": [ + { + "$ref": "#/definitions/docs.ApiReferenceLayout" + }, + { + "type": "null" + } + ], + "description": "Sets how nested object fields are rendered in the API reference.\n\n@default: `indented`\n\n- `indented` renders each nested level as an indented branch with a connector line and a color-coded path breadcrumb.\n- `cards` renders each nested level as a bordered card." + }, "show-nav-availability-badges": { "oneOf": [ { diff --git a/fern-yml.schema.json b/fern-yml.schema.json index 9a4b28fbfc4a..1b304b677b2e 100644 --- a/fern-yml.schema.json +++ b/fern-yml.schema.json @@ -2897,6 +2897,13 @@ "classic" ] }, + "api-reference-layout": { + "type": "string", + "enum": [ + "indented", + "cards" + ] + }, "show-nav-availability-badges": { "type": "boolean" } diff --git a/fern/apis/docs-yml/definition/docs.yml b/fern/apis/docs-yml/definition/docs.yml index 3601e7bd892a..de105f2989c6 100644 --- a/fern/apis/docs-yml/definition/docs.yml +++ b/fern/apis/docs-yml/definition/docs.yml @@ -1005,6 +1005,17 @@ types: - `timeline` renders the searchable timeline-of-cards layout. - `classic` renders the legacy stacked-entries layout with a per-page table of contents. + api-reference-layout: + type: optional + availability: in-development + docs: | + Sets how nested object fields are rendered in the API reference. + + @default: `indented` + + - `indented` renders each nested level as an indented branch with a connector line and a color-coded path breadcrumb. + - `cards` renders each nested level as a bordered card. + show-nav-availability-badges: type: optional availability: in-development @@ -1249,6 +1260,11 @@ types: - timeline - classic + ApiReferenceLayout: + enum: + - indented + - cards + AudienceId: type: string docs: ID of audience which is used to filter the API spec. diff --git a/generators/cli/changes/0.37.0/credentialless-hint-network-errors-and-auth-status-format.yml b/generators/cli/changes/0.37.0/credentialless-hint-network-errors-and-auth-status-format.yml new file mode 100644 index 000000000000..01b2e2d0ba98 --- /dev/null +++ b/generators/cli/changes/0.37.0/credentialless-hint-network-errors-and-auth-status-format.yml @@ -0,0 +1,64 @@ +# yaml-language-server: $schema=../../../../fern-changes-yml.schema.json + +- summary: | + The "no credentials" hint is no longer tied to 401/403. The condition it + describes — this request went out carrying nothing — has nothing to do with + the status code, but the hint was gated on those two, so an API that + expresses "not authenticated" any other way said nothing useful. ElevenLabs + answers 404 for a workspace scoped to the anonymous principal (deliberate: a + 401 would confirm the resource exists), so a logged-out user read + `Workspace 1anonymous1 not found` with no indication that the fix was + `auth login`. + + Any failed status now carries a `No credentials were sent with this + request. Set ...` note in `error.help` when the provider had nothing for + that endpoint. This only annotates: the error keeps its class and exit code, + and the request is still sent, because the server — not the spec — is the + authority on whether an endpoint needed auth. Public endpoints are + unaffected, and 401/403 still escalate to `error[auth]` with exit 2. + type: fix + +- summary: | + The "check for shadowing" advice on a rejected credential now appears only + when there is more than one visible credential source. Shadowing means one + source silently outranking another, which cannot happen with a single + source — yet the note sent every user to `auth status` to compare a list of + one against itself. On a scope failure (`OAuth token does not have required + permissions: user_read`) that was worse than noise: the credential is the + right one from the right place, and the advice points away from the real + fix. The source is still named whenever credentials were sent, since knowing + which one was used is useful either way. + type: fix + +- summary: | + Transport failures no longer report a fabricated HTTP status. A request that + never reached a server — DNS, TLS, connection refused, timeout — was folded + into the generic internal-error shape and emitted `code: 500` with + `reason: "internalError"`, telling a consumer the *API* had failed and + inviting a retry against a host that was never contacted. These now produce + `reason: "networkError"` and **no** `code` field, since the field is + documented as the HTTP status and there is none. Exit code is unchanged at + 5, and the human rendering is labelled `error[network]:`. + + The message also keeps the part that matters: `reqwest` stops at "error + sending request for url (...)" and buries "Connection refused" two levels + down its `source()` chain, so the chain is now walked and joined. SDK-mode + CLIs report network and timeout failures the same way as OpenAPI-mode ones. + type: fix + +- summary: | + `auth status` now honors the global `--format`. It gated JSON output on its + own `--json` boolean, so `auth status --format json` printed the human table + with no error and no hint — the first command an agent runs, silently + ignoring the flag it uses everywhere else. `--format`, `--human`, the + `_OUTPUT` env var and the piped default now all apply; `--json` + remains as an alias so existing scripts keep working. + type: fix + +- summary: | + Fixed a literal `` placeholder appearing in credential hints. The + keyring source rendered `populated by \` auth login\`` instead of naming + the binary. CLIs using an OAuth login flow were unaffected — that provider + overrides the hint and substituted correctly — so the placeholder only + reached users on bearer, header and basic schemes. + type: fix diff --git a/generators/cli/changes/0.37.0/drop-duplicate-error-details.yml b/generators/cli/changes/0.37.0/drop-duplicate-error-details.yml new file mode 100644 index 000000000000..bb32a38e6c93 --- /dev/null +++ b/generators/cli/changes/0.37.0/drop-duplicate-error-details.yml @@ -0,0 +1,15 @@ +- summary: | + `error.details` no longer repeats the sentence already reported in + `error.message`. The parsed server body is still carried structurally, so + fields a consumer cannot get anywhere else — `request_id`, a validation + error's `loc`, a GraphQL error's `path`/`extensions` — are unchanged, but the + one field the message was lifted from is removed, and `details` is + omitted entirely when the body said nothing beyond that message (so + `{"detail": "Not Found"}` now yields just `code`/`message`/`reason`). + + Removal is by position, not by value: when several entries report the same + problem — two required fields both failing with `"field required"`, the + common multi-field validation shape — only the entry the sentence came from + loses it. Dropping every string equal to the message would leave the + remaining problems unattributable to their `loc`. + type: fix diff --git a/generators/cli/changes/0.37.0/fix-duplicate-api-error-output.yml b/generators/cli/changes/0.37.0/fix-duplicate-api-error-output.yml new file mode 100644 index 000000000000..b6f30431ae31 --- /dev/null +++ b/generators/cli/changes/0.37.0/fix-duplicate-api-error-output.yml @@ -0,0 +1,36 @@ +# yaml-language-server: $schema=../../../../fern-changes-yml.schema.json + +- summary: | + Fixed API errors being printed twice and with the server's JSON body escaped + inside the `message` string. An error is now rendered in exactly one + representation — the JSON envelope on stdout for machine formats + (`json`/`jsonl`/`yaml`, which includes the piped/non-TTY default), or a single + `error[api]: ` line on stderr for `table`/`csv` — and the server's + body is parsed rather than stringified: a sentence lands in `message`, the + parsed body in `error.details`, and `reason` is derived from the HTTP status + (`internalServerError` for 500) instead of always `httpError`. Top-level + shapes such as `{"status", "message"}`, `{"error": ""}`, RFC 7807 + `detail`/`title`, and OAuth 2.0 `error_description` are now recognized. Two + consequences worth noting for anyone scripting against these fields: 429 now + always reports `reason: "rateLimited"`. Previously a generated CLI reported + either that or `tooManyRequests` for the same status depending on which code + path produced the error — the runtime's `sdk_executor` already said + `rateLimited`, while the generated request adapter said `tooManyRequests`. + Both now come from one mapping. Also, + `error.code` is always the HTTP status the server answered with. A body's own + `code` never overrides it — not an internal numbering (`{"code": 100234}`, + which used to truncate into a nonsense status), and not one that merely looks + like a status, which could disagree with what was actually served and point + the docs link at the wrong page. The body's `code` is still available under + `error.details`. + type: fix + +- summary: | + Added a global `--human` flag, shorthand for `--format table`, for the one + case TTY detection gets wrong: a person piping into a pager or a file still + wants the interactive rendering. Without it the existing TTY-aware default + applies (`table` on a terminal, `json` when piped), and passing it alongside + `--format` is rejected. There is deliberately no `--json` counterpart: + endpoints with a request body already spell that flag `--json `, so + the name cannot mean "render JSON" everywhere; use `--format json`. + type: feat diff --git a/generators/cli/changes/0.37.0/jsonl-errors-build-failures-and-help-catalog.yml b/generators/cli/changes/0.37.0/jsonl-errors-build-failures-and-help-catalog.yml new file mode 100644 index 000000000000..60e4eddaffd0 --- /dev/null +++ b/generators/cli/changes/0.37.0/jsonl-errors-build-failures-and-help-catalog.yml @@ -0,0 +1,46 @@ +# yaml-language-server: $schema=../../../../fern-changes-yml.schema.json + +- summary: | + `--format jsonl` now emits errors as a single line. The envelope was + pretty-printed regardless of format, so a line-delimited consumer — the only + reason to pick NDJSON — read a bare `{` and failed on the first line. Errors + under `jsonl` are compact; `json` and `yaml` keep the readable multi-line + rendering. + type: fix + +- summary: | + A request that fails to build is now a validation error rather than an + internal one. `reqwest`'s `build()` fails on a malformed URL or header + value, which comes from `--base-url` or a flag the user typed, but the + failure was reported as `code: 500` / `reason: "internalError"` with exit 5. + A mistyped `--base-url` claimed a server status for a request that was never + sent and buried a fixable input error under an internal-error exit code. It + now reports `code: 400` / `reason: "validationError"` with exit 3. + type: fix + +- summary: | + ` --help --format json` now emits the operation catalog, as the + generated README has always documented. It printed the human help instead, + so the README's own example (`--help --format json | jq …`) failed with a + jq parse error. It now returns exactly what `--schema` returns for the same + scope. + + Only an *explicit* machine `--format` triggers this — the piped default does + not, so ` --help | less` still shows help. A path with no catalog entry + (`auth login`, `completion`, `man` — built-ins, not API operations) falls + back to prose rather than erroring; `--schema` on such a path still reports + `discoveryError`, since asking for a document that does not exist is a real + error. The README example was also + corrected: the catalog is an object with `globalFlags` and `operations`, so + counting operations is `jq '.operations | length'`, not `jq 'length'` (which + counted the two top-level keys). + type: fix + +- summary: | + Removed `error::print_error_json` from the vendored runtime. It wrote the + JSON envelope to stdout unconditionally — the exact double-output behaviour + the rest of this release removes — so it was a working copy of the bug left + reachable under an inviting name. It had no callers in the runtime, in the + generator's emitted code, or in any generated CLI. `write_error_json`, which + honours the resolved format, is the entry point. + type: fix diff --git a/generators/cli/changes/0.37.0/keyring-denial-is-an-auth-error.yml b/generators/cli/changes/0.37.0/keyring-denial-is-an-auth-error.yml new file mode 100644 index 000000000000..54d4881ae244 --- /dev/null +++ b/generators/cli/changes/0.37.0/keyring-denial-is-an-auth-error.yml @@ -0,0 +1,40 @@ +# yaml-language-server: $schema=../../../../fern-changes-yml.schema.json + +- summary: | + A credential the CLI could not read is no longer treated as a credential the + user never stored. Denying the OS keychain prompt made + `AuthCredentialSource::resolve` return "nothing here" — the same answer as an + empty keyring — so the CLI sent the request unauthenticated and the user saw + whatever the API says to an anonymous caller. Against ElevenLabs that is + `error[api]: Workspace 1anonymous1 not found.` with exit 1: a 404 for a + problem that is an authentication failure. + + Credential resolution now distinguishes "not configured" from "configured but + unreadable". On the request path a denied or broken keyring surfaces as + `error[auth]` with exit 2, naming the backend failure. A fallback chain stops + at the denial instead of falling through to a later empty source, which would + have masked it again. Probing paths — `auth status`, `has_credentials` — still + degrade to "no credential" rather than aborting, since a status listing should + report what is usable, not fail. + type: fix + +- summary: | + Terminal error output no longer repeats itself. Services routinely echo one + token across several fields — ElevenLabs sends `code`, `status` and `type` all + restating the same `workspace_not_found` — which turned a one-fact 404 into + five lines saying the same thing four times. A detail line whose value already + appeared in the message, the reason, or an earlier line is now dropped, and a + sole wrapper object (`{"detail": {...}}`) no longer prefixes every line with a + word that names nothing the reader is looking for. + + A service-specific reason is now shown on the message line, in parentheses. + It was previously the one useful field that appeared in the JSON envelope but + never in the terminal. A reason derived from the HTTP status (`notFound` on a + 404) stays hidden, since it says nothing the label and message do not. + + error[api]: Workspace 1anonymous1 not found. (workspace_not_found) + request_id: 6c312fb855fc7b476570b44e2b660988 + type: not_found + + The JSON envelope is unchanged and still carries every field. + type: fix diff --git a/generators/cli/changes/0.37.0/show-error-details-and-summarise-page-bodies.yml b/generators/cli/changes/0.37.0/show-error-details-and-summarise-page-bodies.yml new file mode 100644 index 000000000000..3049423aa69a --- /dev/null +++ b/generators/cli/changes/0.37.0/show-error-details-and-summarise-page-bodies.yml @@ -0,0 +1,42 @@ +# yaml-language-server: $schema=../../../../fern-changes-yml.schema.json + +- summary: | + Terminal error output now reports what the server said beyond the failure + sentence. A field-level rejection used to read as `error[api]: field + required` with no way to tell which field — the `loc` was in the JSON + envelope, which is exactly what the person in the terminal cannot see. The + parsed body is now flattened one `path: value` per line under the message: + + error[api]: value is not a valid email address + detail[0].loc: body, email + detail[0].type: value_error.email + + Lists of scalars stay on one line, since splitting FastAPI's two-element + `loc` buries the field name it exists to report. Output is capped at ten + lines, with a final line naming what was elided and pointing at + `--format json`. The JSON envelope is unchanged. + type: fix + +- summary: | + Non-JSON error bodies no longer break the "`message` is one sentence" + guarantee. A CDN or load balancer answering with an HTML page used to have + its whole document — doctype, newlines and markup — pasted into + `error.message`; the previous 500-byte clip did not help, because a short + page is still multi-line markup. Markup and over-long bodies now yield + `HTTP 502 badGateway (non-JSON response, 1284 bytes)` in `message`, with the + bytes preserved (whitespace-collapsed and clipped) in `error.details.body`. + A short plain-text body — the common proxy case — is still the message + itself, now collapsed onto one line if the server wrapped it. + type: fix + +- summary: | + Two error bodies that produced an unhelpful envelope now read correctly. An + empty body gave `message: ""`, which reads as the CLI having lost the error + rather than the server having sent nothing; it now reports + `HTTP `. A body that is a bare JSON string + (`"Service temporarily unavailable"`) was summarised as + `HTTP 500 internalServerError` with the only text the server sent exiled to + `details`; it is now the message. A body that is a bare JSON list of + problems is also recognised, matching the existing `errors`/`detail` + handling one level out. + type: fix diff --git a/generators/cli/changes/0.37.0/split-error-help-from-message.yml b/generators/cli/changes/0.37.0/split-error-help-from-message.yml new file mode 100644 index 000000000000..9eb4f5978224 --- /dev/null +++ b/generators/cli/changes/0.37.0/split-error-help-from-message.yml @@ -0,0 +1,11 @@ +- summary: | + Made the error envelope's `message` field one sentence for every error class. + Advice that is not part of what the server reported — today the "Credentials + were supplied via: ..." hint shown on 401/403 — moved out of `message` into + its own `error.help` field, so a consumer keying off `error.message` sees the + same shape whether or not the CLI had advice to offer. The docs link (opt-in + via `error_docs_base_url`, previously human-only) now also appears as + `error.docs_url`, so the JSON envelope no longer silently drops information + the human rendering shows. Human output keeps the same order, one item per + line: message, then what the server said beyond it, then hint, then link. + type: fix diff --git a/generators/cli/changes/0.37.0/split-usage-text-out-of-error-message.yml b/generators/cli/changes/0.37.0/split-usage-text-out-of-error-message.yml new file mode 100644 index 000000000000..ffa1eeafbcba --- /dev/null +++ b/generators/cli/changes/0.37.0/split-usage-text-out-of-error-message.yml @@ -0,0 +1,15 @@ +- summary: | + Usage errors (a mistyped subcommand, a bad flag value, conflicting flags) no + longer stuff clap's whole multi-line block into `error.message` when output is + machine-readable. The failure sentence stays in `message`, clap's `tip:` and + `[possible values: ...]` lines move to `error.help`, the usage string becomes + `error.usage`, and the `For more information, try '--help'` boilerplate is + dropped — so every field is a single line, as with API errors. Human output on + a terminal is unchanged: it still prints clap's full rendering, tip and usage + included. + + Only clap's own blocks are split. A multi-line message from one of the CLI's + validators — schema validation listing one bullet per violation, say — passes + through whole, since applying the split there would demote the violations to + `help` and leave `message` as the bare header. + type: fix diff --git a/generators/cli/sdk/.sdk-ignore.json b/generators/cli/sdk/.sdk-ignore.json index 154ce95b9641..e60e9bb89362 100644 --- a/generators/cli/sdk/.sdk-ignore.json +++ b/generators/cli/sdk/.sdk-ignore.json @@ -1,5 +1,6 @@ [ "cli/openapi-fixture/**", + "tests/**", "src/bin/strip_schema.rs", ".gitignore", "README.md", diff --git a/generators/cli/sdk/src/app.rs b/generators/cli/sdk/src/app.rs index adcc33729b6a..e4771a84aff8 100644 --- a/generators/cli/sdk/src/app.rs +++ b/generators/cli/sdk/src/app.rs @@ -701,6 +701,10 @@ impl CliApp { let ctx = ErrorDisplayContext { docs_base_url: self.error_docs_base_url.clone(), help_hint: Some(help_hint), + // Resolved from raw argv because an error can precede clap parsing; + // the resolver is the same one the success path uses, so errors land + // in whichever representation the caller already asked for. + format: crate::formatter::resolve_format_from_raw_args(&str_args, &self.name), }; match self.dispatch_pipeline(args, out).await { Ok(PipelineOutcome::Success) => 0, @@ -750,7 +754,12 @@ impl CliApp { // binding to own the path wins. A real `Err` from one binding is // logged and the walk continues — one broken spec cannot mask its // sibling's surface. - if crate::cli_args::wants_schema(&str_args) { + // `--schema`, or `--help` with an explicit machine format — the spelling + // the generated README has always documented for the operation catalog. + let explicit_schema = crate::cli_args::wants_schema(&str_args); + let help_as_schema = crate::cli_args::wants_help(&str_args) + && crate::formatter::explicit_machine_format(&str_args); + if explicit_schema || help_as_schema { let path = crate::cli_args::extract_subcommand_path(&str_args); if path.is_empty() { @@ -840,10 +849,17 @@ impl CliApp { ), } } - return Err(CliError::Discovery(format!( - "--schema: no binding contains path `{}`", - path.join(" ") - ))); + // A path with no schema is an error for an explicit `--schema` — + // the caller asked for a document that does not exist. It must not + // be one for `--help`: built-in subcommands (`auth login`, + // `completion`, `man`) are not API operations, and someone asking + // for help on one should get help, not `discoveryError` exit 4. + if explicit_schema { + return Err(CliError::Discovery(format!( + "--schema: no binding contains path `{}`", + path.join(" ") + ))); + } } // 0c. --spec / --spec-raw: emit embedded OpenAPI spec(s) and exit. @@ -914,6 +930,19 @@ impl CliApp { .value_name("FORMAT") .global(true), ) + // The one shorthand a caller needs beyond TTY detection: piping + // already selects the machine rendering, so only the reverse — + // "I am a human, but I am paging or redirecting" — needs a flag. + // Deliberately not mirrored by a `--json`: endpoints with a request + // body already spell that flag `--json `. + .arg( + clap::Arg::new("human") + .long("human") + .help("Force human-readable output (same as --format table) even when piped") + .action(clap::ArgAction::SetTrue) + .conflicts_with("format") + .global(true), + ) .arg( clap::Arg::new("base-url") .long("base-url") @@ -1212,6 +1241,7 @@ fn graft_merged_subtree( // avoid clap panic on duplicates). let mut seen_arg_ids: std::collections::HashSet = [ "format".to_string(), + "human".to_string(), "base-url".to_string(), "user-agent-suffix".to_string(), "schema".to_string(), diff --git a/generators/cli/sdk/src/auth/credential.rs b/generators/cli/sdk/src/auth/credential.rs index e041e12ed24d..2515011c8952 100644 --- a/generators/cli/sdk/src/auth/credential.rs +++ b/generators/cli/sdk/src/auth/credential.rs @@ -25,6 +25,8 @@ use std::sync::Arc; use secrecy::SecretString; +use crate::error::CliError; + use crate::auth::keyring_store::active_store; type CredentialClosure = Arc Option + Send + Sync>; @@ -133,8 +135,26 @@ impl AuthCredentialSource { /// `Debug`/`Display`/panic messages. Callers that need the raw `&str` /// (to build a `HeaderValue`, base64-encode for basic auth, etc.) /// must opt in explicitly via [`ExposeSecret::expose_secret`]. + /// + /// Treats every failure as "no credential". Correct for *probing* — an + /// `auth status` listing or a `has_credentials` check should report what is + /// usable, not abort — but wrong on the request path, where a credential + /// the user has but the CLI could not read must not silently downgrade the + /// request to unauthenticated. Use [`try_resolve`](Self::try_resolve) there. pub fn resolve(&self) -> Option { - match self { + self.try_resolve().ok().flatten() + } + + /// Resolve the value, distinguishing "not configured" (`Ok(None)`) from + /// "configured but unreadable" (`Err`). + /// + /// The distinction only exists for the keyring today, and it is the one + /// that matters: denying the OS keychain prompt used to be indistinguishable + /// from having stored nothing, so the CLI sent the request anonymously and + /// the user saw whatever the server says to a stranger — a 404, typically — + /// instead of an auth failure. + pub fn try_resolve(&self) -> Result, CliError> { + let value = match self { AuthCredentialSource::Env(name) => std::env::var(name) .ok() .map(|v| v.trim().to_string()) @@ -144,16 +164,29 @@ impl AuthCredentialSource { AuthCredentialSource::File(path) => read_credential_file(path), AuthCredentialSource::Literal(v) if v.is_empty() => None, AuthCredentialSource::Literal(v) => Some(SecretString::from(v.clone())), - AuthCredentialSource::Chain(sources) => sources.iter().find_map(|s| s.resolve()), - AuthCredentialSource::Closure(f, _) => f().filter(|v| !v.is_empty()).map(SecretString::from), + // First source that yields a value wins; a source that *errors* + // stops the walk rather than falling through, so a denied keychain + // is reported instead of being masked by a later empty rung. + AuthCredentialSource::Chain(sources) => { + let mut found = None; + for source in sources { + if let Some(v) = source.try_resolve()? { + found = Some(v); + break; + } + } + found + } + AuthCredentialSource::Closure(f, _) => { + f().filter(|v| !v.is_empty()).map(SecretString::from) + } AuthCredentialSource::Keyring { service, account } => active_store() - .get(service, account) - .ok() - .flatten() + .get(service, account)? .filter(|v| !v.is_empty()) .map(SecretString::from), AuthCredentialSource::Missing => None, - } + }; + Ok(value) } /// The environment-variable name backing this source, if it is an @@ -184,7 +217,15 @@ impl AuthCredentialSource { } AuthCredentialSource::Closure(_, Some(hint)) => vec![hint.clone()], AuthCredentialSource::Keyring { service, account } => { - vec![format!("keyring entry {service}:{account} (populated by ` auth login`)")] + // `service` is the CLI's binary name (see the variant's docs), + // so the command is spellable. It used to render as a literal + // ``, which reached users on any non-OAuth scheme — the + // OAuth provider overrides `credential_hints` and substituted + // correctly, hiding the placeholder on the one path anyone + // looked at. + vec![format!( + "keyring entry {service}:{account} (populated by `{service} auth login`)" + )] } AuthCredentialSource::Literal(_) | AuthCredentialSource::Closure(_, None) @@ -711,6 +752,59 @@ mod tests { assert_eq!(resolved(&s), None); } + /// A store that fails every read, standing in for a denied OS keychain + /// prompt (`keyring::Error::PlatformFailure` on macOS). + #[derive(Debug)] + struct DeniedKeyringStore; + + impl crate::auth::keyring_store::KeyringStore for DeniedKeyringStore { + fn get(&self, _: &str, _: &str) -> Result, CliError> { + Err(CliError::Auth("keyring get failed: User denied access".into())) + } + fn set(&self, _: &str, _: &str, _: &str) -> Result<(), CliError> { + unreachable!() + } + fn delete(&self, _: &str, _: &str) -> Result<(), CliError> { + unreachable!() + } + fn backend_label(&self) -> String { + "denied".into() + } + } + + #[test] + #[serial_test::serial] + fn a_denied_keyring_is_an_error_not_an_absent_credential() { + use crate::auth::keyring_store::set_active_store; + set_active_store(Arc::new(DeniedKeyringStore)); + + let s = AuthCredentialSource::keyring("svc", "OAuth2"); + // The request path must be able to tell these apart: sending the + // request anonymously makes the server answer as if to a stranger, and + // the user sees a 404 instead of an auth failure. + let err = s.try_resolve().unwrap_err(); + assert!(matches!(err, CliError::Auth(_)), "got: {err:?}"); + assert_eq!(err.exit_code(), CliError::EXIT_CODE_AUTH); + // The probing path still degrades to "no credential", so `auth status` + // and `has_credentials` keep working rather than aborting. + assert!(s.resolve().is_none()); + } + + #[test] + #[serial_test::serial] + fn a_chain_stops_at_a_denied_keyring_rather_than_falling_through() { + use crate::auth::keyring_store::set_active_store; + set_active_store(Arc::new(DeniedKeyringStore)); + + // Env is empty, so without propagation the chain would report "no + // credential anywhere" and the denial would vanish. + let chain = AuthCredentialSource::any([ + AuthCredentialSource::keyring("svc", "OAuth2"), + AuthCredentialSource::from_env("__FERN_TEST_DEFINITELY_UNSET"), + ]); + assert!(matches!(chain.try_resolve(), Err(CliError::Auth(_)))); + } + #[test] #[serial_test::serial] fn keyring_source_empty_value_resolves_to_none() { diff --git a/generators/cli/sdk/src/auth/error.rs b/generators/cli/sdk/src/auth/error.rs index 19c7554bf5f3..05984c4bad85 100644 --- a/generators/cli/sdk/src/auth/error.rs +++ b/generators/cli/sdk/src/auth/error.rs @@ -13,8 +13,6 @@ //! //! [hcf]: crate::auth::AuthProvider::has_credentials_for -use serde_json::Value; - use crate::auth::provider::{AuthProvider, EndpointAuthMetadata}; use crate::error::CliError; @@ -60,28 +58,89 @@ pub fn handle_error_response( let base = parse_api_error(status, error_body); return Err(decorate_with_source_hint(base, &hints)); } + return Err(parse_api_error(status, error_body)); + } + + // Any other failure, sent without credentials. The condition the friendly + // message above describes — "this request carried no auth" — has nothing to + // do with the status code, but the message was gated on 401/403, so an API + // that expresses "not authenticated" some other way said nothing useful. + // + // ElevenLabs answers 404 for a workspace scoped to the anonymous principal + // (a deliberate pattern: a 401 would confirm the resource exists), so a + // logged-out user saw `Workspace 1anonymous1 not found` with no hint that + // the fix was `auth login`. + // + // This only *annotates* — the error keeps its class and exit code, because + // the server, not the spec, is the authority on whether this endpoint + // needed auth at all. Public endpoints must keep working. + if !provider.has_credentials_for(endpoint) { + let base = parse_api_error(status, error_body); + if let Some(note) = missing_credentials_note(provider) { + return Err(decorate_with_help(base, note)); + } + return Err(base); } Err(parse_api_error(status, error_body)) } -/// Append a "Credentials were supplied via: …" line to an existing -/// `CliError::Api` message, preserving the structured fields. For -/// non-`Api` variants (defensive — shouldn't happen here), pass through. -fn decorate_with_source_hint(err: CliError, hints: &[String]) -> CliError { - let joined = hints.join(", "); +/// Advice for a request that went out with no credentials at all, or `None` +/// when the CLI declares no auth sources to point at. +fn missing_credentials_note(provider: &dyn AuthProvider) -> Option { + let hints = dedup_preserve_order(provider.credential_hints()); + if hints.is_empty() { + return None; + } + Some(format!( + "No credentials were sent with this request. Set {}.", + hints.join(", ") + )) +} + +/// Attach `note` as the error's `help`, leaving every other field alone. +fn decorate_with_help(err: CliError, note: String) -> CliError { match err { - CliError::Api { code, message, reason } => CliError::Api { + CliError::Api { + code, + message, + reason, + details, + .. + } => CliError::Api { code, - message: format!( - "{message}\nCredentials were supplied via: {joined}. \ - Run `auth status` to see all visible sources and check for shadowing." - ), + message, reason, + details, + help: Some(note), }, other => other, } } +/// Attach a "Credentials were supplied via: …" note to an existing +/// `CliError::Api`. The note goes in `help` rather than `message` — it is +/// advice about the failure, not part of what the server reported. +/// +/// Naming the source that was used is worth saying whenever credentials were +/// sent. The follow-up — "check for shadowing" — is not: shadowing means one +/// source silently outranking another, which cannot happen when there is only +/// one. On a single-source CLI it sent the reader to `auth status` to compare a +/// list of one against itself, which is busywork at best and, on a scope +/// failure like `OAuth token does not have required permissions`, actively +/// points away from the real fix. +fn decorate_with_source_hint(err: CliError, hints: &[String]) -> CliError { + let joined = hints.join(", "); + let note = if hints.len() > 1 { + format!( + "Credentials were supplied via: {joined}. \ + Run `auth status` to see all visible sources and check for shadowing." + ) + } else { + format!("Credentials were supplied via: {joined}.") + }; + decorate_with_help(err, note) +} + /// Deduplicate strings while preserving first-seen order. fn dedup_preserve_order(items: Vec) -> Vec { let mut seen = std::collections::HashSet::new(); @@ -94,38 +153,7 @@ fn dedup_preserve_order(items: Vec) -> Vec { /// Shared parsing for the auth-aware error handler. Returns a structured /// [`CliError::Api`] whether or not the body was JSON. fn parse_api_error(status: reqwest::StatusCode, error_body: &str) -> CliError { - if let Ok(error_json) = serde_json::from_str::(error_body) { - if let Some(err_obj) = error_json.get("error") { - let code = err_obj - .get("code") - .and_then(|c| c.as_u64()) - .unwrap_or(status.as_u16() as u64) as u16; - let message = err_obj - .get("message") - .and_then(|m| m.as_str()) - .unwrap_or("Unknown error") - .to_string(); - let reason = err_obj - .get("errors") - .and_then(|e| e.as_array()) - .and_then(|arr| arr.first()) - .and_then(|e| e.get("reason")) - .and_then(|r| r.as_str()) - .or_else(|| err_obj.get("reason").and_then(|r| r.as_str())) - .unwrap_or("unknown") - .to_string(); - return CliError::Api { - code, - message, - reason, - }; - } - } - CliError::Api { - code: status.as_u16(), - message: error_body.to_string(), - reason: "httpError".to_string(), - } + crate::error::api_error_from_body(status.as_u16(), error_body) } #[cfg(test)] @@ -183,7 +211,12 @@ mod tests { ) .unwrap_err(); match err { - CliError::Api { code, message, reason } => { + CliError::Api { + code, + message, + reason, + .. + } => { assert_eq!(code, 401); assert!(message.contains("invalid authentication credentials")); assert_eq!(reason, "authError"); @@ -223,10 +256,15 @@ mod tests { ) .unwrap_err(); match err { - CliError::Api { code, message, reason } => { + CliError::Api { + code, + message, + reason, + .. + } => { assert_eq!(code, 500); assert_eq!(message, "Internal Server Error Text"); - assert_eq!(reason, "httpError"); + assert_eq!(reason, "internalServerError"); } _ => panic!("Expected Api"), } @@ -426,15 +464,54 @@ mod tests { ) .unwrap_err(); match err { - CliError::Api { message, .. } => { - assert!(message.contains("__FERN_TEST_SHADOW_TOKEN")); - assert!(message.contains("auth status")); + CliError::Api { message, help, .. } => { + // The hint is advice, so it lives in `help` — `message` stays + // exactly what the server said. + assert_eq!(message, "bad token"); + let help = help.expect("credential source hint"); + assert!(help.contains("__FERN_TEST_SHADOW_TOKEN")); + // One source, so nothing can be shadowing anything: the + // `auth status` follow-up would send the reader to compare a + // list of one against itself. + assert!(!help.contains("auth status"), "got: {help}"); } other => panic!("expected Api with source-hint suffix, got: {other:?}"), } std::env::remove_var("__FERN_TEST_SHADOW_TOKEN"); } + #[test] + fn shadowing_advice_appears_only_with_more_than_one_source() { + // Two visible sources is the situation the advice was written for: a + // stale env var can outrank a fresh `auth login`, and which one won is + // not observable from the error alone. + std::env::set_var("__FERN_TEST_CHAIN_A", "stale"); + let p = BearerAuthProvider::new( + "bearer", + AuthCredentialSource::any([ + AuthCredentialSource::from_env("__FERN_TEST_CHAIN_A"), + AuthCredentialSource::keyring("mycli", "bearer"), + ]), + ); + let err = handle_error_response::<()>( + reqwest::StatusCode::UNAUTHORIZED, + r#"{"error":{"code":401,"message":"bad token"}}"#, + &p, + &EndpointAuthMetadata::unspecified(), + ) + .unwrap_err(); + match err { + CliError::Api { help, .. } => { + let help = help.expect("credential source hint"); + assert!(help.contains("__FERN_TEST_CHAIN_A"), "got: {help}"); + assert!(help.contains("keyring entry mycli:bearer"), "got: {help}"); + assert!(help.contains("check for shadowing"), "got: {help}"); + } + other => panic!("expected Api, got: {other:?}"), + } + std::env::remove_var("__FERN_TEST_CHAIN_A"); + } + #[test] fn forbidden_with_credentials_discloses_source() { std::env::set_var("__FERN_TEST_FORBIDDEN_TOKEN", "x"); @@ -450,8 +527,10 @@ mod tests { ) .unwrap_err(); match err { - CliError::Api { message, .. } => { - assert!(message.contains("__FERN_TEST_FORBIDDEN_TOKEN")); + CliError::Api { help, .. } => { + assert!(help + .expect("credential source hint") + .contains("__FERN_TEST_FORBIDDEN_TOKEN")); } other => panic!("expected Api with source-hint suffix, got: {other:?}"), } @@ -473,14 +552,81 @@ mod tests { ) .unwrap_err(); match err { - CliError::Api { message, .. } => { + CliError::Api { message, help, .. } => { assert!(!message.contains("__FERN_TEST_NONAUTH_TOKEN")); + assert!(help.is_none()); } _ => panic!("expected Api"), } std::env::remove_var("__FERN_TEST_NONAUTH_TOKEN"); } + #[test] + fn a_credentialless_request_is_annotated_whatever_the_status() { + // ElevenLabs answers 404 for a workspace scoped to the anonymous + // principal, so a logged-out user used to read `Workspace 1anonymous1 + // not found` with nothing pointing at `auth login`. The hint was gated + // on 401/403; the condition it describes is status-independent. + std::env::remove_var("__FERN_TEST_ABSENT_TOKEN"); + let p = BearerAuthProvider::new( + "bearer", + AuthCredentialSource::from_env("__FERN_TEST_ABSENT_TOKEN"), + ); + let err = handle_error_response::<()>( + reqwest::StatusCode::NOT_FOUND, + r#"{"detail":{"message":"Workspace 1anonymous1 not found.","code":"workspace_not_found"}}"#, + &p, + &EndpointAuthMetadata::unspecified(), + ) + .unwrap_err(); + match err { + CliError::Api { + code, + message, + reason, + help, + .. + } => { + // Class and exit code are untouched: the server, not the spec, + // decides whether this endpoint needed auth. + assert_eq!(code, 404); + assert_eq!(reason, "workspace_not_found"); + assert_eq!(message, "Workspace 1anonymous1 not found."); + let help = help.expect("missing-credentials note"); + assert!(help.contains("No credentials were sent"), "got: {help}"); + assert!(help.contains("__FERN_TEST_ABSENT_TOKEN"), "got: {help}"); + } + other => panic!("expected Api, got: {other:?}"), + } + assert_eq!( + CliError::api(404, "x", "y").exit_code(), + CliError::EXIT_CODE_API + ); + } + + #[test] + fn a_credentialled_request_gets_no_missing_credentials_note() { + // The note must not fire just because a call failed — only when the + // request actually went out with nothing. + std::env::set_var("__FERN_TEST_PRESENT_TOKEN", "x"); + let p = BearerAuthProvider::new( + "bearer", + AuthCredentialSource::from_env("__FERN_TEST_PRESENT_TOKEN"), + ); + let err = handle_error_response::<()>( + reqwest::StatusCode::NOT_FOUND, + r#"{"detail":"Nope"}"#, + &p, + &EndpointAuthMetadata::unspecified(), + ) + .unwrap_err(); + match err { + CliError::Api { help, .. } => assert!(help.is_none(), "got: {help:?}"), + other => panic!("expected Api, got: {other:?}"), + } + std::env::remove_var("__FERN_TEST_PRESENT_TOKEN"); + } + #[test] fn dedup_removes_duplicates_preserving_order() { let input = vec!["a".into(), "b".into(), "a".into(), "c".into(), "b".into()]; diff --git a/generators/cli/sdk/src/auth/login.rs b/generators/cli/sdk/src/auth/login.rs index a114d0b345c3..a2c396ccca2f 100644 --- a/generators/cli/sdk/src/auth/login.rs +++ b/generators/cli/sdk/src/auth/login.rs @@ -399,7 +399,16 @@ fn handle_status( login_flows: &[DynLoginFlow], out: &mut W, ) -> Result<(), CliError> { - let as_json = matches.get_flag("json"); + // `--json` is this subcommand's own boolean and predates the global + // `--format`. Honouring only that made `auth status --format json` print + // the human table with no error and no hint — the one command an agent + // needs before anything else, silently ignoring the flag it was taught to + // use everywhere else. `--format` (and the piped default behind it) now + // decides too; `--json` stays as an alias so existing scripts keep working. + let as_json = matches.get_flag("json") + || crate::formatter::OutputPipeline::from_matches(matches, cli_name) + .map(|pipeline| pipeline.format.is_machine_readable()) + .unwrap_or(false); let store = active_store(); let backend = store.backend_label(); diff --git a/generators/cli/sdk/src/auth/schemes.rs b/generators/cli/sdk/src/auth/schemes.rs index c5704e48f2b3..482111a8fdb1 100644 --- a/generators/cli/sdk/src/auth/schemes.rs +++ b/generators/cli/sdk/src/auth/schemes.rs @@ -61,7 +61,10 @@ impl AuthProvider for BearerAuthProvider { request: reqwest::RequestBuilder, _endpoint: &EndpointAuthMetadata, ) -> Result { - let Some(token) = self.token.resolve() else { + // `try_resolve`, not `resolve`: a credential the user stored but the + // CLI could not read (a denied keychain prompt) must fail here rather + // than sending the request unauthenticated. + let Some(token) = self.token.try_resolve()? else { return Ok(request); }; // Avoid `RequestBuilder::bearer_auth` — it panics on tokens with @@ -179,8 +182,10 @@ impl AuthProvider for BasicAuthProvider { request: reqwest::RequestBuilder, _endpoint: &EndpointAuthMetadata, ) -> Result { - let u = self.username.resolve(); - let p = self.password.resolve(); + // See the note on the bearer path: an unreadable credential is an + // auth failure, not an absent one. + let u = self.username.try_resolve()?; + let p = self.password.try_resolve()?; // In Full mode both must be present; in partial modes the // omitted half is sent as the empty string. @@ -262,7 +267,10 @@ impl AuthProvider for HeaderAuthProvider { request: reqwest::RequestBuilder, _endpoint: &EndpointAuthMetadata, ) -> Result { - let Some(token) = self.token.resolve() else { + // `try_resolve`, not `resolve`: a credential the user stored but the + // CLI could not read (a denied keychain prompt) must fail here rather + // than sending the request unauthenticated. + let Some(token) = self.token.try_resolve()? else { return Ok(request); }; let value = if self.bearer_prefix { diff --git a/generators/cli/sdk/src/cli_args.rs b/generators/cli/sdk/src/cli_args.rs index ee58e70ace87..e492c1a45d2f 100644 --- a/generators/cli/sdk/src/cli_args.rs +++ b/generators/cli/sdk/src/cli_args.rs @@ -55,6 +55,18 @@ pub fn wants_schema(args: &[String]) -> bool { args.iter().any(|a| a == "--schema") } +/// True when raw args contain `--help` / `-h`. +/// +/// Paired with an explicit machine `--format`, this is treated as a request for +/// the same catalog `--schema` emits: the generated README documents +/// ` --help --format json | jq 'length'` as the machine-readable list of +/// every operation, and until now that printed prose and made `jq` fail. +/// Deliberately *not* satisfied by the piped default — ` --help | less` +/// must keep showing help. +pub fn wants_help(args: &[String]) -> bool { + args.iter().any(|a| a == "--help" || a == "-h") +} + /// True when raw args contain the `--spec` flag. /// /// `--spec` emits the effective OpenAPI spec (source + overlays + overrides diff --git a/generators/cli/sdk/src/error.rs b/generators/cli/sdk/src/error.rs index 2bd6e66c3f2a..ea0906d41e17 100644 --- a/generators/cli/sdk/src/error.rs +++ b/generators/cli/sdk/src/error.rs @@ -12,6 +12,22 @@ pub enum CliError { code: u16, message: String, reason: String, + /// The server's response body, parsed, minus whatever `message` + /// already reports. + /// + /// Carried structurally rather than folded into `message`: a body we + /// don't recognise still belongs in the JSON envelope, and serializing + /// it into the message string would emit an escaped JSON document + /// inside a JSON field, forcing consumers to parse twice. `None` when + /// the body held nothing `message` does not already say. + details: Option, + /// Actionable advice that is not part of the failure itself, e.g. which + /// credential source supplied the rejected token. + /// + /// Kept out of `message` so that field is one sentence for every error + /// class — a consumer keying off `error.message` gets the same shape + /// whether or not the CLI had advice to offer. + help: Option, }, #[error("{0}")] @@ -23,6 +39,14 @@ pub enum CliError { #[error("{0}")] Discovery(String), + /// A request that never reached the server: DNS, TLS, connection refused, + /// timeout. Distinct from [`Other`](Self::Other) because there is no HTTP + /// status to report — folding it into `Other` produced `code: 500`, which + /// tells an agent the *server* failed and invites a retry against an API + /// that was never contacted. + #[error("{0}")] + Network(String), + #[error(transparent)] Other(#[from] anyhow::Error), @@ -39,20 +63,40 @@ impl CliError { pub const EXIT_CODE_DISCOVERY: i32 = 4; pub const EXIT_CODE_OTHER: i32 = 5; + /// Construct an [`CliError::Api`] with no structured server details. + pub fn api(code: u16, message: impl Into, reason: impl Into) -> Self { + Self::Api { + code, + message: message.into(), + reason: reason.into(), + details: None, + help: None, + } + } + /// Create a duplicate of this error for passing to hook callbacks /// while retaining the original. `Other(anyhow::Error)` is /// converted to its display string since `anyhow::Error` is not /// `Clone`. pub fn duplicate(&self) -> Self { match self { - Self::Api { code, message, reason } => Self::Api { + Self::Api { + code, + message, + reason, + details, + help, + } => Self::Api { code: *code, message: message.clone(), reason: reason.clone(), + details: details.clone(), + help: help.clone(), }, Self::Validation(msg) => Self::Validation(msg.clone()), Self::Auth(msg) => Self::Auth(msg.clone()), Self::Discovery(msg) => Self::Discovery(msg.clone()), + Self::Network(msg) => Self::Network(msg.clone()), Self::Other(e) => Self::Other(anyhow::anyhow!("{e:#}")), Self::RawSentinel { code } => Self::RawSentinel { code: *code }, } @@ -69,6 +113,9 @@ impl CliError { CliError::Auth(_) => Self::EXIT_CODE_AUTH, CliError::Validation(_) => Self::EXIT_CODE_VALIDATION, CliError::Discovery(_) => Self::EXIT_CODE_DISCOVERY, + // Shares `other`'s exit code: adding a sixth would change the + // documented table every consumer already branches on. + CliError::Network(_) => Self::EXIT_CODE_OTHER, CliError::Other(_) => Self::EXIT_CODE_OTHER, CliError::RawSentinel { .. } => Self::EXIT_CODE_API, } @@ -80,20 +127,41 @@ impl CliError { code, message, reason, - } => json!({ - "error": { + details, + help, + } => { + let mut error = json!({ "code": code, "message": message, "reason": reason, + }); + if let Some(details) = details { + error["details"] = details.clone(); } - }), - CliError::Validation(msg) => json!({ - "error": { + if let Some(help) = help { + error["help"] = json!(help); + } + json!({ "error": error }) + } + CliError::Validation(msg) => { + // A usage error arrives as clap's rendered block — a sentence, + // then a tip, a usage line and boilerplate. The human path wants + // all of it; `message` is supposed to be one sentence, so the + // rest becomes fields of its own rather than embedded newlines. + let usage = UsageText::parse(msg); + let mut error = json!({ "code": 400, - "message": msg, + "message": usage.message, "reason": "validationError", + }); + if let Some(help) = usage.help { + error["help"] = json!(help); } - }), + if let Some(usage) = usage.usage { + error["usage"] = json!(usage); + } + json!({ "error": error }) + } CliError::Auth(msg) => json!({ "error": { "code": 401, @@ -108,6 +176,15 @@ impl CliError { "reason": "discoveryError", } }), + // No `code`: the field is documented as the HTTP status, and this + // request never got one. A consumer testing `.error.code` sees it + // absent rather than a status the server never sent. + CliError::Network(msg) => json!({ + "error": { + "message": msg, + "reason": "networkError", + } + }), CliError::Other(e) => json!({ "error": { "code": 500, @@ -128,6 +205,498 @@ impl CliError { use crate::output::{colorize, sanitize_for_terminal}; +/// Render an error together with its `source()` chain, `": "`-joined. +/// +/// Transport errors bury the useful part: `reqwest`'s Display is "error sending +/// request for url (…)", and "Connection refused" — the only line that tells +/// the user what to change — is two levels down. +pub fn error_chain(err: &dyn std::error::Error) -> String { + let mut parts = vec![err.to_string()]; + let mut cursor = err.source(); + while let Some(source) = cursor { + let text = source.to_string(); + // Skip a link that merely restates its parent. + if !parts.last().is_some_and(|prev| prev.contains(&text)) { + parts.push(text); + } + cursor = source.source(); + } + parts.join(": ") +} + +/// Map an HTTP status code to a short reason string for [`CliError::Api`]. +pub fn http_status_reason(status: u16) -> &'static str { + match status { + 400 => "badRequest", + 401 => "unauthorized", + 403 => "forbidden", + 404 => "notFound", + 408 => "requestTimeout", + 409 => "conflict", + 422 => "unprocessableEntity", + 429 => "rateLimited", + 500 => "internalServerError", + 502 => "badGateway", + 503 => "serviceUnavailable", + 504 => "gatewayTimeout", + _ => "httpError", + } +} + +/// Build a [`CliError::Api`] from an HTTP error response. +/// +/// Recognises the shapes services actually return — the Google-style +/// `{"error": {...}}` envelope, `{"error": ""}`, FastAPI/RFC 7807 +/// `{"detail": {...} | [...] | ""}`, `{"title"}`, OAuth 2.0 +/// `{"error_description"}`, and bare `{"message"}` — and lifts one sentence +/// into `message`. A JSON body it can't interpret keeps its structure in +/// `details` instead of being stringified into `message`, which would escape a +/// whole JSON document inside a JSON field. +pub fn api_error_from_body(status: u16, body: &str) -> CliError { + let Ok(parsed) = serde_json::from_str::(body) else { + return non_json_api_error(status, body); + }; + + // A body that is a bare JSON string *is* the sentence. Falling through + // would summarise it as `HTTP ` and exile the only text + // the server sent to `details`. + if let serde_json::Value::String(text) = &parsed { + let text = text.trim(); + if !text.is_empty() { + return CliError::api(status, text, http_status_reason(status)); + } + } + + // Services wrap the interesting fields one level down under `error` + // (Google, Stripe) or `detail` (FastAPI, which is what a Fern-generated + // Python backend emits), so look inside before falling back to the root. + // A body that is a bare list of problems is the same shape one level out. + let scope_prefix: Vec = ["error", "detail"] + .into_iter() + .find(|k| parsed.get(k).is_some_and(|v| v.is_object())) + .map(|k| vec![Seg::Key(k.to_string())]) + .or_else(|| parsed.as_array()?.first().map(|_| vec![Seg::Index(0)])) + .unwrap_or_default(); + let scope = resolve_path(&parsed, &scope_prefix).unwrap_or(&parsed); + + // A string `code` is a symbolic reason, not an HTTP status — as is Stripe's + // and ElevenLabs' `type`. + let reason = first_error(scope) + .and_then(|(_, e)| str_field(e, "reason")) + .or_else(|| str_field(scope, "reason")) + .or_else(|| str_field(scope, "code")) + .or_else(|| str_field(scope, "type")) + .unwrap_or_else(|| http_status_reason(status).to_string()); + let located = message_from(scope); + let message = located + .as_ref() + .map(|m| m.text.clone()) + .unwrap_or_else(|| format!("HTTP {status} {reason}")); + + // `details` exists to preserve what `message` could not carry — a + // `request_id`, the `loc` of the offending field. Drop the one field the + // sentence came from (repeating it would only say the same thing twice), + // then drop `details` altogether if nothing unique is left. Removal is by + // *path*, not by value: a body whose second entry happens to carry the same + // sentence as the first must keep it, or a consumer cannot tell which + // problem belongs to which `loc`. + let details = match &located { + Some(m) => { + let path: Vec = scope_prefix + .iter() + .chain(m.path.iter()) + .cloned() + .collect(); + prune(remove_at_path(parsed, &path)) + } + None => prune(parsed), + }; + + CliError::Api { + // Always the HTTP status. A body's own `code` is application-defined — + // it may be an internal numbering (`{"code": 100234}`) or even a + // success value on a failed request — so letting it win produced + // envelopes that contradicted the exit code and docs links pointing at + // the wrong page. The body's `code` survives in `details`. + code: status, + message, + help: None, + reason, + details, + } +} + +/// Follow `path` into `value`, or `None` if it does not resolve. +fn resolve_path<'v>(value: &'v serde_json::Value, path: &[Seg]) -> Option<&'v serde_json::Value> { + path.iter().try_fold(value, |cursor, seg| match seg { + Seg::Key(k) => cursor.get(k), + Seg::Index(i) => cursor.get(i), + }) +} + +/// Build a [`CliError::Api`] from a body that is not JSON at all. +/// +/// Three shapes turn up here: an empty body, a one-line message from a proxy, +/// and a full HTML (or XML) error page from a CDN or load balancer. Only the +/// middle one belongs in `message` — a page is neither a sentence nor a single +/// line, and reproducing it there breaks the guarantee every other error class +/// keeps. The bytes are not discarded: they move to `details.body`, clipped. +fn non_json_api_error(status: u16, body: &str) -> CliError { + let reason = http_status_reason(status); + let collapsed = collapse_whitespace(body); + if collapsed.is_empty() { + // An error with no body at all still needs a sentence; an empty + // `message` would read as "the CLI lost the error". + return CliError::api(status, format!("HTTP {status} {reason}"), reason); + } + let is_markup = collapsed.starts_with('<'); + if is_markup || collapsed.len() > MAX_MESSAGE_BODY_BYTES { + return CliError::Api { + code: status, + message: format!( + "HTTP {status} {reason} (non-JSON response, {} bytes)", + body.len() + ), + reason: reason.to_string(), + details: Some(json!({ "body": truncate_body(&collapsed, MAX_MESSAGE_BODY_BYTES) })), + help: None, + }; + } + CliError::api(status, collapsed, reason) +} + +/// Fold every run of whitespace into a single space and trim the ends, so a +/// body that arrived wrapped across lines still reads as one. +fn collapse_whitespace(text: &str) -> String { + text.split_whitespace().collect::>().join(" ") +} + +/// Upper bound on `details` lines shown in the terminal. The whole structure is +/// always in the JSON envelope; this is only about not flooding a screen when a +/// service answers with a hundred per-field problems. +const MAX_DETAIL_LINES: usize = 10; + +/// Flatten `details` into `path: value` lines for the human rendering. +/// +/// A list of scalars stays on one line (`loc: body, email`) — splitting FastAPI's +/// two-element `loc` across two lines buries the field name it exists to report. +/// Returns at most `max` lines, with a final line naming what was elided. +/// `already_shown` are strings the reader has seen on the headline — the +/// message and the reason. A leaf repeating one of them is noise: services +/// routinely echo the same token as `code`, `status` and `type`, which turned a +/// one-fact 404 into five lines that said the same thing four times. +fn detail_lines( + details: &serde_json::Value, + max: usize, + already_shown: &[&str], +) -> Vec { + let mut lines = Vec::new(); + let mut seen: std::collections::HashSet = + already_shown.iter().map(|s| s.to_string()).collect(); + walk_details(unwrap_sole_object(details), &mut String::new(), &mut lines, &mut seen); + if lines.len() > max { + let hidden = lines.len() - max; + lines.truncate(max); + lines.push(format!("… {hidden} more (use --format json to see all)")); + } + lines +} + +/// Descend through wrappers that add a path segment but no information. +/// +/// `{"detail": {...}}` would otherwise prefix every line with `detail.`, which +/// is the same word on every row and never the part the reader is looking for. +/// Only single-key *object* wrappers are unwrapped: for a list, `detail[0].loc` +/// is more legible than `[0].loc`. +fn unwrap_sole_object(mut value: &serde_json::Value) -> &serde_json::Value { + while let serde_json::Value::Object(map) = value { + match map.iter().next() { + Some((_, inner)) if map.len() == 1 && inner.is_object() => value = inner, + _ => break, + } + } + value +} + +fn walk_details( + value: &serde_json::Value, + path: &mut String, + out: &mut Vec, + seen: &mut std::collections::HashSet, +) { + match value { + serde_json::Value::Object(map) => { + for (k, v) in map { + let mark = path.len(); + if !path.is_empty() { + path.push('.'); + } + path.push_str(k); + walk_details(v, path, out, seen); + path.truncate(mark); + } + } + serde_json::Value::Array(items) => { + // All-scalar lists read better joined than exploded one per line. + if let Some(joined) = scalar_list(items) { + if seen.insert(joined.clone()) { + out.push(format!("{path}: {joined}")); + } + return; + } + for (i, v) in items.iter().enumerate() { + let mark = path.len(); + path.push_str(&format!("[{i}]")); + walk_details(v, path, out, seen); + path.truncate(mark); + } + } + other => { + let text = scalar_text(other); + // `seen` grows as we go, so a value repeated across sibling keys is + // printed once — under the first key that carried it. + if seen.insert(text.clone()) { + out.push(format!("{path}: {text}")); + } + } + } +} + +/// `Some(joined)` when every item is a scalar, else `None`. +fn scalar_list(items: &[serde_json::Value]) -> Option { + if items.is_empty() || items.iter().any(|v| v.is_object() || v.is_array()) { + return None; + } + Some( + items + .iter() + .map(scalar_text) + .collect::>() + .join(", "), + ) +} + +/// Render a scalar without JSON's quoting, which adds nothing in a terminal. +fn scalar_text(value: &serde_json::Value) -> String { + match value { + serde_json::Value::String(s) => s.clone(), + serde_json::Value::Null => "null".to_string(), + other => other.to_string(), + } +} + +/// One step along a path into a JSON document. +#[derive(Clone, Debug, PartialEq)] +pub(crate) enum Seg { + Key(String), + Index(usize), +} + +/// A message plus the path it was read from, so `details` can drop exactly that +/// occurrence rather than every string that happens to equal it. +struct LocatedMessage { + text: String, + path: Vec, +} + +/// Remove the value at `path`, leaving everything else untouched. A path that +/// does not resolve leaves `value` unchanged. +pub(crate) fn remove_at_path(mut value: serde_json::Value, path: &[Seg]) -> serde_json::Value { + let Some((last, parents)) = path.split_last() else { + return value; + }; + let mut cursor = &mut value; + for seg in parents { + cursor = match (cursor, seg) { + (serde_json::Value::Object(map), Seg::Key(k)) => match map.get_mut(k) { + Some(next) => next, + None => return value, + }, + (serde_json::Value::Array(items), Seg::Index(i)) => match items.get_mut(*i) { + Some(next) => next, + None => return value, + }, + _ => return value, + }; + } + match (cursor, last) { + (serde_json::Value::Object(map), Seg::Key(k)) => { + map.remove(k); + } + (serde_json::Value::Array(items), Seg::Index(i)) if *i < items.len() => { + items.remove(*i); + } + _ => {} + } + value +} + +/// Drop containers that hold nothing, recursively. `None` means the document +/// carried no information once the message was removed. +pub(crate) fn prune(value: serde_json::Value) -> Option { + match value { + serde_json::Value::Object(map) => { + let kept: serde_json::Map = map + .into_iter() + .filter_map(|(k, v)| prune(v).map(|v| (k, v))) + .collect(); + (!kept.is_empty()).then_some(serde_json::Value::Object(kept)) + } + serde_json::Value::Array(items) => { + let kept: Vec = items.into_iter().filter_map(prune).collect(); + (!kept.is_empty()).then_some(serde_json::Value::Array(kept)) + } + other => Some(other), + } +} + +/// A validation message split into the parts an envelope keeps apart. +/// +/// Single-line messages — the overwhelming majority, raised by our own +/// validators — pass through as `message` with nothing else set. +struct UsageText { + message: String, + help: Option, + usage: Option, +} + +impl UsageText { + /// Boilerplate that only makes sense as terminal output: an agent reading + /// the envelope cannot "try `--help`". + const TRY_HELP: &'static str = "For more information, try"; + + /// Whether `text` is clap's rendered error block rather than one of our + /// own validators' messages. + /// + /// The split below is tuned to clap's layout — first line is the failure, + /// everything after it is a tip, a usage line, or boilerplate. Applied to a + /// message that merely happens to span lines (a schema validation listing + /// one bullet per violation) it would demote the violations to `help` and + /// leave only the header in `message`, so anything not clap-shaped passes + /// through whole. clap is built without the `color` feature here, so the + /// prefix is plain text. + fn is_clap_block(text: &str) -> bool { + let trimmed = text.trim_start(); + trimmed.starts_with("error:") || trimmed.starts_with("Usage:") || text.contains("\nUsage:") + } + + fn parse(text: &str) -> Self { + if !Self::is_clap_block(text) { + return Self { + message: text.trim().to_string(), + help: None, + usage: None, + }; + } + let mut message = Vec::new(); + let mut help = Vec::new(); + let mut usage = Vec::new(); + // Everything from `Usage:` up to the next blank line: clap wraps long + // usage strings onto continuation lines. + let mut in_usage = false; + + for line in text.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() { + in_usage = false; + continue; + } + if let Some(rest) = trimmed.strip_prefix("Usage:") { + in_usage = true; + usage.push(rest.trim().to_string()); + } else if in_usage { + usage.push(trimmed.to_string()); + } else if let Some(rest) = trimmed.strip_prefix("tip:") { + help.push(rest.trim().to_string()); + } else if trimmed.starts_with(Self::TRY_HELP) { + continue; + } else if message.is_empty() { + message.push(trimmed.strip_prefix("error:").unwrap_or(trimmed).trim().to_string()); + } else { + // A wrapped sentence or a `[possible values: ...]` list — advice + // about the failure rather than the failure itself. + help.push(trimmed.to_string()); + } + } + + Self { + message: if message.is_empty() { + text.trim().to_string() + } else { + message.remove(0) + }, + help: (!help.is_empty()).then(|| help.join(" ")), + usage: (!usage.is_empty()).then(|| usage.join(" ")), + } + } +} + +/// Upper bound on a non-JSON body reproduced verbatim in `message`. +const MAX_MESSAGE_BODY_BYTES: usize = 500; + +/// Clip `body` to `max` bytes on a char boundary, marking the elision. +fn truncate_body(body: &str, max: usize) -> String { + if body.len() <= max { + return body.to_string(); + } + let mut end = max; + while end > 0 && !body.is_char_boundary(end) { + end -= 1; + } + format!("{}… ({} bytes total)", &body[..end], body.len()) +} + +/// Read a non-empty string field, if present. +fn str_field(scope: &serde_json::Value, key: &str) -> Option { + scope + .get(key) + .and_then(|v| v.as_str()) + .filter(|s| !s.is_empty()) + .map(str::to_string) +} + +/// The first entry of an `errors`/`detail` list of per-problem objects, which is +/// where Google-style and FastAPI-style bodies put the actual message. Returns +/// the key it was found under so callers can rebuild the path to it. +fn first_error(scope: &serde_json::Value) -> Option<(&'static str, &serde_json::Value)> { + ["errors", "detail"] + .into_iter() + .filter_map(|k| Some((k, scope.get(k)?.as_array()?))) + .find_map(|(k, arr)| arr.first().map(|first| (k, first))) +} + +/// Pull a human-readable sentence out of an error object, accepting the field +/// names services actually use, and report where it came from. +fn message_from(scope: &serde_json::Value) -> Option { + for key in [ + "message", + "msg", + "detail", + "title", + "error_description", + "error", + ] { + if let Some(text) = str_field(scope, key) { + return Some(LocatedMessage { + text, + path: vec![Seg::Key(key.to_string())], + }); + } + } + let (key, first) = first_error(scope)?; + let prefix = [Seg::Key(key.to_string()), Seg::Index(0)]; + match first.as_str() { + Some(text) => Some(LocatedMessage { + text: text.to_string(), + path: prefix.to_vec(), + }), + None => message_from(first).map(|inner| LocatedMessage { + text: inner.text, + path: prefix.into_iter().chain(inner.path).collect(), + }), + } +} + /// All documented exit codes with their human-readable descriptions. pub const EXIT_CODE_TABLE: &[(i32, &str, &str)] = &[ (CliError::EXIT_CODE_API, "api", "API returned a non-success HTTP status"), @@ -239,13 +808,14 @@ fn error_label(err: &CliError) -> String { CliError::Auth(_) => colorize("error[auth]:", "31"), CliError::Validation(_) => colorize("error[validation]:", "33"), CliError::Discovery(_) => colorize("error[discovery]:", "31"), + CliError::Network(_) => colorize("error[network]:", "31"), CliError::Other(_) => colorize("error:", "31"), CliError::RawSentinel { .. } => colorize("error[api]:", "31"), } } -/// Optional context that enriches the stderr error display with a docs link -/// and a `--help` suggestion. Does not affect the JSON envelope on stdout. +/// Context for rendering an error: which representation to emit, plus the +/// docs link and `--help` suggestion that enrich the human one. pub struct ErrorDisplayContext { /// Base URL for per-code documentation links (e.g. `https://docs.example.com/errors/`). /// Appended with the HTTP status code for `CliError::Api` errors. @@ -253,36 +823,120 @@ pub struct ErrorDisplayContext { /// Full help invocation, e.g. `box users list --help`. /// Printed as `Try \`...\`` after the error message. pub help_hint: Option, + /// The resolved output format. + /// + /// Selects *which* representation of the error is emitted, never both: a + /// machine format puts the JSON envelope on stdout, a human one puts a + /// single line on stderr and leaves stdout empty. It also decides the + /// envelope's shape — `jsonl` is line-delimited by definition, so a + /// pretty-printed value there is unreadable to the line-at-a-time consumer + /// the format exists for. + pub format: crate::formatter::OutputFormat, } -pub fn print_error_json(err: &CliError) { - write_error_json(err, &mut std::io::stdout(), None); +impl Default for ErrorDisplayContext { + fn default() -> Self { + Self { + docs_base_url: None, + help_hint: None, + // Matches the format resolver's non-TTY default: a caller with no + // resolved format is a pipe or a test, both of which want JSON. + format: crate::formatter::OutputFormat::Json, + } + } } -pub fn write_error_json(err: &CliError, out: &mut dyn std::io::Write, ctx: Option<&ErrorDisplayContext>) { +/// Render `err` in exactly one representation, chosen by +/// [`ErrorDisplayContext::format`]: the JSON envelope on `out` for a machine +/// format, or a human line (plus details, help hint and docs link) on stderr. +pub fn write_error_json( + err: &CliError, + out: &mut dyn std::io::Write, + ctx: Option<&ErrorDisplayContext>, +) { // Raw-mode sentinel: bytes already on stdout, skip structured JSON. if let CliError::RawSentinel { code } = err { eprintln!("Error: HTTP {code}"); return; } - let json = err.to_json(); - let _ = writeln!( - out, - "{}", - serde_json::to_string_pretty(&json).unwrap_or_default() - ); - eprintln!( - "{} {}", - error_label(err), - sanitize_for_terminal(&err.to_string()) - ); - if let Some(ctx) = ctx { - if let Some(base) = &ctx.docs_base_url { - if let CliError::Api { code, .. } = err { - let url = format!("{}/{}", base.trim_end_matches('/'), code); - eprintln!(" → {}", sanitize_for_terminal(&url)); - } + // One representation per invocation. Emitting both makes a consumer that + // merges stdout and stderr see the same error twice with no way to tell + // which is authoritative. + let format = ctx.map(|c| c.format).unwrap_or(crate::formatter::OutputFormat::Json); + let machine_readable = format.is_machine_readable(); + let docs_url = ctx + .and_then(|ctx| ctx.docs_base_url.as_deref()) + .zip(match err { + CliError::Api { code, .. } => Some(code), + _ => None, + }) + .map(|(base, code)| format!("{}/{}", base.trim_end_matches('/'), code)); + if machine_readable { + let mut json = err.to_json(); + // The human rendering has always printed this link; carrying it in the + // envelope too keeps both representations equally informative. + if let Some(url) = &docs_url { + json["error"]["docs_url"] = json!(url); + } + // `jsonl` means one JSON value per line; pretty-printing puts a bare + // `{` on the first line and breaks every line-at-a-time reader. + let rendered = if matches!(format, crate::formatter::OutputFormat::Jsonl) { + serde_json::to_string(&json).unwrap_or_default() + } else { + serde_json::to_string_pretty(&json).unwrap_or_default() + }; + let _ = writeln!(out, "{rendered}"); + return; + } + // A service-specific reason (`workspace_not_found`) is the second most + // useful thing after the sentence, and the human rendering used to be the + // one place it never appeared. A reason derived from the status carries + // nothing the label and message do not already say, so it stays hidden. + let specific_reason = match err { + CliError::Api { code, reason, .. } if reason != http_status_reason(*code) => { + Some(reason.as_str()) } + _ => None, + }; + match specific_reason { + Some(reason) => eprintln!( + "{} {} ({})", + error_label(err), + sanitize_for_terminal(&err.to_string()), + sanitize_for_terminal(reason) + ), + None => eprintln!( + "{} {}", + error_label(err), + sanitize_for_terminal(&err.to_string()) + ), + } + // What the server said beyond the sentence, before the advice about it. + // Without this a field-level failure reads as `error[api]: field required` + // with no way to tell *which* field — the `loc` is in the envelope but the + // person in the terminal is the one who cannot see it. + if let CliError::Api { + details: Some(details), + message, + reason, + .. + } = err + { + let shown = [message.as_str(), reason.as_str()]; + for line in detail_lines(details, MAX_DETAIL_LINES, &shown) { + eprintln!(" {}", sanitize_for_terminal(&line)); + } + } + if let CliError::Api { + help: Some(help), .. + } = err + { + eprintln!("{}", sanitize_for_terminal(help)); + } + if let Some(url) = &docs_url { + eprintln!(" → {}", sanitize_for_terminal(url)); + } + if let Some(ctx) = ctx { if matches!(err, CliError::Validation(_)) { if let Some(hint) = &ctx.help_hint { // `--help` is the right next step for a malformed flag, but not @@ -370,11 +1024,7 @@ mod tests { #[test] fn test_error_to_json_api() { - let err = CliError::Api { - code: 404, - message: "Not Found".to_string(), - reason: "notFound".to_string(), - }; + let err = CliError::api(404, "Not Found".to_string(), "notFound".to_string()); let json = err.to_json(); assert_eq!(json["error"]["code"], 404); assert_eq!(json["error"]["message"], "Not Found"); @@ -388,11 +1038,91 @@ mod tests { } #[test] - fn test_exit_codes_all_variants() { + fn single_line_validation_messages_pass_through_untouched() { + let err = CliError::Validation( + "Required parameter 'user_id' is missing. Provide it via --user-id or --params" + .to_string(), + ); + let json = err.to_json(); + assert_eq!( + json["error"]["message"], + "Required parameter 'user_id' is missing. Provide it via --user-id or --params" + ); + assert!(json["error"].get("usage").is_none()); + assert!(json["error"].get("help").is_none()); + } + + #[test] + fn clap_usage_block_is_split_out_of_the_message() { + // clap's rendering for a mistyped subcommand, verbatim. + let err = CliError::Validation( + "error: unrecognized subcommand 'lst'\n\n \ + tip: a similar subcommand exists: 'list'\n\n\ + Usage: openapi-fixture users [OPTIONS] \n\n\ + For more information, try '--help'.\n" + .to_string(), + ); + let json = err.to_json(); + assert_eq!(json["error"]["message"], "unrecognized subcommand 'lst'"); + assert_eq!(json["error"]["help"], "a similar subcommand exists: 'list'"); + assert_eq!( + json["error"]["usage"], + "openapi-fixture users [OPTIONS] " + ); + // The one-sentence promise: no embedded newlines anywhere. + for field in ["message", "help", "usage"] { + let value = json["error"][field].as_str().expect("string field"); + assert!(!value.contains('\n'), "{field} should be a single line: {value}"); + } + } + + #[test] + fn flag_conflicts_keep_their_sentence_and_usage_apart() { + let err = CliError::Validation( + "error: the argument '--human' cannot be used with '--format '\n\n\ + Usage: openapi-fixture --human \n\n\ + For more information, try '--help'.\n" + .to_string(), + ); + let json = err.to_json(); assert_eq!( - CliError::Api { code: 404, message: String::new(), reason: String::new() }.exit_code(), - CliError::EXIT_CODE_API + json["error"]["message"], + "the argument '--human' cannot be used with '--format '" ); + assert_eq!(json["error"]["usage"], "openapi-fixture --human "); + assert!(json["error"].get("help").is_none()); + } + + #[test] + fn possible_values_lists_land_in_help() { + let err = CliError::Validation( + "error: invalid value 'xml' for '--format '\n \ + [possible values: json, table, yaml]\n\n\ + For more information, try '--help'.\n" + .to_string(), + ); + let json = err.to_json(); + assert_eq!( + json["error"]["message"], + "invalid value 'xml' for '--format '" + ); + assert_eq!(json["error"]["help"], "[possible values: json, table, yaml]"); + } + + #[test] + fn human_rendering_of_a_usage_error_keeps_the_whole_block() { + // The split is a JSON-envelope concern: `Display` — what the human path + // prints on stderr — must still carry clap's tip and usage lines. + let rendered = "error: unrecognized subcommand 'lst'\n\n \ + tip: a similar subcommand exists: 'list'\n\n\ + Usage: openapi-fixture users [OPTIONS] \n"; + let err = CliError::Validation(rendered.to_string()); + assert_eq!(err.to_string(), rendered); + } + + #[test] + fn test_exit_codes_all_variants() { + assert_eq!(CliError::api(404, "", "").exit_code(), CliError::EXIT_CODE_API); assert_eq!(CliError::Auth(String::new()).exit_code(), CliError::EXIT_CODE_AUTH); assert_eq!(CliError::Validation(String::new()).exit_code(), CliError::EXIT_CODE_VALIDATION); assert_eq!(CliError::Discovery(String::new()).exit_code(), CliError::EXIT_CODE_DISCOVERY); @@ -428,28 +1158,30 @@ mod tests { } #[test] - fn test_print_error_json_all_variants_no_panic() { - print_error_json(&CliError::Api { - code: 500, - message: "oops".to_string(), - reason: "err".to_string(), - }); - print_error_json(&CliError::Validation("bad input".to_string())); - print_error_json(&CliError::Auth("no auth".to_string())); - print_error_json(&CliError::Discovery("no spec".to_string())); - print_error_json(&CliError::Other(anyhow::anyhow!("broken"))); + fn every_variant_renders_without_panicking() { + // Captures rather than writing to the real stdout, and covers `Network` + // and `RawSentinel`, which the previous version of this test predated. + for err in [ + CliError::api(500, "oops".to_string(), "err".to_string()), + CliError::Validation("bad input".to_string()), + CliError::Auth("no auth".to_string()), + CliError::Discovery("no spec".to_string()), + CliError::Network("connection refused".to_string()), + CliError::Other(anyhow::anyhow!("broken")), + CliError::RawSentinel { code: 500 }, + ] { + let mut out = Vec::new(); + write_error_json(&err, &mut out, None); + } } #[test] fn write_error_json_stdout_unchanged_with_context() { - let err = CliError::Api { - code: 401, - message: "Unauthorized".to_string(), - reason: "authError".to_string(), - }; + let err = CliError::api(401, "Unauthorized".to_string(), "authError".to_string()); let ctx = ErrorDisplayContext { docs_base_url: Some("https://docs.example.com/errors".to_string()), help_hint: Some("mycli users list --help".to_string()), + format: crate::formatter::OutputFormat::Json, }; let mut out = Vec::new(); write_error_json(&err, &mut out, Some(&ctx)); @@ -457,6 +1189,36 @@ mod tests { let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); assert_eq!(parsed["error"]["code"], 401); assert_eq!(parsed["error"]["message"], "Unauthorized"); + // The docs link is part of both representations, not just the human one. + assert_eq!( + parsed["error"]["docs_url"], + "https://docs.example.com/errors/401" + ); + } + + #[test] + fn help_is_a_separate_field_from_message() { + // `message` must be one sentence for every error class, so advice such + // as the credential-source hint gets its own field rather than being + // appended with a newline. + let err = CliError::Api { + code: 401, + message: "Invalid API key".to_string(), + reason: "unauthorized".to_string(), + details: None, + help: Some("Credentials were supplied via: MY_CLI_API_KEY.".to_string()), + }; + let parsed = err.to_json(); + assert_eq!(parsed["error"]["message"], "Invalid API key"); + assert_eq!( + parsed["error"]["help"], + "Credentials were supplied via: MY_CLI_API_KEY." + ); + // Absent advice means an absent field, not an empty string. + assert!( + CliError::api(500, "Server blew up", "internalServerError").to_json()["error"]["help"] + .is_null() + ); } #[test] @@ -464,6 +1226,7 @@ mod tests { let ctx = ErrorDisplayContext { docs_base_url: Some("https://docs.example.com/errors".to_string()), help_hint: None, + format: crate::formatter::OutputFormat::Json, }; // Validation errors should not get docs URLs (no HTTP status code). let mut out = Vec::new(); @@ -475,6 +1238,422 @@ mod tests { let stdout = String::from_utf8(out).unwrap(); let parsed: serde_json::Value = serde_json::from_str(&stdout).unwrap(); assert_eq!(parsed["error"]["code"], 400); + assert!(parsed["error"]["docs_url"].is_null()); + } + + #[test] + fn human_format_leaves_stdout_empty() { + // The regression: stdout carried the envelope *and* stderr carried the + // same message, so a consumer merging the two saw the error twice. + let ctx = ErrorDisplayContext { + format: crate::formatter::OutputFormat::Table, + ..Default::default() + }; + let mut out = Vec::new(); + write_error_json( + &CliError::api(500, "Server blew up", "internalServerError"), + &mut out, + Some(&ctx), + ); + assert!( + out.is_empty(), + "human format must not write the envelope to stdout, got: {}", + String::from_utf8_lossy(&out) + ); + } + + #[test] + fn top_level_service_error_is_not_double_encoded() { + // A body with no `error` key used to be stringified whole into + // `message`, emitting an escaped JSON document inside the envelope. + let err = api_error_from_body( + 500, + r#"{"status": "internal_server_error", "message": "Internal Server error. All such crashes are reported to us automatically."}"#, + ); + let json = err.to_json(); + let message = json["error"]["message"].as_str().unwrap(); + assert_eq!( + message, + "Internal Server error. All such crashes are reported to us automatically." + ); + assert!( + serde_json::from_str::(message).is_err(), + "message must be a sentence, not a serialized JSON document" + ); + assert_eq!(json["error"]["reason"], "internalServerError"); + assert_eq!(json["error"]["details"]["status"], "internal_server_error"); + } + + #[test] + fn nested_detail_bodies_yield_a_sentence() { + // Bodies captured verbatim from api.elevenlabs.io: FastAPI puts the + // useful fields under `detail`, as an object, a list, or a bare string. + let err = api_error_from_body( + 401, + r#"{"detail":{"type":"authentication_error","code":"unauthorized","message":"Invalid API key","status":"invalid_api_key","request_id":"f883"}}"#, + ); + assert_eq!(err.to_string(), "Invalid API key"); + let json = err.to_json(); + // A string `code` is a symbolic reason, so it must not land in `code`. + assert_eq!(json["error"]["code"], 401); + assert_eq!(json["error"]["reason"], "unauthorized"); + assert_eq!(json["error"]["details"]["detail"]["request_id"], "f883"); + + let err = api_error_from_body( + 422, + r#"{"detail":[{"type":"missing","loc":["body","text"],"msg":"Field required","input":null}]}"#, + ); + assert_eq!(err.to_string(), "Field required"); + assert_eq!(err.to_json()["error"]["reason"], "unprocessableEntity"); + + let err = api_error_from_body(404, r#"{"detail":"Not Found"}"#); + assert_eq!(err.to_string(), "Not Found"); + assert_eq!(err.to_json()["error"]["reason"], "notFound"); + } + + #[test] + fn details_never_repeat_the_message() { + // The sentence lifted into `message` is dropped from `details`, however + // deeply it sat, while everything unique to the body survives. + let err = api_error_from_body( + 401, + r#"{"detail":{"type":"authentication_error","code":"unauthorized","message":"Invalid API key","request_id":"f883"}}"#, + ); + let json = err.to_json(); + assert_eq!(json["error"]["message"], "Invalid API key"); + assert_eq!(json["error"]["details"]["detail"]["request_id"], "f883"); + assert!( + json["error"]["details"]["detail"].get("message").is_none(), + "got: {json:#}" + ); + + let err = api_error_from_body( + 422, + r#"{"detail":[{"type":"missing","loc":["body","text"],"msg":"Field required"}]}"#, + ); + let json = err.to_json(); + assert_eq!(json["error"]["message"], "Field required"); + assert_eq!(json["error"]["details"]["detail"][0]["loc"][1], "text"); + assert!(json["error"]["details"]["detail"][0].get("msg").is_none()); + } + + #[test] + fn details_are_omitted_when_the_body_is_only_the_message() { + // Nothing to preserve once the sentence is lifted, so the field would + // be an empty husk restating `message`. + for body in [ + r#"{"detail":"Not Found"}"#, + r#"{"message":"Not Found"}"#, + r#"{"error":"Not Found"}"#, + r#"{"errors":["Not Found"]}"#, + ] { + let json = api_error_from_body(404, body).to_json(); + assert_eq!(json["error"]["message"], "Not Found", "body: {body}"); + assert!(json["error"].get("details").is_none(), "body: {body}"); + } + } + + #[test] + fn api_error_from_body_recognizes_common_shapes() { + // Google-style envelope: the reason comes from the body, the code does + // not — `error.code` is the status the server actually answered with. + let err = api_error_from_body( + 401, + r#"{"error":{"code":403,"message":"Denied","errors":[{"reason":"authError"}]}}"#, + ); + match &err { + CliError::Api { + code, + message, + reason, + details, + .. + } => { + assert_eq!(*code, 401); + assert_eq!(message, "Denied"); + assert_eq!(reason, "authError"); + // The body's own code is still reachable, just not as `code`. + assert_eq!(details.as_ref().unwrap()["error"]["code"], 403); + } + other => panic!("expected Api, got: {other:?}"), + } + + // `{"error": ""}`, RFC 7807 `detail`, and OAuth 2.0 + // `error_description` all yield a sentence. + for (body, expected) in [ + ( + r#"{"error":"Something went wrong"}"#, + "Something went wrong", + ), + (r#"{"detail":"Rate limit exceeded"}"#, "Rate limit exceeded"), + (r#"{"error_description":"Token expired"}"#, "Token expired"), + ( + r#"{"errors":[{"message":"Field required"}]}"#, + "Field required", + ), + ] { + assert_eq!( + api_error_from_body(400, body).to_string(), + expected, + "body: {body}" + ); + } + + // Unrecognised JSON: a status summary, with the body kept structurally. + let err = api_error_from_body(503, r#"{"upstream":{"queue":"full"}}"#); + assert_eq!(err.to_string(), "HTTP 503 serviceUnavailable"); + assert_eq!( + err.to_json()["error"]["details"]["upstream"]["queue"], + "full" + ); + + // Non-JSON bodies still surface verbatim. + let err = api_error_from_body(502, "upstream connect error"); + assert_eq!(err.to_string(), "upstream connect error"); + assert!(err.to_json()["error"].get("details").is_none()); + } + + #[test] + fn markup_bodies_are_summarised_not_pasted_into_the_message() { + // A CDN answering 502 with an HTML page: `message` is a sentence about + // what happened, and the markup moves to `details.body` clipped. Size + // alone is not the test — even a short page is multi-line markup, which + // `message` promises never to be. + for page in [ + format!("{}", "x".repeat(4000)), + "\n\n 502 Bad Gateway\n".to_string(), + ] { + let err = api_error_from_body(502, &page); + let message = err.to_string(); + assert_eq!( + message, + format!("HTTP 502 badGateway (non-JSON response, {} bytes)", page.len()) + ); + assert!(!message.contains('\n'), "message must be one line: {message}"); + let json = err.to_json(); + let body = json["error"]["details"]["body"].as_str().unwrap(); + assert!(body.starts_with("<"), "the bytes are kept: {body}"); + assert!(!body.contains('\n'), "details.body is collapsed: {body}"); + assert!(body.len() < 600, "details.body is clipped: {} bytes", body.len()); + } + } + + #[test] + fn non_json_bodies_that_are_a_sentence_stay_in_the_message() { + // The common proxy case: a short plain-text reason. It is the most + // informative thing we have, so it stays where a reader looks first. + let err = api_error_from_body(502, "upstream connect error"); + assert_eq!(err.to_string(), "upstream connect error"); + assert!(err.to_json()["error"].get("details").is_none()); + + // Wrapped across lines by the server, but still one sentence. + let err = api_error_from_body(503, "upstream connect error\n or disconnect"); + assert_eq!(err.to_string(), "upstream connect error or disconnect"); + } + + #[test] + fn bodyless_and_bare_string_responses_still_yield_a_sentence() { + // An empty body used to produce `message: ""`, which reads as the CLI + // having lost the error rather than the server having sent nothing. + for body in ["", " \n "] { + let err = api_error_from_body(504, body); + assert_eq!(err.to_string(), "HTTP 504 gatewayTimeout", "body: {body:?}"); + assert!(err.to_json()["error"].get("details").is_none()); + } + // A body that is a bare JSON string is the sentence itself, not an + // unrecognised document to file under `details`. + let err = api_error_from_body(500, r#""Service temporarily unavailable""#); + assert_eq!(err.to_string(), "Service temporarily unavailable"); + assert!(err.to_json()["error"].get("details").is_none()); + + // A body that is a bare list of problems reads like `errors`/`detail` + // one level out. + let err = api_error_from_body( + 422, + r#"[{"loc":["body","email"],"msg":"value is not a valid email"}]"#, + ); + assert_eq!(err.to_string(), "value is not a valid email"); + assert_eq!(err.to_json()["error"]["details"][0]["loc"][1], "email"); + } + + #[test] + fn human_rendering_reports_the_details_a_reader_cannot_otherwise_see() { + // The envelope carries `loc`; the person in the terminal cannot read the + // envelope. Without these lines a field-level failure is unactionable. + let err = api_error_from_body( + 422, + r#"{"detail":[{"loc":["body","email"],"msg":"value is not a valid email","type":"value_error.email"}]}"#, + ); + let CliError::Api { details, .. } = &err else { + panic!("expected Api"); + }; + let lines = detail_lines(details.as_ref().unwrap(), MAX_DETAIL_LINES, &[]); + // A scalar list stays on one line: the field path is the point. + assert!( + lines.contains(&"detail[0].loc: body, email".to_string()), + "got: {lines:?}" + ); + assert!( + lines.contains(&"detail[0].type: value_error.email".to_string()), + "got: {lines:?}" + ); + assert!(lines.iter().all(|l| !l.contains('"')), "got: {lines:?}"); + } + + #[test] + fn human_details_drop_what_the_headline_already_said() { + // Verbatim from api.elevenlabs.io. `code`, `status` and `type` all + // restate the reason; only `request_id` is new. Rendering all four + // turned a one-fact 404 into five lines saying the same thing. + let err = api_error_from_body( + 404, + r#"{"detail":{"status":"workspace_not_found","message":"Workspace 1anonymous1 not found.","code":"workspace_not_found","type":"not_found","request_id":"6c312f"}}"#, + ); + let CliError::Api { + details, + message, + reason, + .. + } = &err + else { + panic!("expected Api"); + }; + assert_eq!(reason, "workspace_not_found"); + let lines = detail_lines( + details.as_ref().unwrap(), + MAX_DETAIL_LINES, + &[message.as_str(), reason.as_str()], + ); + // The sole `detail.` wrapper is stripped — it prefixed every line and + // named nothing the reader was looking for. + assert_eq!( + lines, + vec![ + "request_id: 6c312f".to_string(), + "type: not_found".to_string() + ], + "got: {lines:?}" + ); + // Nothing is lost: the envelope still carries every field. + let json = err.to_json(); + assert_eq!(json["error"]["details"]["detail"]["status"], "workspace_not_found"); + assert_eq!(json["error"]["details"]["detail"]["code"], "workspace_not_found"); + } + + #[test] + fn a_status_derived_reason_stays_off_the_headline() { + // `notFound` on a 404 says nothing `error[api]` and the message do not. + let err = api_error_from_body(404, r#"{"detail":"Not Found"}"#); + let CliError::Api { code, reason, .. } = &err else { + panic!("expected Api"); + }; + assert_eq!(reason, http_status_reason(*code)); + } + + #[test] + fn jsonl_errors_are_one_line() { + // NDJSON is parsed a line at a time; a pretty-printed envelope puts a + // bare `{` on line one and breaks every such reader. + let err = api_error_from_body(404, r#"{"detail":"Not Found"}"#); + let mut out = Vec::new(); + write_error_json( + &err, + &mut out, + Some(&ErrorDisplayContext { + docs_base_url: None, + help_hint: None, + format: crate::formatter::OutputFormat::Jsonl, + }), + ); + let text = String::from_utf8(out).unwrap(); + assert_eq!(text.lines().count(), 1, "got: {text}"); + serde_json::from_str::(text.trim()).expect("each line must parse"); + + // `json` keeps the readable multi-line rendering. + let mut out = Vec::new(); + write_error_json( + &err, + &mut out, + Some(&ErrorDisplayContext { + docs_base_url: None, + help_hint: None, + format: crate::formatter::OutputFormat::Json, + }), + ); + assert!(String::from_utf8(out).unwrap().lines().count() > 1); + } + + #[test] + fn human_detail_rendering_is_capped() { + // A service answering with a hundred per-field problems must not scroll + // the failure sentence off the screen. + let problems: Vec = (0..40) + .map(|i| json!({ "field": format!("f{i}") })) + .collect(); + let lines = detail_lines(&json!({ "detail": problems }), MAX_DETAIL_LINES, &[]); + assert_eq!(lines.len(), MAX_DETAIL_LINES + 1); + assert!(lines.last().unwrap().contains("30 more"), "got: {lines:?}"); + assert!(lines.last().unwrap().contains("--format json")); + } + + #[test] + fn api_error_from_body_code_is_always_the_http_status() { + // A body's `code` is application-defined and cannot be told apart from + // an HTTP status by inspection, so it never wins: an internal numbering + // would truncate into a nonsense status, and a code that merely *looks* + // like a status can disagree with the one actually served — including a + // success value on a failed request, which contradicts the exit code + // and points the docs link at the wrong page. + for (status, body, expected_in_details) in [ + (400, r#"{"code":100234,"message":"Bad thing"}"#, 100234), + (400, r#"{"code":422,"message":"Bad thing"}"#, 422), + (500, r#"{"code":200,"message":"Bad thing"}"#, 200), + ] { + let json = api_error_from_body(status, body).to_json(); + assert_eq!(json["error"]["code"], status, "body: {body}"); + assert_eq!(json["error"]["message"], "Bad thing", "body: {body}"); + assert_eq!( + json["error"]["details"]["code"], expected_in_details, + "the body's own code must survive in details; body: {body}" + ); + } + } + + #[test] + fn details_keep_a_sibling_that_repeats_the_lifted_sentence() { + // Two fields failing the same way is the common multi-field validation + // shape. Only the entry the sentence was lifted *from* loses its `msg`; + // dropping every string equal to it would leave the second problem + // unattributable. + let err = api_error_from_body( + 422, + r#"{"detail":[{"loc":["body","name"],"msg":"field required"}, + {"loc":["body","email"],"msg":"field required"}]}"#, + ); + let json = err.to_json(); + assert_eq!(json["error"]["message"], "field required"); + let entries = json["error"]["details"]["detail"].as_array().unwrap(); + assert_eq!(entries.len(), 2, "got: {json:#}"); + assert!(entries[0].get("msg").is_none(), "got: {json:#}"); + assert_eq!(entries[1]["msg"], "field required", "got: {json:#}"); + assert_eq!(entries[1]["loc"][1], "email"); + } + + #[test] + fn multi_line_validator_messages_are_not_split_like_clap_output() { + // Our own schema validator emits one bullet per violation. The clap + // splitter would demote every bullet to `help` and leave `message` as + // the bare header, so a non-clap block passes through whole. + let err = CliError::Validation( + "Request body failed schema validation:\n- $.name: expected string, got number\n- $.age: required property missing".to_string(), + ); + let json = err.to_json(); + assert_eq!( + json["error"]["message"], + "Request body failed schema validation:\n- $.name: expected string, got number\n- $.age: required property missing" + ); + assert!(json["error"].get("help").is_none(), "got: {json:#}"); + assert!(json["error"].get("usage").is_none(), "got: {json:#}"); } #[test] @@ -489,6 +1668,7 @@ mod tests { let ctx = ErrorDisplayContext { docs_base_url: None, help_hint: Some("mycli users list --help".to_string()), + format: crate::formatter::OutputFormat::Json, }; // Validation errors should get the hint. let mut out = Vec::new(); @@ -501,7 +1681,7 @@ mod tests { // is unreachable for Api/Auth/Discovery/Other by asserting the helper // doesn't panic and returns clean JSON. for err in [ - CliError::Api { code: 401, message: "denied".to_string(), reason: "authError".to_string() }, + CliError::api(401, "denied", "authError"), CliError::Auth("missing token".to_string()), CliError::Discovery("no spec".to_string()), CliError::Other(anyhow::anyhow!("boom")), @@ -516,7 +1696,7 @@ mod tests { fn write_error_json_no_panic_without_context() { let mut out = Vec::new(); write_error_json( - &CliError::Api { code: 422, message: "invalid".to_string(), reason: "validationError".to_string() }, + &CliError::api(422, "invalid", "validationError"), &mut out, None, ); @@ -526,11 +1706,7 @@ mod tests { #[test] fn test_duplicate_preserves_variant() { - let api = CliError::Api { - code: 404, - message: "Not Found".to_string(), - reason: "notFound".to_string(), - }; + let api = CliError::api(404, "Not Found".to_string(), "notFound".to_string()); let dup = api.duplicate(); assert_eq!(dup.exit_code(), CliError::EXIT_CODE_API); assert_eq!(dup.to_json()["error"]["code"], 404); @@ -635,11 +1811,7 @@ mod tests { #[test] fn is_raw_sentinel_false_for_api_with_raw_reason() { // A server returning reason "raw" must NOT collide with the sentinel. - let err = CliError::Api { - code: 500, - message: String::new(), - reason: "raw".to_string(), - }; + let err = CliError::api(500, String::new(), "raw".to_string()); assert!(!err.is_raw_sentinel()); } @@ -669,19 +1841,22 @@ mod tests { let err = CliError::RawSentinel { code: 500 }; let mut buf: Vec = Vec::new(); write_error_json(&err, &mut buf, None); - assert!(buf.is_empty(), "raw sentinel should suppress stdout JSON, got: {:?}", String::from_utf8_lossy(&buf)); + assert!( + buf.is_empty(), + "raw sentinel should suppress stdout JSON, got: {:?}", + String::from_utf8_lossy(&buf) + ); } #[test] fn write_error_json_normal_api_error_writes_json() { - let err = CliError::Api { - code: 404, - message: "Not Found".to_string(), - reason: "notFound".to_string(), - }; + let err = CliError::api(404, "Not Found".to_string(), "notFound".to_string()); let mut buf: Vec = Vec::new(); write_error_json(&err, &mut buf, None); - assert!(!buf.is_empty(), "normal API error should write JSON to stdout"); + assert!( + !buf.is_empty(), + "normal API error should write JSON to stdout" + ); let s = String::from_utf8(buf).unwrap(); assert!(s.contains("Not Found")); } diff --git a/generators/cli/sdk/src/formatter.rs b/generators/cli/sdk/src/formatter.rs index 32d8f234b7bf..19efb706a4f1 100644 --- a/generators/cli/sdk/src/formatter.rs +++ b/generators/cli/sdk/src/formatter.rs @@ -57,7 +57,8 @@ impl OutputPipeline { /// /// Resolves the output format with this precedence when `--format` is /// **not** passed: - /// 1. an explicit `--format` flag (always wins); + /// 1. an explicit `--format` flag, or the `--human` shorthand + /// (always wins); /// 2. the per-binary `_OUTPUT` env var, if set to a valid format /// (`NAME` is `app_name` uppercased with `-` → `_`, mirroring the /// `_LOG` logging convention); @@ -71,8 +72,17 @@ impl OutputPipeline { /// `--format` values. Callers should map this into their error type /// (e.g. `CliError::Validation`). pub fn from_matches(matches: &clap::ArgMatches, app_name: &str) -> Result { + let flag = |id: &str| { + matches + .try_get_one::(id) + .ok() + .flatten() + .copied() + .unwrap_or(false) + }; let format = match matches.get_one::("format") { Some(s) => OutputFormat::parse(s).map_err(FormatError::UnknownFormat)?, + None if flag("human") => OutputFormat::Table, None => { let env_var = format!("{}_OUTPUT", app_name.to_uppercase().replace('-', "_")); let env_value = std::env::var(env_var).ok(); @@ -223,7 +233,7 @@ fn resolve_default_format(env_value: Option<&str>, stdout_is_terminal: bool) -> } /// Supported output formats. -#[derive(Debug, Clone, PartialEq, Default)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum OutputFormat { /// Pretty-printed JSON (default). #[default] @@ -268,6 +278,75 @@ impl OutputFormat { pub fn from_str(s: &str) -> Self { Self::parse(s).unwrap_or(Self::Json) } + + /// Whether this format is meant for a program rather than a person. + /// + /// Drives the error contract in [`crate::error::write_error_json`]: machine + /// formats get the structured envelope on stdout, human formats get one + /// line on stderr. `Raw` and `Http` are human-side here — they mean "hand + /// me the server's bytes", so an envelope of ours on stdout would corrupt + /// what the caller asked for. + pub fn is_machine_readable(&self) -> bool { + matches!(self, Self::Json | Self::Jsonl | Self::Yaml) + } +} + +/// Resolve the output format from *raw* argv, for use before clap has parsed. +/// +/// The error path runs outside the parsed-matches world (a spec can fail to +/// load before any `ArgMatches` exists), but it must land on the same format +/// the success path would have chosen, so this mirrors +/// [`OutputPipeline::from_matches`]: an explicit `--format` or `--human` +/// wins, then `_OUTPUT`, then the TTY-aware default. Unknown `--format` +/// values fall through to the default rather than erroring — the format +/// resolution performed by clap already reports those. +pub fn resolve_format_from_raw_args(args: &[String], app_name: &str) -> OutputFormat { + // Anything after `--` belongs to the command being invoked, not to us. + let args = args_before_separator(args); + if let Some(parsed) = explicit_format_arg(args).and_then(|v| OutputFormat::parse(&v).ok()) { + return parsed; + } + // Deliberately no `--json` rung: on every endpoint with a request body + // `--json` is that body's flag, so reading it as a format would make + // `create --json '{...}'` silently pin the output format too. + if args.iter().any(|a| a == "--human") { + return OutputFormat::Table; + } + let env_var = format!("{}_OUTPUT", app_name.to_uppercase().replace('-', "_")); + let env_value = std::env::var(env_var).ok(); + resolve_default_format(env_value.as_deref(), std::io::stdout().is_terminal()) +} + +/// True when argv carries an explicit `--format` naming a machine format. +/// +/// Distinct from "the resolved format is machine-readable", which is also true +/// for a plain pipe. Callers that change *what* a command does — rather than +/// how its output is rendered — must key off the explicit request only. +pub fn explicit_machine_format(args: &[String]) -> bool { + explicit_format_arg(args_before_separator(args)) + .and_then(|v| OutputFormat::parse(&v).ok()) + .is_some_and(|f| f.is_machine_readable()) +} + +/// The leading slice of argv that clap treats as our own flags. +fn args_before_separator(args: &[String]) -> &[String] { + match args.iter().position(|a| a == "--") { + Some(i) => &args[..i], + None => args, + } +} + +/// Extract the value of an explicit `--format` / `--format=` flag from argv. +fn explicit_format_arg(args: &[String]) -> Option { + for (i, a) in args.iter().enumerate() { + if let Some(rest) = a.strip_prefix("--format=") { + return Some(rest.to_string()); + } + if a == "--format" { + return args.get(i + 1).cloned(); + } + } + None } /// Format a JSON value according to the specified output format. @@ -1215,6 +1294,135 @@ mod tests { // Default-format precedence: flag > _OUTPUT env > TTY-aware default // ----------------------------------------------------------------------- + #[test] + fn is_machine_readable_covers_structured_formats_only() { + for f in [OutputFormat::Json, OutputFormat::Jsonl, OutputFormat::Yaml] { + assert!(f.is_machine_readable(), "{f:?} should be machine-readable"); + } + // Raw/Http hand back the server's bytes — our envelope would corrupt them. + for f in [ + OutputFormat::Table, + OutputFormat::Csv, + OutputFormat::Raw, + OutputFormat::Http, + ] { + assert!(!f.is_machine_readable(), "{f:?} should be human-side"); + } + } + + #[test] + fn explicit_format_arg_reads_both_spellings() { + let split = vec![ + "users".to_string(), + "--format".to_string(), + "yaml".to_string(), + ]; + assert_eq!(explicit_format_arg(&split).as_deref(), Some("yaml")); + let joined = vec!["--format=csv".to_string()]; + assert_eq!(explicit_format_arg(&joined).as_deref(), Some("csv")); + assert!(explicit_format_arg(&["users".to_string()]).is_none()); + // A trailing `--format` with no value must not panic. + assert!(explicit_format_arg(&["--format".to_string()]).is_none()); + } + + #[test] + fn resolve_format_from_raw_args_honors_human_shorthand() { + // `--human` sits at the same precedence rung as `--format`, so a caller + // can force the interactive rendering without naming a format. + assert_eq!( + resolve_format_from_raw_args(&["users".to_string(), "--human".to_string()], "my-cli"), + OutputFormat::Table + ); + // An explicit --format still outranks it (clap rejects the combination + // before this runs; the pre-clap error path must not disagree). + assert_eq!( + resolve_format_from_raw_args( + &["--human".to_string(), "--format=yaml".to_string()], + "my-cli" + ), + OutputFormat::Yaml + ); + } + + #[test] + fn resolve_format_from_raw_args_leaves_json_body_flag_alone() { + // `--json` is the request-body flag on every endpoint that has a body; + // treating it as an output format would make sending a body silently + // pin the rendering too. With stdout not a terminal under test, the + // TTY-aware default applies either way — what matters is that the flag + // itself carries no format meaning. + assert_eq!( + resolve_format_from_raw_args( + &[ + "users".to_string(), + "create".to_string(), + "--json".to_string(), + "{\"name\":\"Ada\"}".to_string(), + "--human".to_string(), + ], + "my-cli" + ), + OutputFormat::Table + ); + } + + #[test] + fn resolve_format_from_raw_args_ignores_args_after_separator() { + // `mycli run -- --format=table` forwards the flag to the invoked + // command; it must not flip our error representation. + assert_eq!( + args_before_separator(&[ + "run".to_string(), + "--".to_string(), + "--format=table".to_string() + ]), + &["run".to_string()] + ); + assert_eq!( + explicit_format_arg(args_before_separator(&[ + "run".to_string(), + "--".to_string(), + "--format=table".to_string(), + ])), + None + ); + } + + #[test] + fn explicit_machine_format_requires_an_actual_flag() { + // Gates behaviour changes (`--help` -> operation catalog), so a plain + // pipe must not qualify: ` --help | less` has to keep showing help. + assert!(explicit_machine_format(&["--format=json".to_string()])); + assert!(explicit_machine_format(&[ + "--format".to_string(), + "yaml".to_string() + ])); + assert!(!explicit_machine_format(&["--format=table".to_string()])); + assert!(!explicit_machine_format(&["--help".to_string()])); + assert!(!explicit_machine_format(&["--format=nonsense".to_string()])); + // After `--` the flag belongs to the invoked command, not to us. + assert!(!explicit_machine_format(&[ + "run".to_string(), + "--".to_string(), + "--format=json".to_string() + ])); + } + + #[test] + fn resolve_format_from_raw_args_prefers_explicit_flag() { + // Explicit flag beats env and TTY state, so an agent can always pin JSON. + let args = vec!["--format".to_string(), "json".to_string()]; + assert_eq!( + resolve_format_from_raw_args(&args, "my-cli"), + OutputFormat::Json + ); + let args = vec!["--format=table".to_string()]; + assert_eq!( + resolve_format_from_raw_args(&args, "my-cli"), + OutputFormat::Table + ); + } + #[test] fn resolve_default_format_no_env_piped_is_json() { // No env override + non-terminal stdout (piped/redirected) → JSON. diff --git a/generators/cli/sdk/src/graphql/executor.rs b/generators/cli/sdk/src/graphql/executor.rs index 14cdf3a9051d..ced094eef2b8 100644 --- a/generators/cli/sdk/src/graphql/executor.rs +++ b/generators/cli/sdk/src/graphql/executor.rs @@ -64,9 +64,10 @@ fn parse_and_validate_inputs( Map::new() }; - let gql = method.graphql.as_ref().ok_or_else(|| { - CliError::Discovery("GraphQL method info missing from spec".to_string()) - })?; + let gql = method + .graphql + .as_ref() + .ok_or_else(|| CliError::Discovery("GraphQL method info missing from spec".to_string()))?; for (param_name, param_def) in &method.parameters { if param_def.required @@ -111,10 +112,12 @@ fn build_http_request( /// and the partial data is returned. Only when there is no `data` at all do we /// treat the errors as fatal. fn parse_graphql_response(body_text: &str) -> Result { - let json_val: Value = serde_json::from_str(body_text).map_err(|e| CliError::Api { - code: 400, - message: format!("Invalid GraphQL response: {e}"), - reason: "graphql_parse_error".to_string(), + let json_val: Value = serde_json::from_str(body_text).map_err(|e| { + CliError::api( + 400, + format!("Invalid GraphQL response: {e}"), + "graphql_parse_error", + ) })?; let has_data = json_val @@ -132,10 +135,18 @@ fn parse_graphql_response(body_text: &str) -> Result { if has_data { eprintln!("GraphQL partial errors: {message}"); } else { + // GraphQL puts every error's location/path/extensions here; + // keep them for machine consumers. Each entry keeps its own + // `message` because it pairs with that entry's `path` — the + // top-level `message` is a join of all of them, not one lifted + // field, so there is no single occurrence to drop. + let details = crate::error::prune(json!({ "errors": errors })); return Err(CliError::Api { code: 400, message, reason: "graphql_error".to_string(), + details, + help: None, }); } } @@ -155,10 +166,12 @@ fn parse_graphql_response(body_text: &str) -> Result { json_val }; - serde_json::to_string(&unwrapped).map_err(|e| CliError::Api { - code: 500, - message: format!("Failed to serialize GraphQL response: {e}"), - reason: "graphql_serialize_error".to_string(), + serde_json::to_string(&unwrapped).map_err(|e| { + CliError::api( + 500, + format!("Failed to serialize GraphQL response: {e}"), + "graphql_serialize_error", + ) }) } @@ -299,7 +312,15 @@ pub async fn execute_method( } let built = request.build().map_err(|e| { - CliError::Other(anyhow::Error::from(e).context("Failed to build HTTP request")) + // `Validation`, not `Other`: `build()` fails on a malformed URL + // or header value, which comes from `--base-url` or a flag the + // user typed. Reporting it as `code: 500` claimed a server + // status for a request that was never sent, and buried a + // fixable input error under an internal-error exit code. + CliError::Validation(format!( + "Failed to build HTTP request: {}", + crate::error::error_chain(&e) + )) })?; if debug { let query_str = input.body.get("query").and_then(|q| q.as_str()).unwrap_or(""); @@ -379,7 +400,14 @@ pub async fn execute_method( continue; } crate::http::maybe_emit_tls_hint(http_config, &e); - return Err(anyhow::Error::from(e).context("HTTP request failed").into()); + // `Network`, not `Other`: nothing answered, so there is no + // HTTP status to put in `error.code`. The chain is walked + // because reqwest's own Display stops at "error sending + // request" — "Connection refused" is the actionable part. + return Err(CliError::Network(format!( + "HTTP request failed: {}", + crate::error::error_chain(&e) + ))); } } }; diff --git a/generators/cli/sdk/src/openapi/executor.rs b/generators/cli/sdk/src/openapi/executor.rs index e94a96832c32..88faa7bddbb4 100644 --- a/generators/cli/sdk/src/openapi/executor.rs +++ b/generators/cli/sdk/src/openapi/executor.rs @@ -2279,7 +2279,15 @@ pub async fn execute_method( } let built = request.build().map_err(|e| { - CliError::Other(anyhow::Error::from(e).context("Failed to build HTTP request")) + // `Validation`, not `Other`: `build()` fails on a malformed URL + // or header value, which comes from `--base-url` or a flag the + // user typed. Reporting it as `code: 500` claimed a server + // status for a request that was never sent, and buried a + // fixable input error under an internal-error exit code. + CliError::Validation(format!( + "Failed to build HTTP request: {}", + crate::error::error_chain(&e) + )) })?; if debug { crate::debug::dump_request( @@ -2371,7 +2379,14 @@ pub async fn execute_method( // behind corporate proxies / interception tools. The hint is // a side effect; the error then propagates up like any other. crate::http::maybe_emit_tls_hint(http_config, &e); - return Err(anyhow::Error::from(e).context("HTTP request failed").into()); + // `Network`, not `Other`: nothing answered, so there is no + // HTTP status to put in `error.code`. The chain is walked + // because reqwest's own Display stops at "error sending + // request" — "Connection refused" is the actionable part. + return Err(CliError::Network(format!( + "HTTP request failed: {}", + crate::error::error_chain(&e) + ))); } } }; diff --git a/generators/cli/sdk/src/sdk_executor.rs b/generators/cli/sdk/src/sdk_executor.rs index 328451582c31..670eda74a83e 100644 --- a/generators/cli/sdk/src/sdk_executor.rs +++ b/generators/cli/sdk/src/sdk_executor.rs @@ -26,7 +26,7 @@ use std::pin::Pin; use reqwest::{Client, Request, Response}; use crate::auth::{DynAuthProvider, EndpointAuthMetadata}; -use crate::error::CliError; +use crate::error::{api_error_from_body, CliError}; use crate::http::HttpConfig; use crate::openapi::discovery::RetriesConfig; use crate::openapi::executor::{decide_retry, RetryOutcome}; @@ -376,17 +376,11 @@ impl SdkError { /// Convert into the CLI's native error type. pub fn into_cli_error(self) -> CliError { match self { - Self::Http { status, body } => CliError::Api { - code: status, - message: body, - reason: http_status_reason(status).to_string(), - }, - Self::Network(msg) => { - CliError::Other(anyhow::anyhow!("SDK network error: {msg}")) - } - Self::Timeout(msg) => { - CliError::Other(anyhow::anyhow!("SDK request timeout: {msg}")) - } + Self::Http { status, body } => api_error_from_body(status, &body), + // Neither reached a server, so neither has an HTTP status. The + // OpenAPI path reports these as `networkError`; SDK mode must agree. + Self::Network(msg) => CliError::Network(format!("SDK network error: {msg}")), + Self::Timeout(msg) => CliError::Network(format!("SDK request timeout: {msg}")), Self::Auth(msg) => CliError::Auth(msg), Self::Other(msg) => { CliError::Other(anyhow::anyhow!("SDK error: {msg}")) @@ -426,27 +420,6 @@ impl From for SdkError { } } -/// Map an HTTP status code to a short reason string for [`CliError::Api`]. -fn http_status_reason(status: u16) -> &'static str { - match status { - 400 => "badRequest", - 401 => "unauthorized", - 403 => "forbidden", - 404 => "notFound", - 408 => "requestTimeout", - 409 => "conflict", - 422 => "unprocessableEntity", - 429 => "rateLimited", - 500 => "internalServerError", - 502 => "badGateway", - 503 => "serviceUnavailable", - 504 => "gatewayTimeout", - _ => "httpError", - } -} - - - #[cfg(test)] mod tests { use super::*; @@ -500,7 +473,12 @@ mod tests { }; let cli_err = err.into_cli_error(); match cli_err { - CliError::Api { code, message, reason } => { + CliError::Api { + code, + message, + reason, + .. + } => { assert_eq!(code, 404); assert_eq!(message, "not found"); assert_eq!(reason, "notFound"); @@ -510,19 +488,23 @@ mod tests { } #[test] - fn sdk_error_network_maps_to_cli_other() { - let err = SdkError::Network("connection refused".into()); - let cli_err = err.into_cli_error(); - assert!(matches!(cli_err, CliError::Other(_))); - assert!(cli_err.to_string().contains("network error")); - } - - #[test] - fn sdk_error_timeout_maps_to_cli_other() { - let err = SdkError::Timeout("timed out after 30s".into()); - let cli_err = err.into_cli_error(); - assert!(matches!(cli_err, CliError::Other(_))); - assert!(cli_err.to_string().contains("timeout")); + fn sdk_error_network_and_timeout_carry_no_http_status() { + // Neither reached a server. Reporting `code: 500` told an agent the API + // had failed and invited a retry against a host never contacted. + for (err, needle) in [ + (SdkError::Network("connection refused".into()), "network error"), + (SdkError::Timeout("timed out after 30s".into()), "timeout"), + ] { + let cli_err = err.into_cli_error(); + assert!(matches!(cli_err, CliError::Network(_)), "got: {cli_err:?}"); + assert!(cli_err.to_string().contains(needle), "got: {cli_err}"); + let json = cli_err.to_json(); + assert_eq!(json["error"]["reason"], "networkError"); + assert!(json["error"].get("code").is_none(), "got: {json:#}"); + // Still `other` at the process boundary — a sixth exit code would + // change the documented table every consumer branches on. + assert_eq!(cli_err.exit_code(), CliError::EXIT_CODE_OTHER); + } } #[test] @@ -559,11 +541,26 @@ mod tests { } #[test] - fn http_status_reason_known_codes() { - assert_eq!(http_status_reason(401), "unauthorized"); - assert_eq!(http_status_reason(429), "rateLimited"); - assert_eq!(http_status_reason(503), "serviceUnavailable"); - assert_eq!(http_status_reason(999), "httpError"); + fn sdk_error_http_json_body_becomes_structured_details() { + // The generated-SDK path shares the parser with the OpenAPI path, so a + // JSON body must not land in `message` as an escaped document here either. + let err = SdkError::Http { + status: 500, + body: r#"{"status":"internal_server_error","message":"Server blew up"}"#.into(), + }; + match err.into_cli_error() { + CliError::Api { + message, + reason, + details, + .. + } => { + assert_eq!(message, "Server blew up"); + assert_eq!(reason, "internalServerError"); + assert_eq!(details.unwrap()["status"], "internal_server_error"); + } + other => panic!("expected CliError::Api, got: {other:?}"), + } } #[test] diff --git a/generators/cli/sdk/src/websocket/error.rs b/generators/cli/sdk/src/websocket/error.rs index 231c95cc20a1..c2dcaf79aca4 100644 --- a/generators/cli/sdk/src/websocket/error.rs +++ b/generators/cli/sdk/src/websocket/error.rs @@ -60,11 +60,11 @@ pub fn map_handshake_error(err: tungstenite::Error) -> CliError { "WebSocket endpoint not found (404): {}", truncate(&body, 200), )), - 500..=599 => CliError::Api { - code: status, - message: format!("WebSocket upgrade failed: {}", truncate(&body, 200)), - reason: "wsHandshakeServerError".into(), - }, + 500..=599 => CliError::api( + status, + format!("WebSocket upgrade failed: {}", truncate(&body, 200)), + "wsHandshakeServerError", + ), _ => CliError::Other(anyhow::anyhow!( "WebSocket upgrade failed with status {status}: {}", truncate(&body, 200), diff --git a/generators/cli/sdk/tests/help_catalog.rs b/generators/cli/sdk/tests/help_catalog.rs new file mode 100644 index 000000000000..78ec33aebacc --- /dev/null +++ b/generators/cli/sdk/tests/help_catalog.rs @@ -0,0 +1,82 @@ +//! `--help` with an explicit machine format emits the operation catalog. +//! +//! The generated README has always documented +//! ` --help --format json | jq '.operations | length'` as the +//! machine-readable list of every operation; it printed prose instead, so the +//! documented command failed with a jq parse error. +//! +//! Driven through the real binary because the behaviour lives in +//! `dispatch_pipeline`'s pre-clap interception, which has no unit seam. These +//! tests are template-author-only — `tests/**` is excluded from generated +//! output via `.sdk-ignore.json`. + +use std::process::{Command, Output}; + +fn run(args: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_openapi-fixture")) + .args(args) + .output() + .expect("failed to run the fixture binary") +} + +fn stdout(out: &Output) -> String { + String::from_utf8_lossy(&out.stdout).into_owned() +} + +#[test] +fn help_with_explicit_json_emits_the_operation_catalog() { + let out = run(&["--help", "--format", "json"]); + assert_eq!(out.status.code(), Some(0)); + let parsed: serde_json::Value = + serde_json::from_str(&stdout(&out)).expect("must be JSON, not prose"); + let ops = parsed["operations"] + .as_array() + .expect("catalog carries an `operations` array"); + assert!(!ops.is_empty(), "got: {parsed:#}"); + // Identical to what `--schema` produces for the same scope. + assert_eq!(stdout(&out), stdout(&run(&["--schema"]))); +} + +#[test] +fn help_without_an_explicit_format_stays_prose() { + // ` --help | less` is piped, so the resolved format is machine-readable + // by default. Keying off that instead of the explicit flag would replace + // help with JSON for anyone paging it. + let out = run(&["--help"]); + assert_eq!(out.status.code(), Some(0)); + assert!( + serde_json::from_str::(&stdout(&out)).is_err(), + "expected prose, got JSON: {}", + stdout(&out) + ); + + // An explicit *human* format likewise stays prose. + let out = run(&["--help", "--format", "table"]); + assert!(serde_json::from_str::(&stdout(&out)).is_err()); +} + +#[test] +fn help_on_a_builtin_subcommand_falls_back_to_prose() { + // `completion`, `man` and `auth login` are not API operations, so the + // catalog lookup finds nothing. Someone asking for help on one must get + // help — erroring `discoveryError` with exit 4 is a worse answer than the + // prose they asked for. + for path in [ + vec!["completion", "--help", "--format", "json"], + vec!["auth", "login", "--help", "--format", "json"], + ] { + let out = run(&path); + assert_eq!(out.status.code(), Some(0), "args: {path:?}"); + assert!(!stdout(&out).is_empty(), "args: {path:?}"); + } +} + +#[test] +fn explicit_schema_on_a_pathless_command_still_errors() { + // The fallback above must not weaken `--schema` itself: asking for a + // document that does not exist is a real error. + let out = run(&["completion", "--schema"]); + assert_eq!(out.status.code(), Some(4)); + let parsed: serde_json::Value = serde_json::from_str(&stdout(&out)).unwrap(); + assert_eq!(parsed["error"]["reason"], "discoveryError"); +} diff --git a/generators/cli/src/emitReadme.ts b/generators/cli/src/emitReadme.ts index a4a2aaaf186b..fdc8b6744d04 100644 --- a/generators/cli/src/emitReadme.ts +++ b/generators/cli/src/emitReadme.ts @@ -443,14 +443,19 @@ function generateAdvanced(args: { binaryName: string; envPrefix: string }): Bloc "", "### Output formats", "", - "Use the global `--format` flag to control output. Supported values: `json` (default), `table`, `yaml`, `csv`.", + "Use the global `--format` flag to control output. Supported values: `json`, `table`, `yaml`, `csv`, `jsonl`, `raw`, `http`.", + "", + "Without `--format`, output (including errors) is `table` when stdout is a terminal and `json` when it is piped or redirected — so scripts and agents get JSON by default. Pass `--human` to keep the interactive rendering when piping to a pager, and `--format json` to pin JSON in a terminal.", "", "```bash", `# Pipe JSON output through jq`, `${binaryName} --format json | jq`, "", - "# Machine-readable catalog of every operation", - `${binaryName} --help --format json | jq 'length'`, + "# Keep the human rendering even when piped", + `${binaryName} --human | less`, + "", + "# Machine-readable catalog of every operation (same as --schema)", + `${binaryName} --help --format json | jq '.operations | length'`, "```", "", "### Shell completion", diff --git a/generators/cli/src/generateSdk.ts b/generators/cli/src/generateSdk.ts index dc762eca24a0..9dab3490620b 100644 --- a/generators/cli/src/generateSdk.ts +++ b/generators/cli/src/generateSdk.ts @@ -243,11 +243,9 @@ where fn convert_api_error(e: ${sdkCrateSnake}::ApiError) -> CliError { match e { - ${sdkCrateSnake}::ApiError::Http { status, message } => CliError::Api { - code: status, - message, - reason: http_status_reason(status).to_string(), - }, + ${sdkCrateSnake}::ApiError::Http { status, message } => { + fern_cli_sdk::error::api_error_from_body(status, &message) + } ${sdkCrateSnake}::ApiError::Network(err) => { CliError::Other(anyhow::anyhow!("SDK network error: {err}")) } @@ -258,23 +256,6 @@ fn convert_api_error(e: ${sdkCrateSnake}::ApiError) -> CliError { other => CliError::Other(anyhow::anyhow!("SDK error: {other}")), } } - -fn http_status_reason(status: u16) -> &'static str { - match status { - 400 => "badRequest", - 401 => "unauthorized", - 403 => "forbidden", - 404 => "notFound", - 408 => "requestTimeout", - 409 => "conflict", - 429 => "tooManyRequests", - 500 => "internalServerError", - 502 => "badGateway", - 503 => "serviceUnavailable", - 504 => "gatewayTimeout", - _ => "httpError", - } -} `; } diff --git a/generators/cli/src/wireTests/harness.ts b/generators/cli/src/wireTests/harness.ts index 9fa27a11715b..bb5a61bf55cd 100644 --- a/generators/cli/src/wireTests/harness.ts +++ b/generators/cli/src/wireTests/harness.ts @@ -971,10 +971,34 @@ async fn run_case(id: &str) { manifest.binary_name, args.join(" ") ); + // The harness pipes stdout, so the CLI resolves the machine-readable + // rendering and writes the error envelope there — a human line on + // stderr is what a terminal gets instead, and asserting on stderr here + // would pass for any failure, including one that never left the + // process. Parsing the envelope pins the failure to *this* response: + // \`error.code\` must be the status the mock served. + let envelope: serde_json::Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| { + panic!( + "expected a JSON error envelope on stdout for {id} (a {} response), got unparseable output: {e}\n stdout: {stdout}\n stderr: {stderr}", + case.response.status + ) + }); + let error = envelope.get("error").unwrap_or_else(|| { + panic!( + "expected an \`error\` object in the envelope for {id}\n stdout: {stdout}" + ) + }); + assert_eq!( + error.get("code").and_then(|c| c.as_u64()), + Some(u64::from(case.response.status)), + "expected error.code to be the served status for {id}\n stdout: {stdout}" + ); assert!( - !stderr.trim().is_empty(), - "expected an error message on stderr for {id} (a {} response), got empty stderr", - case.response.status + error + .get("message") + .and_then(|m| m.as_str()) + .is_some_and(|m| !m.trim().is_empty()), + "expected a non-empty error.message for {id}\n stdout: {stdout}" ); // A non-zero exit alone proves nothing: the CLI must have failed // *because of the mocked response*, not before it ever made the call. diff --git a/generators/cli/versions.yml b/generators/cli/versions.yml index c9e6db8d80aa..949e136062e0 100644 --- a/generators/cli/versions.yml +++ b/generators/cli/versions.yml @@ -1,4 +1,256 @@ # yaml-language-server: $schema=../../fern-versions-yml.schema.json +- version: 0.37.0 + changelogEntry: + - summary: | + The "no credentials" hint is no longer tied to 401/403. The condition it + describes — this request went out carrying nothing — has nothing to do with + the status code, but the hint was gated on those two, so an API that + expresses "not authenticated" any other way said nothing useful. ElevenLabs + answers 404 for a workspace scoped to the anonymous principal (deliberate: a + 401 would confirm the resource exists), so a logged-out user read + `Workspace 1anonymous1 not found` with no indication that the fix was + `auth login`. + + Any failed status now carries a `No credentials were sent with this + request. Set ...` note in `error.help` when the provider had nothing for + that endpoint. This only annotates: the error keeps its class and exit code, + and the request is still sent, because the server — not the spec — is the + authority on whether an endpoint needed auth. Public endpoints are + unaffected, and 401/403 still escalate to `error[auth]` with exit 2. + type: fix + - summary: | + The "check for shadowing" advice on a rejected credential now appears only + when there is more than one visible credential source. Shadowing means one + source silently outranking another, which cannot happen with a single + source — yet the note sent every user to `auth status` to compare a list of + one against itself. On a scope failure (`OAuth token does not have required + permissions: user_read`) that was worse than noise: the credential is the + right one from the right place, and the advice points away from the real + fix. The source is still named whenever credentials were sent, since knowing + which one was used is useful either way. + type: fix + - summary: | + Transport failures no longer report a fabricated HTTP status. A request that + never reached a server — DNS, TLS, connection refused, timeout — was folded + into the generic internal-error shape and emitted `code: 500` with + `reason: "internalError"`, telling a consumer the *API* had failed and + inviting a retry against a host that was never contacted. These now produce + `reason: "networkError"` and **no** `code` field, since the field is + documented as the HTTP status and there is none. Exit code is unchanged at + 5, and the human rendering is labelled `error[network]:`. + + The message also keeps the part that matters: `reqwest` stops at "error + sending request for url (...)" and buries "Connection refused" two levels + down its `source()` chain, so the chain is now walked and joined. SDK-mode + CLIs report network and timeout failures the same way as OpenAPI-mode ones. + type: fix + - summary: | + `auth status` now honors the global `--format`. It gated JSON output on its + own `--json` boolean, so `auth status --format json` printed the human table + with no error and no hint — the first command an agent runs, silently + ignoring the flag it uses everywhere else. `--format`, `--human`, the + `_OUTPUT` env var and the piped default now all apply; `--json` + remains as an alias so existing scripts keep working. + type: fix + - summary: | + Fixed a literal `` placeholder appearing in credential hints. The + keyring source rendered `populated by \` auth login\`` instead of naming + the binary. CLIs using an OAuth login flow were unaffected — that provider + overrides the hint and substituted correctly — so the placeholder only + reached users on bearer, header and basic schemes. + type: fix + - summary: | + `error.details` no longer repeats the sentence already reported in + `error.message`. The parsed server body is still carried structurally, so + fields a consumer cannot get anywhere else — `request_id`, a validation + error's `loc`, a GraphQL error's `path`/`extensions` — are unchanged, but the + one field the message was lifted from is removed, and `details` is + omitted entirely when the body said nothing beyond that message (so + `{"detail": "Not Found"}` now yields just `code`/`message`/`reason`). + + Removal is by position, not by value: when several entries report the same + problem — two required fields both failing with `"field required"`, the + common multi-field validation shape — only the entry the sentence came from + loses it. Dropping every string equal to the message would leave the + remaining problems unattributable to their `loc`. + type: fix + - summary: | + Fixed API errors being printed twice and with the server's JSON body escaped + inside the `message` string. An error is now rendered in exactly one + representation — the JSON envelope on stdout for machine formats + (`json`/`jsonl`/`yaml`, which includes the piped/non-TTY default), or a single + `error[api]: ` line on stderr for `table`/`csv` — and the server's + body is parsed rather than stringified: a sentence lands in `message`, the + parsed body in `error.details`, and `reason` is derived from the HTTP status + (`internalServerError` for 500) instead of always `httpError`. Top-level + shapes such as `{"status", "message"}`, `{"error": ""}`, RFC 7807 + `detail`/`title`, and OAuth 2.0 `error_description` are now recognized. Two + consequences worth noting for anyone scripting against these fields: 429 now + always reports `reason: "rateLimited"`. Previously a generated CLI reported + either that or `tooManyRequests` for the same status depending on which code + path produced the error — the runtime's `sdk_executor` already said + `rateLimited`, while the generated request adapter said `tooManyRequests`. + Both now come from one mapping. Also, + `error.code` is always the HTTP status the server answered with. A body's own + `code` never overrides it — not an internal numbering (`{"code": 100234}`, + which used to truncate into a nonsense status), and not one that merely looks + like a status, which could disagree with what was actually served and point + the docs link at the wrong page. The body's `code` is still available under + `error.details`. + type: fix + - summary: | + Added a global `--human` flag, shorthand for `--format table`, for the one + case TTY detection gets wrong: a person piping into a pager or a file still + wants the interactive rendering. Without it the existing TTY-aware default + applies (`table` on a terminal, `json` when piped), and passing it alongside + `--format` is rejected. There is deliberately no `--json` counterpart: + endpoints with a request body already spell that flag `--json `, so + the name cannot mean "render JSON" everywhere; use `--format json`. + type: feat + - summary: | + `--format jsonl` now emits errors as a single line. The envelope was + pretty-printed regardless of format, so a line-delimited consumer — the only + reason to pick NDJSON — read a bare `{` and failed on the first line. Errors + under `jsonl` are compact; `json` and `yaml` keep the readable multi-line + rendering. + type: fix + - summary: | + A request that fails to build is now a validation error rather than an + internal one. `reqwest`'s `build()` fails on a malformed URL or header + value, which comes from `--base-url` or a flag the user typed, but the + failure was reported as `code: 500` / `reason: "internalError"` with exit 5. + A mistyped `--base-url` claimed a server status for a request that was never + sent and buried a fixable input error under an internal-error exit code. It + now reports `code: 400` / `reason: "validationError"` with exit 3. + type: fix + - summary: | + ` --help --format json` now emits the operation catalog, as the + generated README has always documented. It printed the human help instead, + so the README's own example (`--help --format json | jq …`) failed with a + jq parse error. It now returns exactly what `--schema` returns for the same + scope. + + Only an *explicit* machine `--format` triggers this — the piped default does + not, so ` --help | less` still shows help. A path with no catalog entry + (`auth login`, `completion`, `man` — built-ins, not API operations) falls + back to prose rather than erroring; `--schema` on such a path still reports + `discoveryError`, since asking for a document that does not exist is a real + error. The README example was also + corrected: the catalog is an object with `globalFlags` and `operations`, so + counting operations is `jq '.operations | length'`, not `jq 'length'` (which + counted the two top-level keys). + type: fix + - summary: | + Removed `error::print_error_json` from the vendored runtime. It wrote the + JSON envelope to stdout unconditionally — the exact double-output behaviour + the rest of this release removes — so it was a working copy of the bug left + reachable under an inviting name. It had no callers in the runtime, in the + generator's emitted code, or in any generated CLI. `write_error_json`, which + honours the resolved format, is the entry point. + type: fix + - summary: | + A credential the CLI could not read is no longer treated as a credential the + user never stored. Denying the OS keychain prompt made + `AuthCredentialSource::resolve` return "nothing here" — the same answer as an + empty keyring — so the CLI sent the request unauthenticated and the user saw + whatever the API says to an anonymous caller. Against ElevenLabs that is + `error[api]: Workspace 1anonymous1 not found.` with exit 1: a 404 for a + problem that is an authentication failure. + + Credential resolution now distinguishes "not configured" from "configured but + unreadable". On the request path a denied or broken keyring surfaces as + `error[auth]` with exit 2, naming the backend failure. A fallback chain stops + at the denial instead of falling through to a later empty source, which would + have masked it again. Probing paths — `auth status`, `has_credentials` — still + degrade to "no credential" rather than aborting, since a status listing should + report what is usable, not fail. + type: fix + - summary: | + Terminal error output no longer repeats itself. Services routinely echo one + token across several fields — ElevenLabs sends `code`, `status` and `type` all + restating the same `workspace_not_found` — which turned a one-fact 404 into + five lines saying the same thing four times. A detail line whose value already + appeared in the message, the reason, or an earlier line is now dropped, and a + sole wrapper object (`{"detail": {...}}`) no longer prefixes every line with a + word that names nothing the reader is looking for. + + A service-specific reason is now shown on the message line, in parentheses. + It was previously the one useful field that appeared in the JSON envelope but + never in the terminal. A reason derived from the HTTP status (`notFound` on a + 404) stays hidden, since it says nothing the label and message do not. + + error[api]: Workspace 1anonymous1 not found. (workspace_not_found) + request_id: 6c312fb855fc7b476570b44e2b660988 + type: not_found + + The JSON envelope is unchanged and still carries every field. + type: fix + - summary: | + Terminal error output now reports what the server said beyond the failure + sentence. A field-level rejection used to read as `error[api]: field + required` with no way to tell which field — the `loc` was in the JSON + envelope, which is exactly what the person in the terminal cannot see. The + parsed body is now flattened one `path: value` per line under the message: + + error[api]: value is not a valid email address + detail[0].loc: body, email + detail[0].type: value_error.email + + Lists of scalars stay on one line, since splitting FastAPI's two-element + `loc` buries the field name it exists to report. Output is capped at ten + lines, with a final line naming what was elided and pointing at + `--format json`. The JSON envelope is unchanged. + type: fix + - summary: | + Non-JSON error bodies no longer break the "`message` is one sentence" + guarantee. A CDN or load balancer answering with an HTML page used to have + its whole document — doctype, newlines and markup — pasted into + `error.message`; the previous 500-byte clip did not help, because a short + page is still multi-line markup. Markup and over-long bodies now yield + `HTTP 502 badGateway (non-JSON response, 1284 bytes)` in `message`, with the + bytes preserved (whitespace-collapsed and clipped) in `error.details.body`. + A short plain-text body — the common proxy case — is still the message + itself, now collapsed onto one line if the server wrapped it. + type: fix + - summary: | + Two error bodies that produced an unhelpful envelope now read correctly. An + empty body gave `message: ""`, which reads as the CLI having lost the error + rather than the server having sent nothing; it now reports + `HTTP `. A body that is a bare JSON string + (`"Service temporarily unavailable"`) was summarised as + `HTTP 500 internalServerError` with the only text the server sent exiled to + `details`; it is now the message. A body that is a bare JSON list of + problems is also recognised, matching the existing `errors`/`detail` + handling one level out. + type: fix + - summary: | + Made the error envelope's `message` field one sentence for every error class. + Advice that is not part of what the server reported — today the "Credentials + were supplied via: ..." hint shown on 401/403 — moved out of `message` into + its own `error.help` field, so a consumer keying off `error.message` sees the + same shape whether or not the CLI had advice to offer. The docs link (opt-in + via `error_docs_base_url`, previously human-only) now also appears as + `error.docs_url`, so the JSON envelope no longer silently drops information + the human rendering shows. Human output keeps the same order, one item per + line: message, then what the server said beyond it, then hint, then link. + type: fix + - summary: | + Usage errors (a mistyped subcommand, a bad flag value, conflicting flags) no + longer stuff clap's whole multi-line block into `error.message` when output is + machine-readable. The failure sentence stays in `message`, clap's `tip:` and + `[possible values: ...]` lines move to `error.help`, the usage string becomes + `error.usage`, and the `For more information, try '--help'` boilerplate is + dropped — so every field is a single line, as with API errors. Human output on + a terminal is unchanged: it still prints clap's full rendering, tip and usage + included. + + Only clap's own blocks are split. A multi-line message from one of the CLI's + validators — schema validation listing one bullet per violation, say — passes + through whole, since applying the split there would demote the violations to + `help` and leave `message` as the bare header. + type: fix + createdAt: "2026-08-21" + irVersion: 67 - version: 0.36.0 changelogEntry: - summary: | diff --git a/packages/cli/cli/changes/5.101.0/api-reference-layout.yml b/packages/cli/cli/changes/5.101.0/api-reference-layout.yml new file mode 100644 index 000000000000..3bc125d1d643 --- /dev/null +++ b/packages/cli/cli/changes/5.101.0/api-reference-layout.yml @@ -0,0 +1,7 @@ +# yaml-language-server: $schema=../../../../../fern-changes-yml.schema.json + +- summary: | + Add a `layout.api-reference-layout` option to docs.yml. Set it to `cards` to render + nested API reference fields as bordered cards, or `indented` (the default) to render + each nested level as an indented branch with a connector line and a path breadcrumb. + type: feat diff --git a/packages/cli/cli/versions.yml b/packages/cli/cli/versions.yml index 8446269e4af3..2b92a0d350a8 100644 --- a/packages/cli/cli/versions.yml +++ b/packages/cli/cli/versions.yml @@ -1,4 +1,13 @@ # yaml-language-server: $schema=../../../fern-versions-yml.schema.json +- version: 5.101.0 + changelogEntry: + - summary: | + Add a `layout.api-reference-layout` option to docs.yml. Set it to `cards` to render + nested API reference fields as bordered cards, or `indented` (the default) to render + each nested level as an indented branch with a connector line and a path breadcrumb. + type: feat + createdAt: "2026-08-21" + irVersion: 67 - version: 5.100.0 changelogEntry: - summary: | diff --git a/packages/cli/configuration-loader/src/docs-yml/parseDocsConfiguration.ts b/packages/cli/configuration-loader/src/docs-yml/parseDocsConfiguration.ts index af145c4c4d01..a317f710c0fb 100644 --- a/packages/cli/configuration-loader/src/docs-yml/parseDocsConfiguration.ts +++ b/packages/cli/configuration-loader/src/docs-yml/parseDocsConfiguration.ts @@ -676,6 +676,11 @@ function convertLayoutConfig( // fern-platform companion PR. Part of the `as unknown as` cast below // until the published FDR SDK adds `changelogLayout`. changelogLayout: layout.changelogLayout, + // Passed through as-is (no default): omitted renders the indented + // connector-line tree, "cards" renders the legacy nested cards. Resolved + // by the fern-platform companion PR. Part of the `as unknown as` cast + // below until the published FDR SDK adds `apiReferenceLayout`. + apiReferenceLayout: layout.apiReferenceLayout, // Opt-in (default off, resolved by the fern-platform companion PR): // when true the sidebar renders inline availability badges. Part of the // `as unknown as` cast below until the published FDR SDK adds the field. diff --git a/packages/cli/configuration/src/docs-yml/DocsYmlSchemas.ts b/packages/cli/configuration/src/docs-yml/DocsYmlSchemas.ts index 8ba13aa8e0ab..fcdbd73287bd 100644 --- a/packages/cli/configuration/src/docs-yml/DocsYmlSchemas.ts +++ b/packages/cli/configuration/src/docs-yml/DocsYmlSchemas.ts @@ -94,6 +94,8 @@ export const HeaderPosition = z.enum(["fixed", "static"]); export const ChangelogLayout = z.enum(["timeline", "classic"]); +export const ApiReferenceLayout = z.enum(["indented", "cards"]); + export const ProductSwitcherThemeConfig = z.enum(["default", "toggle", "tabs"]); export const LanguageSwitcherThemeConfig = z.enum(["default", "minimal"]); @@ -308,6 +310,7 @@ export const LayoutConfig = z.object({ "hide-feedback": z.boolean().optional(), "mobile-toc": z.boolean().optional(), "changelog-layout": ChangelogLayout.optional(), + "api-reference-layout": ApiReferenceLayout.optional(), "show-nav-availability-badges": z.boolean().optional() }); diff --git a/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/ApiReferenceLayout.ts b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/ApiReferenceLayout.ts new file mode 100644 index 000000000000..dba82aa5ccce --- /dev/null +++ b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/ApiReferenceLayout.ts @@ -0,0 +1,7 @@ +// This file was auto-generated by Fern from our API Definition. + +export const ApiReferenceLayout = { + Indented: "indented", + Cards: "cards", +} as const; +export type ApiReferenceLayout = (typeof ApiReferenceLayout)[keyof typeof ApiReferenceLayout]; diff --git a/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/LayoutConfig.ts b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/LayoutConfig.ts index c3c75407aa50..4b7cf61d81e3 100644 --- a/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/LayoutConfig.ts +++ b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/LayoutConfig.ts @@ -105,6 +105,15 @@ export interface LayoutConfig { * - `classic` renders the legacy stacked-entries layout with a per-page table of contents. */ changelogLayout?: FernDocsConfig.ChangelogLayout; + /** + * Sets how nested object fields are rendered in the API reference. + * + * @default: `indented` + * + * - `indented` renders each nested level as an indented branch with a connector line and a color-coded path breadcrumb. + * - `cards` renders each nested level as a bordered card. + */ + apiReferenceLayout?: FernDocsConfig.ApiReferenceLayout; /** * If `show-nav-availability-badges` is set to true, availability badges (Beta, Deprecated, etc.) * are rendered inline next to navigation items in the sidebar. Defaults to false. The page-header diff --git a/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/index.ts b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/index.ts index 84d60bfdde9a..60f61364323a 100644 --- a/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/index.ts +++ b/packages/cli/configuration/src/docs-yml/schemas/sdk/api/resources/docs/types/index.ts @@ -10,6 +10,7 @@ export * from "./AnalyticsConfig.js"; export * from "./AnnouncementConfig.js"; export * from "./ApiReferenceConfiguration.js"; export * from "./ApiReferenceEndpointConfiguration.js"; +export * from "./ApiReferenceLayout.js"; export * from "./ApiReferenceLayoutItem.js"; export * from "./ApiReferenceOperationConfiguration.js"; export * from "./ApiReferencePackageConfiguration.js"; diff --git a/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/ApiReferenceLayout.ts b/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/ApiReferenceLayout.ts new file mode 100644 index 000000000000..3b2897327651 --- /dev/null +++ b/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/ApiReferenceLayout.ts @@ -0,0 +1,14 @@ +// This file was auto-generated by Fern from our API Definition. + +import type * as FernDocsConfig from "../../../../api/index.js"; +import * as core from "../../../../core/index.js"; +import type * as serializers from "../../../index.js"; + +export const ApiReferenceLayout: core.serialization.Schema< + serializers.ApiReferenceLayout.Raw, + FernDocsConfig.ApiReferenceLayout +> = core.serialization.enum_(["indented", "cards"]); + +export declare namespace ApiReferenceLayout { + export type Raw = "indented" | "cards"; +} diff --git a/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/LayoutConfig.ts b/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/LayoutConfig.ts index e948a9cc74c4..9ad9d87745e8 100644 --- a/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/LayoutConfig.ts +++ b/packages/cli/configuration/src/docs-yml/schemas/sdk/serialization/resources/docs/types/LayoutConfig.ts @@ -3,6 +3,7 @@ import type * as FernDocsConfig from "../../../../api/index.js"; import * as core from "../../../../core/index.js"; import type * as serializers from "../../../index.js"; +import { ApiReferenceLayout } from "./ApiReferenceLayout.js"; import { ChangelogLayout } from "./ChangelogLayout.js"; import { ContentAlignment } from "./ContentAlignment.js"; import { HeaderPosition } from "./HeaderPosition.js"; @@ -26,6 +27,7 @@ export const LayoutConfig: core.serialization.ObjectSchema