Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes

## Unreleased

- A request that gets no response says why: a timeout (for the auth service, its fixed 30-second limit, which `--timeout` does not change), a connection not made within the 10-second connect limit (which `--timeout` does not change), a failed TLS connection, a connection that could not be made, or one that closed early. A hint names the next step. A write that timed out or closed early may have been applied, so its hint says to look up the resource ID before running the command again, never to retry. This covers API, auth, price, and checkout stream requests, which before said only that the request failed.
- `--output-file` on a path that already exists says so and suggests a new path, instead of showing an OS error; any other refusal keeps its cause and gets the same hint. Both say that no change was sent.
- `-o` is short for `--output`, as in `voltage wallets list -o json`.

Expand Down
2 changes: 1 addition & 1 deletion docs/guides/troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ Authentication or authorization failed. See [HTTP 401](#http-401) and [HTTP 403]

### Exit code 4

The connection failed. For a payment or treasury movement, the request may have been sent: the error reports the ID, and the CLI never resubmits it. Read it with `voltage payments get PAYMENT_ID` before you decide to retry. See [payment IDs](../concepts/safety-model.md#payment-ids-and-the-request-journal).
The request got no response. The message says why: no response within `--timeout` (for the auth service, within its fixed 30 seconds, which `--timeout` does not change), no connection within 10 seconds (an unreachable host or a firewall; `--timeout` does not change this limit), a failed TLS connection (often a certificate this system does not trust, or a proxy that inspects TLS), a connection that could not be made (DNS, a refused port, or a proxy), or a connection that closed early. The hint names the next step. A read can simply be retried, but a change that timed out or whose connection closed early may have been applied, so the hint says to look up its resource ID first: a rerun can repeat the change. For a payment or treasury movement, the request may have been sent: the error reports the ID, and the CLI never resubmits it. Read it with `voltage payments get PAYMENT_ID` before you decide to retry. See [payment IDs](../concepts/safety-model.md#payment-ids-and-the-request-journal).

### Exit code 5

Expand Down
37 changes: 30 additions & 7 deletions src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
read_secret, shell_word,
},
dry_run,
error::{CredentialSources, ErrorDetail, ErrorKind},
error::{CredentialSources, ErrorDetail, ErrorKind, ResponseLimit},
input::{PaginationMode, Request},
output::{Envelope, Outcome, Output},
payment::{
Expand Down Expand Up @@ -245,12 +245,26 @@ impl Api {
.request(method.into(), path, query, body)
.send()
.await
.map_err(|_| {
Error::transport(if read_only {
"HTTP read request failed. Check the API URL and whether the API service is running."
.map_err(|error| {
if read_only {
Error::no_response(
&error,
"HTTP read request",
"",
"API",
ResponseLimit::Timeout,
false,
)
} else {
"HTTP request failed; a write may have been submitted. No mutation was retried."
})
Error::no_response(
&error,
"HTTP request",
"; a write may have been submitted. No mutation was retried.",
"API",
ResponseLimit::Timeout,
true,
)
}
})?;
let status = response.status().as_u16();
let retry_after = retry_after(response.headers());
Expand Down Expand Up @@ -336,7 +350,16 @@ impl Api {
.send(),
)
.await
.map_err(|_| Error::transport("Could not open checkout event stream"))?;
.map_err(|error| {
Error::no_response(
&error,
"Opening the checkout event stream",
"",
"API",
ResponseLimit::Timeout,
false,
)
})?;
let status = response.status().as_u16();
if !response.status().is_success() {
return Err(Error::new(
Expand Down
41 changes: 36 additions & 5 deletions src/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
InputSource, LOCK_TIMEOUT, Login, Scope, ScopeSources, Settings, Source, name_in_use,
read_secret, shell_word, unreadable_name,
},
error::{CredentialRejection, CredentialSources, ErrorDetail},
error::{CredentialRejection, CredentialSources, ErrorDetail, ResponseLimit},
secret::Secret,
terminal::{self, Terminal},
};
Expand All @@ -37,7 +37,8 @@ const MAX_LOGIN_WAIT: u64 = 600;
const MIN_POLL_INTERVAL: u64 = 5;
const MAX_POLL_INTERVAL: u64 = 600;
const SLOW_DOWN_STEP: u64 = 5;
const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
/// `TransportFailure::ConnectTimeout` names this limit in its message.
pub(crate) const CONNECT_TIMEOUT: Duration = Duration::from_secs(10);
/// Auth responses are small JSON documents; anything larger is not one.
const MAX_AUTH_RESPONSE_BYTES: usize = 1024 * 1024;

Expand Down Expand Up @@ -188,9 +189,14 @@ async fn auth_request(
terminal: Terminal,
) -> Result<AuthResponse> {
let _progress = terminal.progress("Waiting for auth service...");
let response = request.send().await.map_err(|_| {
Error::transport(
"Authentication request failed; no token request was automatically retried",
let response = request.send().await.map_err(|error| {
Comment thread
thebrandonlucas marked this conversation as resolved.
Resolved
Error::no_response(
&error,
"Authentication request",
"; no token request was automatically retried",
"auth",
ResponseLimit::Fixed(AUTH_TIMEOUT),
false,
)
})?;
let status = response.status().as_u16();
Expand Down Expand Up @@ -1297,6 +1303,31 @@ mod tests {
assert_eq!(&body[..], b"token");
}

#[tokio::test]
async fn an_auth_timeout_names_its_fixed_limit_not_timeout_flag() {
// The listener takes the connection but never answers. The client's short limit
// stands in for AUTH_TIMEOUT, which the message must still name.
let silent = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let request = client(Duration::from_millis(100))
.unwrap()
.get(format!("http://{}/", silent.local_addr().unwrap()));
let Err(error) = auth_request(request, Terminal::new(true, true)).await else {
panic!("a silent auth service must fail");
};
assert_eq!(error.kind, crate::error::ErrorKind::Transport);
assert_eq!(
error.message,
"Authentication request failed: no response within 30 seconds; no token request was automatically retried"
);
assert_eq!(
error.hint.as_deref(),
Some(
"Retry. The auth service has a fixed 30-second limit, which --timeout does not change."
)
);
drop(silent);
}

#[tokio::test]
async fn oversized_auth_responses_are_rejected() {
let server = wiremock::MockServer::start().await;
Expand Down
Loading
Loading