From ebd905aedea181518b654c741b6910518743257d Mon Sep 17 00:00:00 2001 From: "blu@blu.cx" <38222767+thebrandonlucas@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:53:18 -0500 Subject: [PATCH] feat(cli): name the next command in scope, credential, and login errors Errors for a missing --org, --env, --wallet, or webhook ID now name the command that lists those IDs and every way to supply one. The organization and environment listings are marked browser login only, and the wallet hint says VOLTAGE_WALLET_ID is read only without --profile. An unknown or ambiguous credential or profile lists the saved names. organizations list and environments list with an API key say which key was used and how to select a browser login. An expired or revoked login (OAuth invalid_grant) says so, at refresh and at the organization token exchange, and gives the voltage logout --local and voltage login commands that replace it. The commands quote the account name for the shell and repeat --auth-url when the login is not on the default auth service, since that flag is never saved. API rejections carry a next step, also when the body is not JSON. HTTP 401, and the HTTP 400 missing_credentials the API returns for a key it does not recognize, point to voltage auth status; for a login the hint also gives the commands that sign in again. HTTP 403 points to voltage context. For a checkout session or stream token, both point to the token in --token-file, VOLTAGE_CHECKOUT_TOKEN, or VOLTAGE_STREAM_TOKEN. quote_required points to voltage quotes create and notes that a wallet on a BTC line of credit needs no quote. Hints can now be built at runtime and are scrubbed like messages, since they can name saved credentials. A 4xx rejection's message leads with the API's own reason, such as "Quote is required for payment", instead of only the HTTP status. A 5xx message gives only the status, and its text stays in the details. The presented credentials are redacted from the message as well as the details, so an API reason that echoes a key cannot print it. Human output shows the rest of the API error as indented field/value lines rather than a JSON line, including fields beside the error such as a request ID, and keeps the reason when the message does not carry it. --json still reports the complete response. The payments guide shows the quote-then-receive flow a USD wallet needs. --- docs/guides/payments.md | 9 +- docs/reference/exit-codes.md | 2 + docs/reference/output.md | 2 +- src/api.rs | 184 +++++++++++--- src/auth.rs | 117 +++++++-- src/config.rs | 66 ++++-- src/error.rs | 14 +- src/input.rs | 13 +- src/output.rs | 116 ++++++++- src/output/human.rs | 8 + src/startup.rs | 6 +- tests/cli_integration.rs | 447 ++++++++++++++++++++++++++++++++++- 12 files changed, 895 insertions(+), 89 deletions(-) diff --git a/docs/guides/payments.md b/docs/guides/payments.md index f09189c..84e0346 100644 --- a/docs/guides/payments.md +++ b/docs/guides/payments.md @@ -29,13 +29,20 @@ For a BOLT11 receive, `--qr` prints the invoice as a terminal QR code and `--cop ## Quotes and lines of credit -Quotes apply to USD lines of credit: a USD payment requires `--quote` with a quote for that line of credit. +A wallet on a USD line of credit needs a quote for every payment; without one, the API rejects the payment with `quote_required`. `voltage wallets get WALLET_ID` shows the wallet's `line_of_credit_id`, network, and balance currency. + +To receive into a USD wallet, quote the USD amount to BTC, wait until `quotes get` shows a `quote` with no `error`, then receive the same USD amount with that quote: ```sh voltage quotes create --profile prod --credit-line CREDIT_LINE_ID \ --network mutinynet --amount 10 --unit usd --to btc --execute +voltage quotes get QUOTE_ID --profile prod +voltage payments receive --profile prod --wallet WALLET_ID \ + --currency usd --kind bolt11 --amount 10 --unit usd --quote QUOTE_ID --wait ready --execute ``` +A quote is single-use and expires; a quoted receive's expiration is capped by the quote's remaining life. + On-chain and BIP21 payments and treasury movements are features Voltage enables per organization; the API rejects them with `feature_flag_disabled` until then, and the CLI says so. ## Prices diff --git a/docs/reference/exit-codes.md b/docs/reference/exit-codes.md index d47ecaa..1414ec2 100644 --- a/docs/reference/exit-codes.md +++ b/docs/reference/exit-codes.md @@ -12,3 +12,5 @@ | 130 | Interrupted | After exit code 4 for a payment, check its ID before you retry; see [payment IDs](../concepts/safety-model.md#payment-ids-and-the-request-journal). After exit code 5, keep watching with `voltage payments get PAYMENT_ID --profile prod --wait completed`. + +Many errors for exit codes 2 and 3 carry a `hint:` (or `error.hint` with `--json`) that names the next command: the discovery command for a missing `--org`, `--env`, or `--wallet`, the saved credential and profile names for an unknown or ambiguous selection, `voltage logout --local --account NAME` and then `voltage login --account NAME` for an expired or revoked login, `voltage auth status` for a rejected account credential (HTTP 401), and `voltage context` for an account credential that cannot reach the selected scope (HTTP 403). A rejected checkout token points to `--token-file` or the variable that supplies it, never to the account credential. A payment rejected with `quote_required` points to `voltage quotes create`. diff --git a/docs/reference/output.md b/docs/reference/output.md index ab97dcc..6f520ee 100644 --- a/docs/reference/output.md +++ b/docs/reference/output.md @@ -27,6 +27,6 @@ API fields stay inside `data`. With `--all`, JSON output collects the pages into Diagnostics, prompts, and progress go to stderr. Interactive stderr shows immediate, cancellable status for network requests and waits; redirected stderr gets no spinner/control codes. `-q, --quiet` suppresses optional status and notices, never requested results, errors, warnings, or recovery IDs. -Human errors start with `error:` and may include a safe `hint:`. `--json` makes command-line parse errors machine-readable as well as runtime errors. A downstream consumer that closes stdout early is treated as successful pipeline completion. +Human errors start with `error:`, which leads with the API's reason when it rejects a request with a client error (HTTP 4xx), and may include a safe `hint:` and an indented `details:` block of `field value` lines. `--json` reports the complete API response in `error.detail`, and makes command-line parse errors machine-readable as well as runtime errors. A downstream consumer that closes stdout early is treated as successful pipeline completion. Related: [scripting](../guides/scripting.md), [safety model](../concepts/safety-model.md#secrets-and-redaction). diff --git a/src/api.rs b/src/api.rs index ab8de38..9bf3d14 100644 --- a/src/api.rs +++ b/src/api.rs @@ -95,6 +95,29 @@ impl Authorization { Self::None => Vec::new(), } } + + /// The next step when the API rejects this credential (401) or what it can reach (403). + /// Checkout tokens are never the account credential, so they name their own source. + fn rejection_hint(&self, status: u16) -> Option<&'static str> { + match (self, status) { + (Self::Account(ApiCredential::ApiKey(_)), 401) => { + Some("Check the key with `voltage auth status`; the API does not accept it.") + } + (Self::Account(ApiCredential::OrganizationToken(_)), 401) => Some( + "Check the credential with `voltage auth status`. To sign in again, run `voltage logout --local --account NAME`, then `voltage login --account NAME`.", + ), + (Self::Account(_), 403) => Some( + "Check the organization, environment, and credential in use with `voltage context`.", + ), + (Self::CheckoutSession(_), 401 | 403) => Some( + "Check that --token-file or VOLTAGE_CHECKOUT_TOKEN holds this checkout session's token; checkout never uses the account credential.", + ), + (Self::CheckoutStream(_), 401 | 403) => Some( + "Check that --token-file or VOLTAGE_STREAM_TOKEN holds this checkout stream's token; checkout never uses the account credential.", + ), + _ => None, + } + } } /// A checkout credential from `--token-file`, else from `variable`. @@ -222,19 +245,35 @@ impl Api { } else { ErrorKind::for_http_status(status) }; - Error::new(kind, format!("HTTP {status}: non-JSON response omitted")) + let error = Error::new(kind, format!("HTTP {status}: non-JSON response omitted")); + match self.authorization.rejection_hint(status) { + Some(hint) => error.with_hint(hint), + None => error, + } })? }; if !(200..300).contains(&status) { - return Err(Error::new( + let rejection = ApiErrorView::deserialize(&body).unwrap_or_default().error; + let mut error = Error::new( ErrorKind::for_http_status(status), - rejection_message(status, &body), - ) - .with_detail(ErrorDetail::Http { - http_status: status, - data: body, - }) - .redacted(&self.secrets())); + rejection.message(status), + ); + // A credential the API does not recognize is a rejected credential, whatever + // the status, so it gets the same next step as a 401. + let hint = if rejection.kind == ApiErrorType::MissingCredentials { + self.authorization.rejection_hint(401) + } else { + self.authorization.rejection_hint(status) + }; + if let Some(hint) = hint.or_else(|| rejection.hint()) { + error = error.with_hint(hint); + } + return Err(error + .with_detail(ErrorDetail::Http { + http_status: status, + data: body, + }) + .redacted(&self.secrets())); } Ok(Response { status, @@ -311,6 +350,9 @@ pub(crate) enum BodyFailure { #[serde(rename_all = "snake_case")] enum ApiErrorType { FeatureFlagDisabled, + /// Staging and production answer a key they do not recognize this way, not with 401. + MissingCredentials, + QuoteRequired, #[default] #[serde(other)] Other, @@ -330,19 +372,36 @@ struct ApiErrorView { error: ApiErrorBody, } -/// The message for a rejected request; a disabled organization feature names the fix. -fn rejection_message(status: u16, body: &Value) -> String { - let rejection = ApiErrorView::deserialize(body).unwrap_or_default(); - match rejection.error.kind { - ApiErrorType::FeatureFlagDisabled => format!( - "Voltage API returned HTTP {status}: {}. Ask Voltage to enable this feature for the organization.", - rejection - .error - .detail - .as_deref() - .unwrap_or("this feature is not enabled for the organization") - ), - ApiErrorType::Other => format!("Voltage API returned HTTP {status}"), +impl ApiErrorBody { + /// The message for a rejected request: the API's own reason for a client error (4xx), + /// and for a disabled organization feature, the fix. A server error's text is not a + /// reason to show; it stays in the detail. + fn message(&self, status: u16) -> String { + let reason = self + .detail + .as_deref() + .filter(|_| (400..500).contains(&status)); + match (&self.kind, reason) { + (ApiErrorType::FeatureFlagDisabled, reason) => format!( + "Voltage API returned HTTP {status}: {}. Ask Voltage to enable this feature for the organization.", + reason.unwrap_or("this feature is not enabled for the organization") + ), + (_, Some(reason)) => format!("Voltage API returned HTTP {status}: {reason}"), + (_, None) => format!("Voltage API returned HTTP {status}"), + } + } + + /// The next step an error type names. The CLI has not read the wallet, so a missing + /// quote describes the wallets that need one rather than this wallet. + fn hint(&self) -> Option<&'static str> { + match self.kind { + ApiErrorType::QuoteRequired => Some( + "A wallet on a USD line of credit needs a quote for each payment. Create one with `voltage quotes create`, wait until `voltage quotes get QUOTE_ID` shows it, then pass --quote QUOTE_ID. A wallet on a BTC line of credit needs no quote.", + ), + ApiErrorType::FeatureFlagDisabled + | ApiErrorType::MissingCredentials + | ApiErrorType::Other => None, + } } } @@ -1407,27 +1466,94 @@ mod tests { } #[test] - fn disabled_features_are_explained_and_other_rejections_stay_generic() { - let disabled = json!({"error": { + fn credential_rejections_name_the_next_step_for_their_own_credential() { + let account = Authorization::Account(ApiCredential::ApiKey(Secret::new("key".into()))); + let hint = |authorization: &Authorization, status| authorization.rejection_hint(status); + assert!(hint(&account, 401).unwrap().contains("voltage auth status")); + // Signing in again cannot fix a rejected key, so only a login names `voltage login`. + assert!(!hint(&account, 401).unwrap().contains("voltage login")); + let login = + Authorization::Account(ApiCredential::OrganizationToken(Secret::new("t".into()))); + let renew = hint(&login, 401).unwrap(); + assert!( + renew.contains("voltage logout --local --account NAME") + && renew.contains("voltage login --account NAME"), + "{renew}" + ); + assert!(hint(&account, 403).unwrap().contains("voltage context")); + assert_eq!(hint(&account, 404), None); + assert_eq!(hint(&account, 500), None); + let session = Authorization::CheckoutSession(Secret::new("session".into())); + for status in [401, 403] { + let next = hint(&session, status).unwrap(); + assert!(next.contains("VOLTAGE_CHECKOUT_TOKEN"), "{next}"); + assert!(!next.contains("voltage auth status"), "{next}"); + } + let stream = Authorization::CheckoutStream(Secret::new("stream".into())); + assert!(hint(&stream, 401).unwrap().contains("VOLTAGE_STREAM_TOKEN")); + assert_eq!(hint(&session, 404), None); + assert_eq!(hint(&Authorization::None, 401), None); + } + + /// The parsed `error` object of a rejection body. + fn rejection(body: Value) -> ApiErrorBody { + ApiErrorView::deserialize(body).unwrap_or_default().error + } + + #[test] + fn rejections_lead_with_the_api_reason_and_explain_known_types() { + let disabled = rejection(json!({"error": { "type": "feature_flag_disabled", "code": "disabled", "detail": "OnChain payments are not enabled for this organization", "context": {"feature": "on_chain"} - }}); + }})); assert_eq!( - rejection_message(400, &disabled), + disabled.message(400), "Voltage API returned HTTP 400: OnChain payments are not enabled for this organization. Ask Voltage to enable this feature for the organization." ); - let other = json!({"error": {"type": "invalid_amount", "detail": "too small"}}); + let other = rejection(json!({"error": {"type": "invalid_amount", "detail": "too small"}})); assert_eq!( - rejection_message(400, &other), + other.message(400), + "Voltage API returned HTTP 400: too small" + ); + assert_eq!(other.hint(), None); + let quote = rejection(json!({"error": { + "type": "quote_required", "code": "required", + "detail": "Quote is required for payment", + "context": {"field": "quote_id", "required": true} + }})); + assert_eq!( + quote.message(400), + "Voltage API returned HTTP 400: Quote is required for payment" + ); + let hint = quote.hint().unwrap(); + assert!(hint.starts_with("A wallet on a USD line of credit needs a quote")); + assert!(hint.contains("voltage quotes create")); + assert!(hint.contains("BTC line of credit needs no quote")); + assert_eq!( + rejection(json!({"error": {"type": "invalid_amount"}})).message(400), "Voltage API returned HTTP 400" ); assert_eq!( - rejection_message(502, &Value::Null), + rejection(Value::Null).message(502), "Voltage API returned HTTP 502" ); } + #[test] + fn server_errors_keep_their_text_out_of_the_message() { + let internal = + rejection(json!({"error": {"type": "internal", "detail": "db pool exhausted"}})); + assert_eq!(internal.message(500), "Voltage API returned HTTP 500"); + let disabled = rejection(json!({"error": { + "type": "feature_flag_disabled", "detail": "internal text" + }})); + assert_eq!( + disabled.message(503), + "Voltage API returned HTTP 503: this feature is not enabled for the organization. Ask Voltage to enable this feature for the organization." + ); + } + #[test] fn retry_hints_prefer_milliseconds_then_seconds_then_dates() { let mut headers = HeaderMap::new(); diff --git a/src/auth.rs b/src/auth.rs index 98275a8..0162150 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -114,17 +114,32 @@ impl AuthResponse { (200..300).contains(&self.status) } - /// Fail with the server's error code when it is a plain identifier, never its free text. - fn require_success(&self) -> Result<()> { - if self.is_success() { - return Ok(()); - } - let error = serde_json::from_slice::(&self.body) + /// The server's OAuth error code when it is a plain identifier, never its free text. + fn error_code(&self) -> Option { + serde_json::from_slice::(&self.body) .ok() .and_then(|body| body.error) .filter(|code| { code.len() < 80 && code.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') }) + } + + /// `require_success` for a request made with a saved login: OAuth reports an expired, + /// revoked, or already spent grant as `invalid_grant`, so the error names the fix. + fn require_login(&self, account: &str, url: &str) -> Result<()> { + if !self.is_success() && self.error_code().as_deref() == Some("invalid_grant") { + return Err(expired_login(account, url)); + } + self.require_success() + } + + /// Fail with the server's error code when it is a plain identifier, never its free text. + fn require_success(&self) -> Result<()> { + if self.is_success() { + return Ok(()); + } + let error = self + .error_code() .unwrap_or_else(|| "authentication_failed".into()); Err(Error::auth(format!( "Authentication failed (HTTP {}, {error})", @@ -610,11 +625,12 @@ pub async fn resolve( flags: &GlobalFlags, submission: &SubmissionState, ) -> Result { - let (name, account) = - match select(settings, scope, flags).map_err(|rejected| rejected.problem.error(scope))? { - Selected::Ambient(key) => return Ok(Credential::ApiKey(key)), - Selected::Saved { name, account } => (name, account), - }; + let (name, account) = match select(settings, scope, flags) + .map_err(|rejected| rejected.problem.error(scope, settings))? + { + Selected::Ambient(key) => return Ok(Credential::ApiKey(key)), + Selected::Saved { name, account } => (name, account), + }; match account.kind { AccountKind::ApiKey => settings.read_credential(&name), AccountKind::User => { @@ -640,7 +656,7 @@ pub async fn resolve( submission.session_changing(SessionChange::Refresh { account: name.clone(), }); - let refreshed = Credential::Login(refresh(&url, login, flags.terminal()).await?); + let refreshed = Credential::Login(refresh(&url, login, &name, flags.terminal()).await?); settings.write_credential(&name, account.store, &refreshed)?; submission.session_settled(); Ok(refreshed) @@ -657,7 +673,7 @@ fn saved_login(settings: &Settings, name: &str) -> Result { /// Rotate an expiring login; the refresh token is single use, so the result is saved /// before any caller sees it. -async fn refresh(url: &str, login: Login, terminal: Terminal) -> Result { +async fn refresh(url: &str, login: Login, account: &str, terminal: Terminal) -> Result { let response = auth_request( client(AUTH_TIMEOUT)? .post(format!("{url}/oauth/token")) @@ -669,11 +685,42 @@ async fn refresh(url: &str, login: Login, terminal: Terminal) -> Result { terminal, ) .await?; - response.require_success()?; + response.require_login(account, url)?; let token: TokenResponse = response.json("Invalid token response")?; token.into_login(login.user_id, login.email) } +/// A saved login the auth service no longer accepts, with the commands that replace it. +fn expired_login(account: &str, url: &str) -> Error { + let account = shell_word(account); + // `--auth-url` is never saved, so a login against another auth service names it again. + let auth_url = if base_url(AUTH_URL).is_ok_and(|default| default == url) { + String::new() + } else { + format!(" --auth-url {}", shell_word(url)) + }; + Error::auth(format!( + "The saved login for {account} has expired or was revoked" + )) + // `login` refuses to replace a saved credential, so the stale one goes first. + .with_hint(format!( + "Remove it with `voltage logout --local --account {account}`, then sign in again with `voltage login --account {account}{auth_url}`." + )) +} + +/// `value` as one word a POSIX shell reads back unchanged, for a hint's copyable command. +fn shell_word(value: &str) -> String { + let plain = !value.is_empty() + && value + .chars() + .all(|c| c.is_ascii_alphanumeric() || "@%+,-./:=_".contains(c)); + if plain { + value.to_owned() + } else { + format!("'{}'", value.replace('\'', r"'\''")) + } +} + /// The credential for organization API calls: the API key as is, or the login exchanged /// for a token scoped to `--org`. pub async fn resolve_organization( @@ -715,7 +762,8 @@ pub async fn exchange( flags.terminal(), ) .await?; - response.require_success()?; + // A session revoked before its access token expires fails here, not at refresh. + response.require_login(&name, &url)?; let token: OrganizationTokenResponse = response.json("Invalid organization token response")?; if token.access_token.expose().is_empty() || token.issued_token_type != ACCESS_TOKEN_TYPE @@ -827,7 +875,26 @@ pub async fn discover( what: Discovery, submission: &SubmissionState, ) -> Result { - let requires_login = || Error::auth("Organization/environment discovery requires a user login"); + // An API key cannot list organizations or environments; say which key was used. + let requires_login = || { + let command = match what { + Discovery::Organizations => "organizations list", + Discovery::Environments => "environments list", + }; + if ambient_api_key(flags).is_some() { + return Error::auth(format!( + "{command} needs a browser login, but this command used VOLTAGE_API_KEY from the environment" + )) + .with_hint( + "Pass --account NAME to use a saved browser login (`voltage login` creates one), or unset VOLTAGE_API_KEY.", + ); + } + let used = settings.account_name(scope).unwrap_or_default(); + Error::auth(format!( + "{command} needs a browser login, but the selected credential {used} is an API key" + )) + .with_hint("Select a browser login with --account NAME, or sign in with `voltage login`.") + }; let token = match what { Discovery::Organizations => match resolve(settings, scope, flags, submission).await? { Credential::Login(login) => login.access_token, @@ -966,6 +1033,24 @@ mod tests { assert_eq!(body.error, Some(DeviceGrantError::Other)); } + #[test] + fn expired_logins_name_commands_that_can_be_pasted() { + let hint = |account, url| expired_login(account, url).hint.unwrap(); + let default = base_url(AUTH_URL).unwrap(); + assert_eq!( + hint("me@example.com", &default), + "Remove it with `voltage logout --local --account me@example.com`, then sign in again with `voltage login --account me@example.com`." + ); + // `--auth-url` is never saved, so another auth service is named again. + assert!(hint("me", "https://auth.staging.example.test").ends_with( + "`voltage login --account me --auth-url https://auth.staging.example.test`." + )); + assert!(hint("my team", &default).contains("--account 'my team'`")); + assert_eq!(shell_word("it's"), r"'it'\''s'"); + assert_eq!(shell_word(""), "''"); + assert_eq!(shell_word("$(id)"), "'$(id)'"); + } + #[test] fn failed_responses_report_only_identifier_error_codes() { let response = AuthResponse { diff --git a/src/config.rs b/src/config.rs index 73a8b66..63c5469 100644 --- a/src/config.rs +++ b/src/config.rs @@ -110,20 +110,35 @@ pub(crate) enum CredentialProblem { } impl CredentialProblem { - pub(crate) fn error(self, scope: &Scope) -> Error { + pub(crate) fn error(self, scope: &Scope, settings: &Settings) -> Error { + let saved = joined(settings.config.accounts.keys()); match self { - Self::EmptyApiKey => Error::auth("VOLTAGE_API_KEY is empty"), - Self::NoCredential => Error::auth("No credential is available") - .with_hint("Run voltage login, select --account, or provide VOLTAGE_API_KEY."), - Self::MultipleCredentials => Error::usage("Multiple credentials are saved") - .with_hint("Select one with --account NAME or --profile NAME."), + Self::EmptyApiKey => Error::auth("VOLTAGE_API_KEY is empty") + .with_hint("Unset VOLTAGE_API_KEY to use a saved credential, or set it to a key."), + Self::NoCredential => Error::auth("No credential is available").with_hint( + "Sign in with `voltage login`, select one with --account NAME, or provide VOLTAGE_API_KEY.", + ), + Self::MultipleCredentials => Error::usage("Multiple credentials are saved").with_hint( + format!( + "Saved credentials: {}. Select one with --account NAME or --profile NAME.", + saved.unwrap_or_default() + ), + ), Self::UnknownCredential => Error::auth(format!( "Unknown credential {}", scope.account.as_deref().unwrap_or_default() - )), + )) + .with_hint(match saved { + Some(saved) => format!("Saved credentials: {saved}."), + None => "No credentials are saved; sign in with `voltage login` or import a key with `voltage auth import-key`.".into(), + }), Self::BoundElsewhere => Error::usage( "API key is bound to a different organization or environment; select a matching credential", - ), + ) + .with_hint(format!( + "Saved credentials: {}. Select a key for this scope or a browser login with --account NAME.", + saved.unwrap_or_default() + )), } } } @@ -233,12 +248,17 @@ pub struct Scope { pub account: Option, } +/// How to find and supply each scope value a command can require. +pub(crate) const ORGANIZATION_HINT: &str = "Find your organization ID with `voltage organizations list` (browser login only), then pass --org ID, set VOLTAGE_ORGANIZATION_ID, or save it in a profile with `voltage profiles create`."; +pub(crate) const ENVIRONMENT_HINT: &str = "List environments with `voltage environments list --org ORG_ID` (browser login only), then pass one --env ID, set VOLTAGE_ENVIRONMENT_ID, or save it in a profile with `voltage profiles create`."; +pub(crate) const WALLET_HINT: &str = "List wallets with `voltage wallets list`, or create one with `voltage wallets create`, then pass --wallet ID. VOLTAGE_WALLET_ID is read only when no --profile is selected."; +pub(crate) const WEBHOOK_HINT: &str = + "List webhooks with `voltage webhooks list`, then pass --webhook ID."; + impl Scope { pub fn require_org(&self) -> Result { - self.org.ok_or_else(|| { - Error::usage("--org is required") - .with_hint("Pass --org UUID or select a profile with --profile NAME.") - }) + self.org + .ok_or_else(|| Error::usage("--org is required").with_hint(ORGANIZATION_HINT)) } /// Operations bound to one environment refuse an ambiguous list. @@ -247,7 +267,7 @@ impl Scope { [env] => Ok(*env), _ => Err( Error::usage("Exactly one --env is required for this operation") - .with_hint("Pass one --env UUID or select a profile with --profile NAME."), + .with_hint(ENVIRONMENT_HINT), ), } } @@ -363,7 +383,7 @@ impl Settings { self.config .profiles .get(name) - .ok_or_else(|| Error::usage(format!("Unknown profile {name}"))) + .ok_or_else(|| self.unknown_profile(name)) }) .transpose()?; let ambient = |name: &str| -> Result<(Option, Source)> { @@ -430,7 +450,7 @@ impl Settings { /// The saved credential a command uses: the selected one, or the only one. pub fn account_name(&self, scope: &Scope) -> Result { self.select_account(scope) - .map_err(|problem| problem.error(scope)) + .map_err(|problem| problem.error(scope, self)) } /// The saved credential a scope selects: the named one, or the only one saved. @@ -452,6 +472,16 @@ impl Settings { } } + /// An unknown profile name, with the saved names so the next attempt can pick one. + pub(crate) fn unknown_profile(&self, name: &str) -> Error { + Error::usage(format!("Unknown profile {name}")).with_hint( + match joined(self.config.profiles.keys()) { + Some(saved) => format!("Saved profiles: {saved}."), + None => "No profiles are saved; create one with `voltage profiles create NAME --org ID --env ID`.".into(), + }, + ) + } + pub fn account(&self, name: &str) -> Result<&Account> { self.config .accounts @@ -594,6 +624,12 @@ fn sourced(value: Option, source: Source) -> (Option, Source) { (value, source) } +/// Saved names, comma-separated, or `None` when there are none. +fn joined<'a>(names: impl Iterator) -> Option { + let names: Vec<&str> = names.map(String::as_str).collect(); + (!names.is_empty()).then(|| names.join(", ")) +} + fn read_config(dir: &Path) -> Result { let path = dir.join(CONFIG_FILE); if !path.exists() { diff --git a/src/error.rs b/src/error.rs index c68dd99..61abbce 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,8 +1,9 @@ //! Failure categories, their documented exit codes, and the typed detail reported with them. -use crate::output::redact; +use crate::output::{redact, redact_text}; use serde::Serialize; use serde_json::Value; +use std::borrow::Cow; use uuid::Uuid; pub type Result = std::result::Result; @@ -110,7 +111,8 @@ impl ErrorDetail { pub struct Error { pub kind: ErrorKind, pub message: String, - pub hint: Option<&'static str>, + /// The next step to take, naming the command when there is one. + pub hint: Option>, pub detail: Option, pub(crate) stdout_closed: bool, } @@ -150,8 +152,8 @@ impl Error { Self::new(ErrorKind::Interrupted, message) } - pub fn with_hint(mut self, hint: &'static str) -> Self { - self.hint = Some(hint); + pub fn with_hint(mut self, hint: impl Into>) -> Self { + self.hint = Some(hint.into()); self } @@ -178,7 +180,9 @@ impl Error { self.stdout_closed } + /// Remove the presented credentials from the message and detail; both can quote the API. pub fn redacted(mut self, secrets: &[&str]) -> Self { + redact_text(&mut self.message, secrets); if let Some(detail) = &mut self.detail { detail.redact(secrets); } @@ -259,5 +263,7 @@ mod tests { redacted, json!({"http_status": 404, "data": {"api_key": "[REDACTED]"}}) ); + let echoed = Error::api("HTTP 400: Bad key key-1").redacted(&["key-1", ""]); + assert_eq!(echoed.message, "HTTP 400: Bad key [REDACTED]"); } } diff --git a/src/input.rs b/src/input.rs index 91c5bb1..5742cfb 100644 --- a/src/input.rs +++ b/src/input.rs @@ -10,7 +10,7 @@ use crate::{ ApiInvocation, BodySource, Filter, FriendlyFlags, PaymentFlags, QueryOverride, QuoteFlags, UpdateWalletFlags, UpdateWebhookFlags, WalletFlags, WebhookFlags, }, - config::{InputSource, Scope}, + config::{InputSource, ORGANIZATION_HINT, Scope, WALLET_HINT, WEBHOOK_HINT}, payment::{Amount, AmountUnit, Currency, Network, PaymentDirection, ReceiveKind}, registry::{Method, Operation, OperationId, Parameter, ParameterLocation, ScopeParameter}, terminal::read_detached, @@ -457,9 +457,9 @@ fn send_body(flags: &PaymentFlags, wallet: Uuid, currency: Currency) -> Result, scope: &Scope) -> Result { - let wallet = scope - .wallet - .ok_or_else(|| Error::usage("--wallet is required for payment creation"))?; + let wallet = scope.wallet.ok_or_else(|| { + Error::usage("--wallet is required for payment creation").with_hint(WALLET_HINT) + })?; let currency = require(flags.currency, "currency")?; if flags.unit.is_some() && flags.amount.is_none() { return Err(Error::usage("--unit requires --amount")); @@ -643,14 +643,17 @@ fn path(operation: &Operation, resource_id: Option, scope: &Scope) -> Resu match parameter.scope() { Some(ScopeParameter::Organization) => { Error::usage(format!("--org is required for {command}")) - .with_hint("Pass --org UUID or select a profile with --profile NAME.") + .with_hint(ORGANIZATION_HINT) } Some(ScopeParameter::Wallet) => { Error::usage(format!("--wallet is required for {command}")) + .with_hint(WALLET_HINT) } Some(ScopeParameter::Webhook) => { Error::usage(format!("--webhook is required for {command}")) + .with_hint(WEBHOOK_HINT) } + // `single_env` above already reports a missing or ambiguous --env. Some(ScopeParameter::Environment | ScopeParameter::Environments) | None => { Error::usage(format!("Missing {} for {command}", parameter.name)) } diff --git a/src/output.rs b/src/output.rs index 3c23dd2..db0b029 100644 --- a/src/output.rs +++ b/src/output.rs @@ -269,15 +269,18 @@ pub fn redact(value: &mut Value, secrets: &[&str]) { redact(value, secrets); } } - Value::String(text) => { - for secret in secrets.iter().filter(|secret| !secret.is_empty()) { - *text = text.replace(secret, REDACTED); - } - } + Value::String(text) => redact_text(text, secrets), Value::Null | Value::Bool(_) | Value::Number(_) => {} } } +/// Replace any presented credential text in free text, such as a message quoting the API. +pub fn redact_text(text: &mut String, secrets: &[&str]) { + for secret in secrets.iter().filter(|secret| !secret.is_empty()) { + *text = text.replace(secret, REDACTED); + } +} + #[derive(Serialize)] struct ErrorReport<'a> { error: ErrorBody<'a>, @@ -288,7 +291,7 @@ struct ErrorBody<'a> { message: &'a str, exit_code: i32, #[serde(skip_serializing_if = "Option::is_none")] - hint: Option<&'static str>, + hint: Option<&'a str>, detail: Option<&'a ErrorDetail>, } @@ -298,7 +301,7 @@ pub fn report_error(error: &Error, format: OutputFormat) { error: ErrorBody { message: &error.message, exit_code: error.kind.exit_code(), - hint: error.hint, + hint: error.hint.as_deref(), detail: error.detail.as_ref(), }, }; @@ -310,20 +313,111 @@ pub fn report_error(error: &Error, format: OutputFormat) { } else { // Messages can interpolate untrusted input; keep control characters off the terminal. eprintln!("error: {}", scrub_line(&error.message)); - if let Some(hint) = error.hint { - // Hints are static today; scrub defensively if they ever become dynamic. + if let Some(hint) = &error.hint { + // Hints can name saved credentials and profiles, so they are scrubbed like messages. eprintln!("hint: {}", scrub_line(hint)); } - if let Some(detail) = body["error"]["detail"].as_object() { - eprintln!("details: {}", scrub_line(&json!(detail).to_string())); + if let Some(detail) = human_detail(&body["error"]["detail"], &error.message) { + let mut lines = Vec::new(); + if human::detail(&mut lines, &detail).is_ok() { + eprintln!("details:"); + for line in String::from_utf8_lossy(&lines).lines() { + eprintln!(" {line}"); + } + } } } } +/// The part of an error's detail a person needs. A client error (4xx) shows the API's error +/// without the reason `message` already gives; `--json` keeps the whole response. +fn human_detail(detail: &Value, message: &str) -> Option { + let status = detail.get("http_status").and_then(Value::as_u64); + let detail = match (status, detail.get("data")) { + (Some(status), Some(data)) => match data.get("error") { + Some(Value::Object(error)) => { + let mut error = error.clone(); + if (400..500).contains(&status) + && error + .get("detail") + .and_then(Value::as_str) + .is_some_and(|reason| message.contains(reason)) + { + error.remove("detail"); + } + // Keep the response's other fields, such as a request ID, beside the error's. + for (key, value) in data.as_object().into_iter().flatten() { + if key != "error" { + error.entry(key.clone()).or_insert_with(|| value.clone()); + } + } + Value::Object(error) + } + _ => data.clone(), + }, + _ => detail.clone(), + }; + match &detail { + Value::Null => None, + Value::Object(map) if map.is_empty() => None, + _ => Some(detail), + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn human_details_drop_the_reason_the_message_already_gives() { + let message = "Voltage API returned HTTP 400: Quote is required for payment"; + let rejection = json!({"http_status": 400, "data": {"error": { + "type": "quote_required", "code": "required", + "detail": "Quote is required for payment", + "context": {"field": "quote_id", "required": true} + }}}); + assert_eq!( + human_detail(&rejection, message), + Some(json!({"type": "quote_required", "code": "required", + "context": {"field": "quote_id", "required": true}})) + ); + let reason_only = json!({"http_status": 400, "data": {"error": {"detail": "too small"}}}); + assert_eq!( + human_detail(&reason_only, "Voltage API returned HTTP 400: too small"), + None + ); + // A reason the message does not carry stays, so human output never loses it. + let untyped = json!({"http_status": 400, "data": {"error": { + "type": null, "detail": "Wallet is frozen" + }}}); + assert_eq!( + human_detail(&untyped, "Voltage API returned HTTP 400"), + Some(json!({"type": null, "detail": "Wallet is frozen"})) + ); + // Fields beside the error, such as a request ID, are kept. + let traced = json!({"http_status": 400, "data": { + "error": {"detail": "too small"}, "request_id": "req-1" + }}); + assert_eq!( + human_detail(&traced, "Voltage API returned HTTP 400: too small"), + Some(json!({"request_id": "req-1"})) + ); + let server = json!({"http_status": 500, "data": {"error": {"detail": "db down"}}}); + assert_eq!( + human_detail(&server, "Voltage API returned HTTP 500: db down"), + Some(json!({"detail": "db down"})) + ); + let unstructured = json!({"http_status": 502, "data": "bad gateway"}); + assert_eq!(human_detail(&unstructured, ""), Some(json!("bad gateway"))); + assert_eq!( + human_detail(&json!({"http_status": 502, "data": null}), ""), + None + ); + let pending = json!({"resource_id": "id", "outcome": "pending"}); + assert_eq!(human_detail(&pending, ""), Some(pending.clone())); + assert_eq!(human_detail(&Value::Null, ""), None); + } + #[test] fn secrets_are_redacted_recursively() { let mut value = json!({"data":{"shared_secret":"abc","description":"token abc"}}); diff --git a/src/output/human.rs b/src/output/human.rs index af81ed7..5484849 100644 --- a/src/output/human.rs +++ b/src/output/human.rs @@ -61,6 +61,14 @@ pub fn render(out: &mut impl Write, envelope: &Envelope, width: Option) - Ok(()) } +/// An error's detail as aligned `field value` lines, or one line when it is not an object. +pub fn detail(out: &mut impl Write, detail: &Value) -> Result<()> { + match detail { + Value::Object(map) => fields(out, map, false), + other => Ok(writeln!(out, "{}", cell(other))?), + } +} + /// A bare array, or a page's `items` or `entries`. fn list_items(data: &Value) -> Option<&[Value]> { data.as_array() diff --git a/src/startup.rs b/src/startup.rs index 689e36f..405f3b1 100644 --- a/src/startup.rs +++ b/src/startup.rs @@ -214,11 +214,11 @@ async fn profiles( ) -> Result { let _lock = settings.lock(terminal).await?; settings.reload()?; - let unknown = || Error::usage("Unknown profile"); match command { ProfileCommand::List => Envelope::local(&settings.config.profiles), ProfileCommand::Get { name } => { - Envelope::local(settings.config.profiles.get(&name).ok_or_else(unknown)?) + let profile = settings.config.profiles.get(&name); + Envelope::local(profile.ok_or_else(|| settings.unknown_profile(&name))?) } ProfileCommand::Create { name } => { if settings.config.profiles.contains_key(&name) { @@ -238,7 +238,7 @@ async fn profiles( } ProfileCommand::Delete { name } => { if settings.config.profiles.remove(&name).is_none() { - return Err(unknown()); + return Err(settings.unknown_profile(&name)); } settings.save()?; Envelope::local(ProfileDeleted { diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 5fbfa57..5563daf 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -121,7 +121,8 @@ fn human_runtime_errors_have_a_prefix_and_safe_hint() { error.starts_with("error: --org is required for wallets list\n"), "{error}" ); - assert!(error.contains("hint: Pass --org UUID or select a profile")); + // The hint names the command that finds the missing value. + assert!(error.contains("hint: Find your organization ID with `voltage organizations list`")); assert!(!error.contains("api_key")); } @@ -1585,7 +1586,6 @@ async fn interrupt_in_flight( } /// The JSON error report: the last line of stderr, after any recovery notice. -#[cfg(unix)] fn error_report(stderr: &[u8]) -> Value { serde_json::from_str(String::from_utf8_lossy(stderr).lines().last().unwrap()).unwrap() } @@ -2570,6 +2570,51 @@ async fn wait_tolerates_projection_delay_and_distinguishes_invoice_from_settleme // Pagination, output, and transport failures // --------------------------------------------------------------------------------------- +#[tokio::test(flavor = "multi_thread")] +async fn api_rejections_lead_with_the_reason_and_keep_the_response_for_json() { + let (server, dir) = fixture().await; + let rejection = json!({"error": { + "type": "quote_required", "code": "required", + "detail": "Quote is required for payment", + "context": {"field": "quote_id", "required": true} + }}); + route("POST", payments_path()) + .respond_with(ResponseTemplate::new(400).set_body_json(rejection.clone())) + .expect(2) + .mount(&server) + .await; + let mut human = human_process_at(dir.path(), &server.uri()); + human.args(receive_args(&[ + "--wallet", WALLET, "--amount", "1000", "--unit", "sats", "--output", "table", + ])); + let error = stderr(&Command::from_std(human).assert().code(1)); + let report = error + .split_once("error: ") + .map(|(_, report)| report) + .unwrap_or_default(); + assert_eq!( + report, + "Voltage API returned HTTP 400: Quote is required for payment\n\ + hint: A wallet on a USD line of credit needs a quote for each payment. Create one with \ + `voltage quotes create`, wait until `voltage quotes get QUOTE_ID` shows it, then pass \ + --quote QUOTE_ID. A wallet on a BTC line of credit needs no quote.\n\ + details:\n \ + code required\n \ + context.field quote_id\n \ + context.required true\n \ + type quote_required\n", + "{error}" + ); + + let mut machine = process_at(dir.path(), &server.uri()); + machine.args(receive_args(&[ + "--wallet", WALLET, "--amount", "1000", "--unit", "sats", + ])); + let output = Command::from_std(machine).assert().code(1); + let report = error_report(&output.get_output().stderr); + assert_eq!(report["error"]["detail"]["data"], rejection); +} + #[tokio::test(flavor = "multi_thread")] async fn human_output_fits_the_terminal_and_scrubs_api_text() { let (server, dir) = fixture().await; @@ -2825,6 +2870,393 @@ async fn auth_errors_redact_echoed_credentials() { assert!(!stderr(&result).contains(ACCOUNT_KEY)); } +#[tokio::test(flavor = "multi_thread")] +async fn api_reasons_that_echo_the_key_are_redacted_in_json_and_human_output() { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(400).set_body_json(json!({"error": { + "type": "invalid_request", "detail": format!("Bad key {ACCOUNT_KEY}") + }}))) + .mount(&server) + .await; + let machine = cli(dir.path(), &server) + .args(["wallets", "list", "--org", ORG]) + .assert() + .code(1); + let error = json_error(&machine); + assert_eq!( + error["error"]["message"], + "Voltage API returned HTTP 400: Bad key [REDACTED]" + ); + assert_eq!( + error["error"]["detail"]["data"]["error"]["detail"], + "Bad key [REDACTED]" + ); + assert!(!stderr(&machine).contains(ACCOUNT_KEY)); + + let mut human = human_process_at(dir.path(), &server.uri()); + human.args(["wallets", "list", "--org", ORG, "--output", "table"]); + let human = stderr(&Command::from_std(human).assert().code(1)); + assert!( + human.starts_with("error: Voltage API returned HTTP 400: Bad key [REDACTED]\n"), + "{human}" + ); + assert!(!human.contains(ACCOUNT_KEY), "{human}"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn server_error_text_stays_out_of_the_message_and_is_redacted() { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(500).set_body_json(json!({"error": { + "type": "internal", "detail": format!("query failed for {ACCOUNT_KEY}") + }}))) + .mount(&server) + .await; + let machine = cli(dir.path(), &server) + .args(["wallets", "list", "--org", ORG]) + .assert() + .failure(); + let error = json_error(&machine); + assert_eq!(error["error"]["message"], "Voltage API returned HTTP 500"); + assert_eq!( + error["error"]["detail"]["data"]["error"]["detail"], + "query failed for [REDACTED]" + ); + + let mut human = human_process_at(dir.path(), &server.uri()); + human.args(["wallets", "list", "--org", ORG, "--output", "table"]); + let human = stderr(&Command::from_std(human).assert().failure()); + assert!( + human.starts_with("error: Voltage API returned HTTP 500\n"), + "{human}" + ); + assert!(human.contains("query failed for [REDACTED]"), "{human}"); + assert!(!human.contains(ACCOUNT_KEY), "{human}"); +} + +/// The JSON error report a failed `--json` command writes as its last stderr line. +fn json_error(assertion: &Assert) -> Value { + error_report(&assertion.get_output().stderr) +} + +#[tokio::test(flavor = "multi_thread")] +async fn rejected_credentials_name_the_command_that_checks_them() { + for (status, next) in [(401, "voltage auth status"), (403, "voltage context")] { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(status)) + .mount(&server) + .await; + let result = cli(dir.path(), &server) + .args(["wallets", "list", "--org", ORG]) + .assert() + .code(3); + let error = json_error(&result); + assert!( + error["error"]["hint"].as_str().unwrap().contains(next), + "{error}" + ); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn a_revoked_login_names_the_command_that_renews_it() { + // An expiring login fails at refresh; a session revoked before its access token + // expires fails at the organization token exchange. Both name the same fix. + for expiring in [true, false] { + let (server, dir) = fixture().await; + seeded_login(dir.path(), &server, expiring); + route("POST", "/oauth/token") + .respond_with( + ResponseTemplate::new(400).set_body_json(json!({"error":"invalid_grant"})), + ) + .mount(&server) + .await; + let result = cli(dir.path(), &server) + .args(["wallets", "list", "--account", LOGIN_NAME, "--org", ORG]) + .assert() + .code(3); + let error = json_error(&result); + assert_eq!( + error["error"]["message"], + format!("The saved login for {LOGIN_NAME} has expired or was revoked") + ); + // The mock auth service is not the default one, so the hint names it again. + assert_eq!( + error["error"]["hint"], + format!( + "Remove it with `voltage logout --local --account {LOGIN_NAME}`, then sign in again with `voltage login --account {LOGIN_NAME} --auth-url {}`.", + server.uri() + ), + "expiring={expiring}" + ); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn an_unrecognized_key_names_the_command_that_checks_it() { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(400).set_body_json(json!({"error": { + "type": "missing_credentials", "detail": "Missing credential" + }}))) + .mount(&server) + .await; + let result = cli(dir.path(), &server) + .args(["wallets", "list", "--org", ORG]) + .assert() + .code(1); + let error = json_error(&result); + assert_eq!( + error["error"]["message"], + "Voltage API returned HTTP 400: Missing credential" + ); + assert_eq!( + error["error"]["hint"], + "Check the key with `voltage auth status`; the API does not accept it." + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn rejections_without_a_json_body_still_name_the_next_step() { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(401).set_body_string("denied")) + .mount(&server) + .await; + let result = cli(dir.path(), &server) + .args(["wallets", "list", "--org", ORG]) + .assert() + .code(3); + let error = json_error(&result); + assert_eq!( + error["error"]["message"], + "HTTP 401: non-JSON response omitted" + ); + assert!( + error["error"]["hint"] + .as_str() + .unwrap() + .contains("voltage auth status"), + "{error}" + ); +} + +#[tokio::test(flavor = "multi_thread")] +async fn human_errors_keep_a_reason_and_request_id_the_message_lacks() { + let (server, dir) = fixture().await; + route("GET", wallets_path(ORG)) + .respond_with(ResponseTemplate::new(400).set_body_json(json!({ + "error": {"type": null, "detail": "Wallet is frozen"}, + "request_id": "req-123" + }))) + .mount(&server) + .await; + let mut human = human_process_at(dir.path(), &server.uri()); + human.args(["wallets", "list", "--org", ORG, "--output", "table"]); + let shown = stderr(&Command::from_std(human).assert().code(1)); + // The untyped error leaves the reason out of the message, so the details keep it. + assert!( + shown.starts_with("error: Voltage API returned HTTP 400\n"), + "{shown}" + ); + assert!(shown.contains("Wallet is frozen"), "{shown}"); + assert!(shown.contains("req-123"), "{shown}"); +} + +#[tokio::test(flavor = "multi_thread")] +async fn credential_and_profile_errors_list_the_saved_names() { + let (server, dir) = fixture().await; + let mut settings = seeded_api_key(dir.path(), "first", "first-key"); + settings.config.accounts.insert( + "second".into(), + Account { + store: CredentialStore::File, + kind: AccountKind::ApiKey, + email: None, + organization_id: Some(uuid(ORG)), + environment_id: Some(uuid(ENV)), + auth_url: config::AUTH_URL.into(), + }, + ); + settings.save().unwrap(); + let hint = |args: &[&str], code: i32| { + let result = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(["wallets", "list", "--org", ORG]) + .args(args) + .assert() + .code(code); + json_error(&result)["error"]["hint"] + .as_str() + .unwrap() + .to_owned() + }; + assert!(hint(&[], 2).starts_with("Saved credentials: first, second.")); + assert_eq!( + hint(&["--account", "third"], 3), + "Saved credentials: first, second." + ); + assert!(hint(&["--profile", "missing"], 2).starts_with("No profiles are saved")); + // `first` is bound to ORG, so it cannot reach another organization. + let elsewhere = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(["wallets", "list", "--org", RESOURCE, "--account", "first"]) + .assert() + .code(2); + assert_eq!( + json_error(&elsewhere)["error"]["hint"], + "Saved credentials: first, second. Select a key for this scope or a browser login with --account NAME." + ); + + settings.config.profiles.insert( + "main".into(), + Profile { + organization_id: uuid(ORG), + environment_id: uuid(ENV), + account: "first".into(), + }, + ); + settings.save().unwrap(); + assert_eq!(hint(&["--profile", "missing"], 2), "Saved profiles: main."); + for verb in ["get", "delete"] { + let result = cli(dir.path(), &server) + .args(["profiles", verb, "missing"]) + .assert() + .code(2); + let error = json_error(&result); + assert_eq!(error["error"]["message"], "Unknown profile missing"); + assert_eq!(error["error"]["hint"], "Saved profiles: main."); + } + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn credential_errors_without_saved_credentials_name_how_to_add_one() { + let (server, dir) = fixture().await; + let unknown = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(["wallets", "list", "--org", ORG, "--account", "missing"]) + .assert() + .code(3); + assert_eq!( + json_error(&unknown)["error"]["hint"], + "No credentials are saved; sign in with `voltage login` or import a key with `voltage auth import-key`." + ); + let empty = cli(dir.path(), &server) + .env("VOLTAGE_API_KEY", " ") + .args(["wallets", "list", "--org", ORG]) + .assert() + .code(3); + assert_eq!( + json_error(&empty)["error"]["hint"], + "Unset VOLTAGE_API_KEY to use a saved credential, or set it to a key." + ); + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn missing_scope_errors_name_the_command_that_lists_it() { + let (server, dir) = fixture().await; + let error = |args: &[&str]| { + let result = cli(dir.path(), &server).args(args).assert().code(2); + json_error(&result)["error"].clone() + }; + let webhook = error(&["webhooks", "deliveries", "list", "--org", ORG, "--env", ENV]); + assert_eq!( + webhook["message"], + "--webhook is required for webhooks deliveries list" + ); + assert_eq!( + webhook["hint"], + "List webhooks with `voltage webhooks list`, then pass --webhook ID." + ); + for envs in [&[][..], &["--env", ENV, "--env", RESOURCE][..]] { + let mut args = vec!["payments", "get", RESOURCE, "--org", ORG]; + args.extend_from_slice(envs); + let environment = error(&args); + assert_eq!( + environment["message"], + "Exactly one --env is required for this operation" + ); + assert!( + environment["hint"] + .as_str() + .unwrap() + .starts_with("List environments with `voltage environments list --org ORG_ID`"), + "{environment}" + ); + } + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn discovery_with_an_api_key_says_which_key_was_used() { + let (server, dir) = fixture().await; + let result = cli(dir.path(), &server) + .args(["organizations", "list"]) + .assert() + .code(3); + let error = json_error(&result); + assert!( + error["error"]["message"] + .as_str() + .unwrap() + .contains("VOLTAGE_API_KEY from the environment"), + "{error}" + ); + + // A saved API key selected without the ambient one is named instead. + seeded_api_key(dir.path(), "saved", "saved-key"); + for (args, command) in [ + (&["organizations", "list"][..], "organizations list"), + ( + &["environments", "list", "--org", ORG][..], + "environments list", + ), + ] { + let result = cli(dir.path(), &server) + .env_remove("VOLTAGE_API_KEY") + .args(args) + .assert() + .code(3); + let error = json_error(&result); + assert_eq!( + error["error"]["message"], + format!( + "{command} needs a browser login, but the selected credential saved is an API key" + ) + ); + assert_eq!( + error["error"]["hint"], + "Select a browser login with --account NAME, or sign in with `voltage login`." + ); + } + assert!(server.received_requests().await.unwrap().is_empty()); +} + +#[tokio::test(flavor = "multi_thread")] +async fn rejected_checkout_tokens_point_to_the_checkout_token_not_the_account() { + let (server, dir) = fixture().await; + Mock::given(method("GET")) + .respond_with(ResponseTemplate::new(401)) + .mount(&server) + .await; + let result = cli(dir.path(), &server) + .env("VOLTAGE_CHECKOUT_TOKEN", CHECKOUT_KEY) + .args(["checkout", "sessions", "get", RESOURCE]) + .assert() + .code(3); + let hint = json_error(&result)["error"]["hint"] + .as_str() + .unwrap() + .to_owned(); + assert!(hint.contains("VOLTAGE_CHECKOUT_TOKEN"), "{hint}"); + assert!(!hint.contains("voltage auth status"), "{hint}"); + assert!(!hint.contains("voltage login"), "{hint}"); +} + #[tokio::test(flavor = "multi_thread")] async fn failed_wallet_reads_do_not_report_uncertain_writes() { use std::io::{Read, Write}; @@ -3230,7 +3662,11 @@ async fn an_api_key_without_an_organization_keeps_the_original_error() { "{}", run.shown ); - assert!(run.shown.contains("--profile NAME"), "{}", run.shown); + assert!( + run.shown.contains("voltage organizations list"), + "{}", + run.shown + ); // A payment needs a wallet, listed within an organization the key cannot list, so the // person is told why nothing is offered. @@ -3430,7 +3866,10 @@ async fn a_failed_listing_or_exchange_ends_the_pick_with_its_own_error() { &[("Organization:", b"\r")], ) .await; - assert_failed_pick(&run, "Authentication failed (HTTP 400, invalid_grant)"); + assert_failed_pick( + &run, + "The saved login for person has expired or was revoked", + ); auth.verify().await; // The auth service fails to list environments in the picked organization.