From 139d7ec3c2f451d04d2e7b74d9c05bb20ed0bf9d Mon Sep 17 00:00:00 2001 From: Tee8z Date: Fri, 23 Oct 2026 12:27:19 -0400 Subject: [PATCH] fix(cli): never let a Retry-After shorten a watch's pause `payments list --watch` waited the server's `Retry-After` in place of its own pause. `retry_after()` returns zero for `Retry-After: 0`, `x-retry-after-ms: 0`, or an HTTP date already past, which clock skew can produce, so a 429 or 5xx carrying such a hint was retried at once, again and again, for as long as the server answered that way. On a refresh that worked, the same hint could make the watch poll faster than `--interval`. `watch::pause` takes the longer of the hint and the watch's own pause: `--interval` after a refresh that worked, the backoff after a failure. Retry-After is the least time the server asks for, so it can lengthen a pause but never shorten it. The backoff now advances on every failure, also one with a hint, so repeated rate limits wait longer; a success still resets it. --- CHANGELOG.md | 2 +- docs/guides/payments.md | 2 +- src/api.rs | 12 ++++++------ src/watch.rs | 17 +++++++++++++++++ tests/cli_integration.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 65 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39995ce..8a6df11 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ User-visible changes to `voltage`, newest first. Each pull request that changes - `voltage wallets get` on a terminal leads with the wallet's name, its balances (with a USD estimate on mainnet), and a 14-day sparkline of completed payments. - `--qr` on a terminal draws the invoice black on white with a quiet zone, so it scans on dark themes, under a `Scan to pay` caption with the amount, memo, and expiry, and after the result so it stays on screen; `--wait completed` counts down to the expiry. The caption leads with the network, its dot in the website's color. - A payment that settles on a terminal closes with `⚡ Payment ID completed` instead of a check mark, printed after the payment's record so it stays on screen. -- `voltage payments list --watch` follows payments until Ctrl-C: a live table that marks new and changed payments on a terminal, or one `snapshot`, `new`, or `updated` event per payment when piped or with `--json`. `--interval` sets the refresh (default 5 seconds). +- `voltage payments list --watch` follows payments until Ctrl-C: a live table that marks new and changed payments on a terminal, or one `snapshot`, `new`, or `updated` event per payment when piped or with `--json`. `--interval` sets the refresh (default 5 seconds). A server's `Retry-After` can lengthen the pause between refreshes but never shorten it. - `voltage` with no command, on a terminal, greets you with the wordmark, who you are signed in as, your saved scope, and commands to try; off a terminal it is still a usage error. An empty wallet, payment, or webhook list, and a new profile, end with a `›` line naming the next command. ## 0.2.0 - 2026-10-22 diff --git a/docs/guides/payments.md b/docs/guides/payments.md index d69eb07..1d1b401 100644 --- a/docs/guides/payments.md +++ b/docs/guides/payments.md @@ -45,7 +45,7 @@ voltage payments list --profile prod --wallet WALLET_ID --watch voltage payments list --profile prod --watch --interval 2 --json ``` -On a terminal it is a live table on the alternate screen, newest first, with `●` beside each payment that is new or changed since the last refresh; Ctrl-C puts your screen back and prints `Stopped watching payments.` (exit code 130). Piped or with `--json`, it writes one event envelope per payment instead: `snapshot` for each payment in the first read, then `new` or `updated` as they appear, with the payment in `data` and its ID in `id`. Each refresh reads the first page of the most recently updated payments (`sort_key=updated_at`, `sort_order=DESC`), so `--watch` cannot be combined with `--all`, `--cursor`, `--sort-key`, or `--sort-order`; more changes than one page holds (`--limit`, the API's default otherwise) between two refreshes are not all reported. A refresh that fails in transport, with a server error (5xx), or with a rate limit (429) is retried after the server's `Retry-After`, or after a pause that grows up to a minute, and the live view says why. A rejected credential (401) is resolved again once, so a saved login exchanges a fresh organization token. Any other error ends the watch. `--timeout` limits each refresh's request, not the watch, and `--quiet` also hides `Stopped watching payments.` On a terminal, rows beyond its height are left out and counted. +On a terminal it is a live table on the alternate screen, newest first, with `●` beside each payment that is new or changed since the last refresh; Ctrl-C puts your screen back and prints `Stopped watching payments.` (exit code 130). Piped or with `--json`, it writes one event envelope per payment instead: `snapshot` for each payment in the first read, then `new` or `updated` as they appear, with the payment in `data` and its ID in `id`. Each refresh reads the first page of the most recently updated payments (`sort_key=updated_at`, `sort_order=DESC`), so `--watch` cannot be combined with `--all`, `--cursor`, `--sort-key`, or `--sort-order`; more changes than one page holds (`--limit`, the API's default otherwise) between two refreshes are not all reported. A refresh that fails in transport, with a server error (5xx), or with a rate limit (429) is retried after a pause that grows up to a minute, or after the server's `Retry-After` when that is longer, and the live view says why. A `Retry-After` can lengthen the pause the watch would otherwise take, `--interval` after a refresh that worked, but never shorten it. A rejected credential (401) is resolved again once, so a saved login exchanges a fresh organization token. Any other error ends the watch. `--timeout` limits each refresh's request, not the watch, and `--quiet` also hides `Stopped watching payments.` On a terminal, rows beyond its height are left out and counted. ## Show an invoice diff --git a/src/api.rs b/src/api.rs index 8e13c73..212f777 100644 --- a/src/api.rs +++ b/src/api.rs @@ -966,9 +966,9 @@ impl Call<'_> { /// screen that marks payments new or changed since the last read; other output gets one /// event envelope per payment, `snapshot` for the first read, then `new` or `updated`. /// A refresh the server could not serve (a 5xx, a 429, or a transport failure) is retried - /// after its `Retry-After`, or after a growing pause; a rejected credential (401) is - /// resolved again once, since a saved login's organization token is short-lived; any - /// other failure ends the watch. + /// after a growing pause, or after its `Retry-After` when that is longer; a rejected + /// credential (401) is resolved again once, since a saved login's organization token is + /// short-lived; any other failure ends the watch. async fn watch(&mut self, mut response: Response, interval: Duration) -> Result<()> { // Each read would otherwise draw a status line over the table. self.api.terminal = self.api.terminal.quieted(); @@ -977,7 +977,7 @@ impl Call<'_> { let mut seen = watch::Seen::default(); let mut failure = None; let mut backoff = Backoff::new(interval, watch::BACKOFF_CEILING); - let mut pause = response.retry_after.unwrap_or(interval); + let mut pause = watch::pause(response.retry_after, interval); let mut reauthorized = false; loop { let changes = seen.update(&response.body); @@ -1008,7 +1008,7 @@ impl Call<'_> { .await; match sent { Ok(next) => { - pause = next.retry_after.unwrap_or(interval); + pause = watch::pause(next.retry_after, interval); response = next; failure = None; reauthorized = false; @@ -1021,7 +1021,7 @@ impl Call<'_> { pause = Duration::ZERO; } Err(error) if watch::retries(&error) => { - pause = error.retry_after.unwrap_or_else(|| backoff.pause()); + pause = watch::pause(error.retry_after, backoff.pause()); failure = Some(error); } Err(error) => return Err(error), diff --git a/src/watch.rs b/src/watch.rs index 4b38719..129755b 100644 --- a/src/watch.rs +++ b/src/watch.rs @@ -17,6 +17,13 @@ pub fn follow_updates(query: &mut Vec<(String, String)>) { ); } +/// The pause before the next refresh. `Retry-After` is the least time the server asks for, +/// so it can lengthen the watch's own pause, `--interval` or the backoff after a failure, +/// but never shorten it: a hint of zero, or a date already past, must not refresh at once. +pub fn pause(hint: Option, own: Duration) -> Duration { + hint.map_or(own, |hint| hint.max(own)) +} + /// A refresh the server could not serve now: a transport failure, a 429, or any 5xx, with or /// without a JSON body. pub fn retries(error: &Error) -> bool { @@ -132,6 +139,16 @@ mod tests { ); } + #[test] + fn a_retry_hint_lengthens_a_pause_but_never_shortens_it() { + let five = Duration::from_secs(5); + assert_eq!(pause(None, five), five); + assert_eq!(pause(Some(Duration::ZERO), five), five); + assert_eq!(pause(Some(Duration::from_secs(1)), five), five); + let longer = Duration::from_secs(30); + assert_eq!(pause(Some(longer), five), longer); + } + #[test] fn a_page_is_cut_to_the_rows_that_fit() { let page = json!({"items": [{"id": "a"}, {"id": "b"}, {"id": "c"}], "next_cursor": "x"}); diff --git a/tests/cli_integration.rs b/tests/cli_integration.rs index 53c22e4..0fb39ad 100644 --- a/tests/cli_integration.rs +++ b/tests/cli_integration.rs @@ -6377,6 +6377,46 @@ async fn a_watch_retries_a_server_error_and_ends_on_a_client_error() { server.verify().await; } +#[cfg(unix)] +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn a_watch_never_refreshes_sooner_than_its_own_pause_for_a_zero_retry_hint() { + let (server, dir) = fixture().await; + let a = "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa"; + // `Retry-After: 0` on a success and on a 429 asks for no wait at all. The watch still + // waits its interval after the success, and at least half of it after the 429. + route("GET", payments_path()) + .respond_with(in_order(vec![ + ok(json!({"items": [{"id": a, "status": "receiving"}]})), + ok(json!({"items": [{"id": a, "status": "receiving"}]})) + .insert_header("Retry-After", "0"), + ResponseTemplate::new(429).insert_header("Retry-After", "0"), + ResponseTemplate::new(403).set_body_json(json!({"error": {"detail": "Forbidden"}})), + ])) + .expect(4) + .mount(&server) + .await; + let started = std::time::Instant::now(); + cli(dir.path(), &server) + .args([ + "payments", + "list", + "--org", + ORG, + "--env", + ENV, + "--watch", + "--interval", + "1", + ]) + .timeout(Duration::from_secs(20)) + .assert() + .code(3); + // Interval, interval, then at least half the interval: a lower bound, so never flaky. + let elapsed = started.elapsed(); + assert!(elapsed >= Duration::from_millis(2500), "{elapsed:?}"); + server.verify().await; +} + /// What a pseudo-terminal has shown so far, and whether it has closed. #[cfg(unix)] #[derive(Default)]