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.