From cb567e86c0eb441723f0531b763af3ccacad8440 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Wed, 10 Jun 2026 15:55:02 -0400 Subject: [PATCH 1/5] Remove QN_CLI__API_KEY env var; add global --config-file flag API keys now come from exactly two sources, highest precedence first: the --api-key flag, then the config file (--config-file path if given, else ~/.config/qn/config.toml). The environment variable was removed deliberately: a key left exported in a shell is invisible state that outlives the session it was set for, and is the easiest way to run a destructive command against the wrong account. CI callers should write a config file and point --config-file at it. auth login/logout/status/whoami honor --config-file for both reads and writes. The [output] section is also read from the --config-file path. New tests: --config-file supplies the key, --api-key beats it, missing file exits 4, and a subprocess test proving the env var is inert. --- README.md | 26 ++++++++------ src/cli.rs | 14 +++++--- src/commands/auth.rs | 17 ++++------ src/config.rs | 72 +++++++++++++++++---------------------- src/context.rs | 21 ++++++++---- src/errors.rs | 2 +- tests/auth.rs | 80 ++++++++++++++++++++++++++++++++++++++++++++ tests/cli_smoke.rs | 15 ++++++++- tests/common/mod.rs | 16 +++++++-- tests/endpoint.rs | 3 -- 10 files changed, 184 insertions(+), 82 deletions(-) create mode 100644 tests/auth.rs diff --git a/README.md b/README.md index e97ef13..ff8fa9c 100644 --- a/README.md +++ b/README.md @@ -57,12 +57,19 @@ You will need a Quicknode API key to get started. Once you have that, you can ru `qn` resolves your API key from the first source that matches: 1. `--api-key ` flag -2. `QN_CLI__API_KEY` environment variable -3. `~/.config/qn/config.toml` — or `$XDG_CONFIG_HOME/qn/config.toml` if that env var is set. Managed by `qn auth login`. +2. The config file: the `--config-file ` flag if given, otherwise + `~/.config/qn/config.toml` — or `$XDG_CONFIG_HOME/qn/config.toml` if that + env var is set. Managed by `qn auth login`. -If none match, `qn` exits with code 4 and tells you to run `qn auth login`. -Regular commands never prompt — only `qn auth login` does. This keeps scripts -and CI deterministic. +There is deliberately **no environment-variable key source**: a key left +exported in a shell is invisible state that outlives the session it was set +for, and makes it far too easy to run a destructive command against the wrong +account. For CI, write a config file and point `--config-file` at it (or pass +`--api-key` from your secret store). + +If no source matches, `qn` exits with code 4 and tells you to run +`qn auth login`. Regular commands never prompt — only `qn auth login` does. +This keeps scripts and CI deterministic. ```sh qn auth login # prompts for the key, writes it to ~/.config/qn/config.toml @@ -196,12 +203,9 @@ qn completions powershell > qn.ps1 ## Configuration via environment -| Variable | Description | -|---|---| -| `QN_CLI__API_KEY` | Your Quicknode API key | - -`qn` deliberately uses its own `QN_CLI__` namespace so the CLI's env vars don't -collide with — or silently leak into — direct use of the underlying SDK. The +`qn` reads no API credentials from the environment (see +[Authentication](#authentication) for why). It honors the conventional +output-related variables only: `NO_COLOR` and `TERM=dumb` disable color. The CLI hands the key to the SDK explicitly; it does not read the SDK's `QN_SDK__*` environment namespace. diff --git a/src/cli.rs b/src/cli.rs index 41aa5a8..45764fa 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -21,16 +21,21 @@ use crate::output::Format; about = "Command-line interface for the Quicknode API.", long_about = "qn lets you manage Quicknode endpoints, streams, webhooks, and the KV store from the terminal.\n\n\ Use `qn --help` (e.g. `qn endpoint --help`) for command details.\n\n\ - Authentication is resolved in this order: --api-key flag, QN_CLI__API_KEY env var,\n\ - ~/.config/qn/config.toml. Run `qn auth login` to save a key the first time.", + Authentication is resolved in this order: --api-key flag, then the config file\n\ + (--config-file path if given, else ~/.config/qn/config.toml). Run `qn auth login`\n\ + to save a key the first time.", propagate_version = true, disable_help_subcommand = true )] pub struct Cli { - /// API key. Overrides QN_CLI__API_KEY and the config file. - #[arg(long, global = true, env = "QN_CLI__API_KEY", hide_env_values = true)] + /// API key. Overrides the config file. + #[arg(long, global = true)] pub api_key: Option, + /// Path to an alternate config file (default: ~/.config/qn/config.toml). + #[arg(long, global = true, value_name = "PATH")] + pub config_file: Option, + /// Output format. `table` is the default human view; the others are /// pipeline-friendly serialized forms. If unset, falls back to the /// `[output] format = "…"` value in ~/.config/qn/config.toml, then `table`. @@ -122,6 +127,7 @@ impl Cli { pub fn global_args(&self) -> GlobalArgs { GlobalArgs { api_key: self.api_key.clone(), + config_file: self.config_file.clone(), format: self.format, wide: self.wide, // format resolved-from-config in Ctx::from_global; auth.rs falls diff --git a/src/commands/auth.rs b/src/commands/auth.rs index 49c324b..ceae9d6 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -50,7 +50,7 @@ pub async fn run(args: Args, global: GlobalArgs) -> Result<(), CliError> { } async fn login(args: LoginArgs, global: GlobalArgs) -> Result<(), CliError> { - let path = config::config_path().ok_or_else(|| { + let path = global.resolve_config_path().ok_or_else(|| { CliError::Arg("no config directory available on this platform".to_string()) })?; @@ -82,7 +82,7 @@ async fn login(args: LoginArgs, global: GlobalArgs) -> Result<(), CliError> { } fn logout(global: GlobalArgs) -> Result<(), CliError> { - let path = config::config_path().ok_or_else(|| { + let path = global.resolve_config_path().ok_or_else(|| { CliError::Arg("no config directory available on this platform".to_string()) })?; config::delete_config(&path)?; @@ -111,15 +111,10 @@ async fn whoami(global: GlobalArgs) -> Result<(), CliError> { /// Resolves the key just like the rest of the CLI — no prompting. Returns the /// raw key (callers must redact before printing) along with its source. fn resolve_non_interactive(global: &GlobalArgs) -> Result<(String, KeySource), CliError> { - let env_key = config::read_env_api_key(); - let path = config::config_path(); - config::resolve_api_key( - global.api_key.as_deref(), - env_key.as_deref(), - path.as_deref(), - false, - || unreachable!("prompt disabled for auth status/whoami"), - ) + let path = global.resolve_config_path(); + config::resolve_api_key(global.api_key.as_deref(), path.as_deref(), false, || { + unreachable!("prompt disabled for auth status/whoami") + }) } /// Show the last 4 chars only. Char-based slicing — never panics on multi-byte input. diff --git a/src/config.rs b/src/config.rs index 3516de2..64d0a1a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -2,13 +2,17 @@ //! //! Resolution order, highest to lowest precedence: //! 1. `--api-key` flag -//! 2. `QN_CLI__API_KEY` env var -//! 3. config file +//! 2. config file (`--config-file` path if given, else the default path) //! -//! When all three fail we return [`CliError::NoApiKey`] which exits 4 with a -//! message directing the user to `qn auth login`. The `qn auth login` command -//! is the only place that prompts interactively; other commands never block -//! waiting for input. +//! There is deliberately no environment-variable source: a key left exported +//! in a shell is invisible state that outlives the session it was set for, +//! and is the easiest way to run a destructive command against the wrong +//! account. +//! +//! When both sources fail we return [`CliError::NoApiKey`] which exits 4 with +//! a message directing the user to `qn auth login`. The `qn auth login` +//! command is the only place that prompts interactively; other commands never +//! block waiting for input. use std::fs; use std::io::IsTerminal; @@ -19,12 +23,9 @@ use serde::{Deserialize, Serialize}; use crate::errors::CliError; use crate::output::Format; -const ENV_API_KEY: &str = "QN_CLI__API_KEY"; - #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum KeySource { Flag, - Env, ConfigFile, Prompt, } @@ -33,7 +34,6 @@ impl KeySource { pub fn label(self) -> &'static str { match self { KeySource::Flag => "--api-key flag", - KeySource::Env => "QN_CLI__API_KEY env var", KeySource::ConfigFile => "config file", KeySource::Prompt => "interactive prompt", } @@ -199,18 +199,17 @@ pub fn delete_config(path: &Path) -> Result<(), CliError> { } } -/// Resolves an API key per the documented precedence: flag > env > config file. +/// Resolves an API key per the documented precedence: flag > config file. /// /// `allow_prompt` and `prompt` exist only so `qn auth login` can opt into the /// interactive path. Regular commands pass `allow_prompt = false`; if the -/// three non-interactive sources fail they get `Err(NoApiKey)`. +/// non-interactive sources fail they get `Err(NoApiKey)`. /// /// `prompt` is supplied by the caller so tests can inject a deterministic /// closure instead of touching the real terminal. In production /// [`prompt_for_api_key`] is the implementation used by `qn auth login`. pub fn resolve_api_key( flag: Option<&str>, - env_key: Option<&str>, config_path: Option<&Path>, allow_prompt: bool, prompt: impl FnOnce() -> Result, @@ -218,9 +217,6 @@ pub fn resolve_api_key( if let Some(k) = flag.map(str::trim).filter(|s| !s.is_empty()) { return Ok((k.to_string(), KeySource::Flag)); } - if let Some(k) = env_key.map(str::trim).filter(|s| !s.is_empty()) { - return Ok((k.to_string(), KeySource::Env)); - } if let Some(path) = config_path { if let Some(cfg) = load_from(path)? { if let Some(k) = cfg @@ -251,13 +247,6 @@ pub fn can_prompt() -> bool { std::io::stdin().is_terminal() && std::io::stderr().is_terminal() } -/// Reads `QN_CLI__API_KEY` from the environment. -pub fn read_env_api_key() -> Option { - std::env::var(ENV_API_KEY) - .ok() - .filter(|s| !s.trim().is_empty()) -} - /// Interactive prompt for an API key. Hidden input on the terminal. pub fn prompt_for_api_key() -> Result { use dialoguer::Password; @@ -277,35 +266,34 @@ mod tests { } #[test] - fn flag_wins_over_everything() { - let (k, src) = - resolve_api_key(Some("from-flag"), Some("from-env"), None, true, fail_prompt).unwrap(); + fn flag_wins_over_config_and_prompt() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + save_api_key(&path, "from-config").unwrap(); + + let (k, src) = resolve_api_key(Some("from-flag"), Some(&path), true, fail_prompt).unwrap(); assert_eq!(k, "from-flag"); assert_eq!(src, KeySource::Flag); } #[test] - fn env_wins_over_config_and_prompt() { - let (k, src) = resolve_api_key(None, Some("from-env"), None, true, fail_prompt).unwrap(); - assert_eq!(k, "from-env"); - assert_eq!(src, KeySource::Env); - } + fn empty_flag_falls_through_to_config() { + let dir = tempdir().unwrap(); + let path = dir.path().join("config.toml"); + save_api_key(&path, "from-config").unwrap(); - #[test] - fn empty_flag_falls_through_to_env() { - let (k, src) = - resolve_api_key(Some(" "), Some("from-env"), None, true, fail_prompt).unwrap(); - assert_eq!(k, "from-env"); - assert_eq!(src, KeySource::Env); + let (k, src) = resolve_api_key(Some(" "), Some(&path), false, fail_prompt).unwrap(); + assert_eq!(k, "from-config"); + assert_eq!(src, KeySource::ConfigFile); } #[test] - fn config_used_when_no_flag_or_env() { + fn config_used_when_no_flag() { let dir = tempdir().unwrap(); let path = dir.path().join("config.toml"); save_api_key(&path, "from-config").unwrap(); - let (k, src) = resolve_api_key(None, None, Some(&path), false, fail_prompt).unwrap(); + let (k, src) = resolve_api_key(None, Some(&path), false, fail_prompt).unwrap(); assert_eq!(k, "from-config"); assert_eq!(src, KeySource::ConfigFile); } @@ -315,14 +303,14 @@ mod tests { let dir = tempdir().unwrap(); let path = dir.path().join("does-not-exist.toml"); let (k, src) = - resolve_api_key(None, None, Some(&path), true, || Ok("prompted".to_string())).unwrap(); + resolve_api_key(None, Some(&path), true, || Ok("prompted".to_string())).unwrap(); assert_eq!(k, "prompted"); assert_eq!(src, KeySource::Prompt); } #[test] fn no_inputs_with_prompt_disabled_returns_no_api_key() { - let err = resolve_api_key(None, None, None, false, fail_prompt).unwrap_err(); + let err = resolve_api_key(None, None, false, fail_prompt).unwrap_err(); assert!(matches!(err, CliError::NoApiKey)); } @@ -331,7 +319,7 @@ mod tests { let dir = tempdir().unwrap(); let path = dir.path().join("config.toml"); std::fs::write(&path, "this is = not valid = toml\n[[[").unwrap(); - let err = resolve_api_key(None, None, Some(&path), false, fail_prompt).unwrap_err(); + let err = resolve_api_key(None, Some(&path), false, fail_prompt).unwrap_err(); assert!(matches!(err, CliError::BadConfig { .. }), "got: {err:?}"); } diff --git a/src/context.rs b/src/context.rs index f86bee7..8179951 100644 --- a/src/context.rs +++ b/src/context.rs @@ -16,6 +16,9 @@ use crate::output::{Format, OutputCtx}; #[derive(Debug, Clone, Default)] pub struct GlobalArgs { pub api_key: Option, + /// `--config-file`: alternate config TOML. `None` means the default path + /// (`~/.config/qn/config.toml`). + pub config_file: Option, /// `None` means the user didn't pass `--format`; resolve via config file /// (then the TTY-aware default: `Table` on a TTY, `Toon` off) when we /// build the [`Ctx`]. @@ -49,8 +52,13 @@ impl GlobalArgs { resolve_output_inner(self.format, self.wide, cfg_format, cfg_wide, stdout_is_tty) } + /// The config file to read: `--config-file` if given, else the default path. + pub fn resolve_config_path(&self) -> Option { + self.config_file.clone().or_else(config::config_path) + } + fn load_output_config(&self) -> (Option, bool) { - let Some(p) = config::config_path() else { + let Some(p) = self.resolve_config_path() else { return (None, false); }; match config::load_from(&p) { @@ -87,18 +95,17 @@ pub struct Ctx { impl Ctx { /// Construct the SDK + output ctx from `global`. Resolves the API key per - /// the documented precedence: flag > env > config file. If none of those - /// supply a key we return `CliError::NoApiKey` — regular commands do not - /// prompt; the user is directed to `qn auth login`. + /// the documented precedence: flag > config file (`--config-file` path if + /// given, else the default). If neither supplies a key we return + /// `CliError::NoApiKey` — regular commands do not prompt; the user is + /// directed to `qn auth login`. pub fn from_global(global: GlobalArgs) -> Result { - let config_path = config::config_path(); - let env_key = config::read_env_api_key(); + let config_path = global.resolve_config_path(); let stdout_is_tty = std::io::stdout().is_terminal(); let (format, wide) = global.resolve_output(stdout_is_tty); let (api_key, _) = config::resolve_api_key( global.api_key.as_deref(), - env_key.as_deref(), config_path.as_deref(), false, || unreachable!("prompt disabled for non-auth commands"), diff --git a/src/errors.rs b/src/errors.rs index 8aabf95..59eb792 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -9,7 +9,7 @@ use thiserror::Error; #[derive(Debug, Error)] pub enum CliError { - #[error("no API key found. Set QN_CLI__API_KEY or run 'qn auth login'")] + #[error("no API key found. Run 'qn auth login', or pass --api-key or --config-file")] NoApiKey, #[error("config file at {path} is invalid: {source}")] diff --git a/tests/auth.rs b/tests/auth.rs new file mode 100644 index 0000000..44679cd --- /dev/null +++ b/tests/auth.rs @@ -0,0 +1,80 @@ +//! Key-resolution integration tests: `--config-file` and flag precedence. + +mod common; + +use common::{run_qn, run_qn_no_key}; +use serde_json::json; +use wiremock::matchers::{header, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +fn write_config(dir: &tempfile::TempDir, key: &str) -> std::path::PathBuf { + let path = dir.path().join("config.toml"); + std::fs::write(&path, format!("[api]\nkey = \"{key}\"\n")).unwrap(); + path +} + +#[tokio::test] +async fn config_file_flag_supplies_the_api_key() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .and(header("x-api-key", "from-config-file")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "data": [], + "pagination": { "total": 0, "limit": 20, "offset": 0 } + }))) + .expect(1) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let cfg = write_config(&dir, "from-config-file"); + let out = run_qn_no_key( + &server.uri(), + &["--config-file", cfg.to_str().unwrap(), "endpoint", "list"], + ) + .await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn api_key_flag_wins_over_config_file() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .and(header("x-api-key", "test")) // the harness-injected flag value + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "data": [], + "pagination": { "total": 0, "limit": 20, "offset": 0 } + }))) + .expect(1) + .mount(&server) + .await; + + let dir = tempfile::tempdir().unwrap(); + let cfg = write_config(&dir, "from-config-file"); + let out = run_qn( + &server.uri(), + &["--config-file", cfg.to_str().unwrap(), "endpoint", "list"], + ) + .await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn missing_config_file_with_no_other_source_exits_4() { + let server = MockServer::start().await; + let dir = tempfile::tempdir().unwrap(); + let cfg = dir.path().join("does-not-exist.toml"); + let out = run_qn_no_key( + &server.uri(), + &["--config-file", cfg.to_str().unwrap(), "endpoint", "list"], + ) + .await; + assert_eq!(out.exit_code, 4, "stderr={}", out.stderr); + assert!( + out.stderr.contains("no API key found"), + "stderr={}", + out.stderr + ); +} diff --git a/tests/cli_smoke.rs b/tests/cli_smoke.rs index 17e609e..a97f6dc 100644 --- a/tests/cli_smoke.rs +++ b/tests/cli_smoke.rs @@ -10,7 +10,6 @@ use predicates::prelude::*; fn bin() -> Command { let mut c = Command::cargo_bin("qn").expect("qn binary built"); // Make sure no inherited env hijacks the tests. - c.env_remove("QN_CLI__API_KEY"); c.env_remove("NO_COLOR"); c.env_remove("HOME"); // so config lookup doesn't read a real ~/.config/qn c.env("HOME", std::env::temp_dir()); @@ -82,6 +81,20 @@ fn no_api_key_no_tty_exits_4() { .stderr(predicate::str::contains("no API key found")); } +#[test] +fn env_var_api_key_is_not_a_key_source() { + // QN_CLI__API_KEY support was removed deliberately (a key left exported in + // a shell outlives its session). Setting it must not authenticate anything. + bin() + .env("QN_CLI__API_KEY", "should-be-ignored") + .args(["endpoint", "list"]) + .args(["--no-input"]) + .assert() + .failure() + .code(4) + .stderr(predicate::str::contains("no API key found")); +} + #[test] fn completions_zsh_works() { bin() diff --git a/tests/common/mod.rs b/tests/common/mod.rs index a66f64c..8618171 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -24,19 +24,31 @@ pub struct RunOutput { /// The SDK is pointed at `base_url`, an API key of `"test"` is supplied, and /// `--no-input` is set so no command can hang on a prompt. pub async fn run_qn(base_url: &str, extra_args: &[&str]) -> RunOutput { + run_qn_inner(base_url, extra_args, true).await +} + +/// Like [`run_qn`] but without injecting `--api-key test`, for tests that +/// exercise key resolution themselves (e.g. via `--config-file`). +pub async fn run_qn_no_key(base_url: &str, extra_args: &[&str]) -> RunOutput { + run_qn_inner(base_url, extra_args, false).await +} + +async fn run_qn_inner(base_url: &str, extra_args: &[&str], inject_key: bool) -> RunOutput { // We can't easily redirect stdout/stderr of the parent process from within // a tokio test, so we don't try. Tests assert via wiremock matchers (for // request shape) and via the exit code; visible-output assertions are in // the cli_smoke.rs subprocess tests instead. let mut argv: Vec = vec![ "qn".to_string(), - "--api-key".to_string(), - "test".to_string(), "--base-url".to_string(), base_url.to_string(), "--no-input".to_string(), "--no-color".to_string(), ]; + if inject_key { + argv.insert(1, "--api-key".to_string()); + argv.insert(2, "test".to_string()); + } argv.extend(extra_args.iter().map(|s| s.to_string())); let cli = match Cli::try_parse_from(&argv) { diff --git a/tests/endpoint.rs b/tests/endpoint.rs index 3549a72..50e9b01 100644 --- a/tests/endpoint.rs +++ b/tests/endpoint.rs @@ -42,7 +42,6 @@ async fn list_endpoints_json_output_is_valid_json() { let output = Command::cargo_bin("qn") .unwrap() - .env_remove("QN_CLI__API_KEY") .env_remove("HOME") .env("HOME", std::env::temp_dir()) .args([ @@ -89,7 +88,6 @@ async fn list_endpoints_all_formats_render() { for fmt in ["json", "yaml", "md", "toon", "table"] { let output = Command::cargo_bin("qn") .unwrap() - .env_remove("QN_CLI__API_KEY") .env_remove("HOME") .env("HOME", std::env::temp_dir()) .args([ @@ -136,7 +134,6 @@ async fn list_endpoints_wide_md_includes_urls() { let output = Command::cargo_bin("qn") .unwrap() - .env_remove("QN_CLI__API_KEY") .env_remove("HOME") .env("HOME", std::env::temp_dir()) .args([ From 30be7c2708318c54f9a82a38842b2eebb32582c0 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Wed, 10 Jun 2026 16:03:18 -0400 Subject: [PATCH 2/5] Add retry with full-jitter backoff to read-only commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-only commands (list/show/logs/metrics/usage/billing/chain/kv reads/whoami) now retry transient failures: HTTP 429/500/502/503/504 plus transport timeouts and connection errors. Backoff is full-jitter exponential (base 500ms, capped at 8s per sleep), defaulting to 3 retries; the global --retries flag tunes it and 0 disables. Mutating commands never retry automatically: a retried create could provision (and bill) twice. stream test-filter is the one POST that retries — it evaluates a filter against historical data and changes nothing. Adds src/retry.rs (5 unit tests on paused tokio time) and tests/retry.rs (5 wiremock tests, including proof that POST create hits the server exactly once on a 500). New dep: fastrand (tiny, zero-dependency RNG for jitter). --- Cargo.lock | 1 + Cargo.toml | 4 +- README.md | 12 +++ src/cli.rs | 7 ++ src/commands/auth.rs | 4 +- src/commands/billing.rs | 5 +- src/commands/chain.rs | 3 +- src/commands/endpoint/mod.rs | 22 +++-- src/commands/endpoint/ratelimit.rs | 8 +- src/commands/endpoint/security.rs | 11 ++- src/commands/endpoint/tag.rs | 2 +- src/commands/kv/actions.rs | 53 +++++------ src/commands/metrics.rs | 11 ++- src/commands/stream/actions.rs | 18 ++-- src/commands/team.rs | 7 +- src/commands/usage.rs | 20 +++- src/commands/webhook/actions.rs | 10 +- src/context.rs | 3 + src/lib.rs | 1 + src/retry.rs | 143 +++++++++++++++++++++++++++++ tests/retry.rs | 111 ++++++++++++++++++++++ 21 files changed, 389 insertions(+), 67 deletions(-) create mode 100644 src/retry.rs create mode 100644 tests/retry.rs diff --git a/Cargo.lock b/Cargo.lock index 6a4fd97..74fb14e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1487,6 +1487,7 @@ dependencies = [ "clap_complete", "comfy-table", "dialoguer", + "fastrand", "humantime", "insta", "predicates", diff --git a/Cargo.toml b/Cargo.toml index a469f6e..b2a2989 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,8 @@ path = "src/lib.rs" quicknode-sdk = "0.1" clap = { version = "4", features = ["derive", "env", "wrap_help"] } clap_complete = "4" -tokio = { version = "1", features = ["macros", "rt-multi-thread"] } +tokio = { version = "1", features = ["macros", "rt-multi-thread", "time"] } +fastrand = "2" serde = { version = "1", features = ["derive"] } serde_json = "1" serde_yml = "0.0.12" @@ -41,6 +42,7 @@ url = "2" [dev-dependencies] reqwest = { version = "0.12", default-features = false } +tokio = { version = "1", features = ["test-util"] } wiremock = "0.6" assert_cmd = "2" predicates = "3" diff --git a/README.md b/README.md index ff8fa9c..252c69d 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,18 @@ CLI hands the key to the SDK explicitly; it does not read the SDK's The hidden `--base-url ` flag overrides the API host for all four sub-clients at once (used for integration tests and on-prem mirrors). +## Retries + +Read-only commands (`list`, `show`, `logs`, `metrics`, `usage`, …) retry +transient failures — HTTP 429/5xx, timeouts, connection errors — with +exponential backoff and full jitter. The default is 3 retries; tune it with +the global `--retries ` flag (`--retries 0` disables). + +Commands that modify resources (`create`, `update`, `delete`, `pause`, …) +**never** retry automatically: the API has no idempotency keys yet, so a +retried create could provision twice. If a mutation fails with a transient +error, check whether it took effect before re-running it. + ## Exit codes | Code | Meaning | diff --git a/src/cli.rs b/src/cli.rs index 45764fa..0521324 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -64,6 +64,12 @@ pub struct Cli { #[arg(long, global = true)] pub no_input: bool, + /// Max automatic retries for read-only commands on transient failures + /// (HTTP 429/5xx, timeouts). Uses exponential backoff with jitter. + /// 0 disables retries. Commands that modify resources never retry. + #[arg(long, global = true, default_value_t = 3, value_name = "N")] + pub retries: u32, + /// Skip confirmation prompts. Pass twice for destructive bulk operations like `stream delete-all`. #[arg(short = 'y', long = "yes", global = true, action = ArgAction::Count)] pub yes: u8, @@ -137,6 +143,7 @@ impl Cli { verbose: self.verbose, no_input: self.no_input, yes_count: self.yes, + retries: self.retries, base_url: self.base_url.clone(), } } diff --git a/src/commands/auth.rs b/src/commands/auth.rs index ceae9d6..0e3d73f 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -72,7 +72,7 @@ async fn login(args: LoginArgs, global: GlobalArgs) -> Result<(), CliError> { // Quick validation against the API so we don't silently save a bogus key. let sdk = QuicknodeSdk::new(&SdkFullConfig::from_api_key(key.clone()))?; - sdk.admin.list_chains().await?; + crate::retry::retrying(global.retries, || sdk.admin.list_chains()).await?; config::save_api_key(&path, &key)?; if !global.quiet { @@ -102,7 +102,7 @@ async fn whoami(global: GlobalArgs) -> Result<(), CliError> { let (key, source) = resolve_non_interactive(&global)?; let redacted = redact(&key); let sdk = QuicknodeSdk::new(&SdkFullConfig::from_api_key(key))?; - let result = sdk.admin.list_chains().await; + let result = crate::retry::retrying(global.retries, || sdk.admin.list_chains()).await; let ok = result.is_ok(); print_status(&global, source, &redacted, Some(ok)); result.map(|_| ()).map_err(Into::into) diff --git a/src/commands/billing.rs b/src/commands/billing.rs index 70ee578..a47c88c 100644 --- a/src/commands/billing.rs +++ b/src/commands/billing.rs @@ -7,6 +7,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; +use crate::retry::retrying; #[derive(Debug, ClapArgs)] pub struct Args { @@ -30,12 +31,12 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { } async fn invoices(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_invoices().await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.list_invoices()).await?; crate::output::emit(&ctx.out, &InvoicesView(resp)) } async fn payments(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_payments().await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.list_payments()).await?; crate::output::emit(&ctx.out, &PaymentsView(resp)) } diff --git a/src/commands/chain.rs b/src/commands/chain.rs index 91b2673..08d4ad0 100644 --- a/src/commands/chain.rs +++ b/src/commands/chain.rs @@ -7,6 +7,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, set_header_bold, write_table, Render}; +use crate::retry::retrying; #[derive(Debug, ClapArgs)] pub struct Args { @@ -28,7 +29,7 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { } async fn list(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_chains().await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.list_chains()).await?; crate::output::emit(&ctx.out, &ChainsView(resp)) } diff --git a/src/commands/endpoint/mod.rs b/src/commands/endpoint/mod.rs index 1b94edf..0145544 100644 --- a/src/commands/endpoint/mod.rs +++ b/src/commands/endpoint/mod.rs @@ -9,6 +9,7 @@ use quicknode_sdk::admin::{ use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; +use crate::retry::retrying; use crate::time_arg; mod bulk; @@ -226,7 +227,7 @@ async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { if !a.tag_labels.is_empty() { req.tag_labels = Some(a.tag_labels); } - let resp = ctx.sdk.admin.get_endpoints(&req).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_endpoints(&req)).await?; crate::output::emit(&ctx.out, &render::EndpointsView(resp)) } @@ -255,7 +256,7 @@ async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { } async fn show(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.show_endpoint(id).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.show_endpoint(id)).await?; let data = resp .data .ok_or_else(|| CliError::Arg(format!("endpoint {id} not found")))?; @@ -305,7 +306,7 @@ async fn set_status(id: &str, status: &str, ctx: Ctx) -> Result<(), CliError> { } async fn urls(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_endpoint_urls(id).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_endpoint_urls(id)).await?; let data = resp .data .ok_or_else(|| CliError::Arg(format!("API returned no URL data for endpoint {id}")))?; @@ -322,12 +323,18 @@ async fn logs(a: LogsArgs, ctx: Ctx) -> Result<(), CliError> { next_at: a.next_at, include_details: Some(a.details), }; - let resp = ctx.sdk.admin.get_endpoint_logs(&a.id, &req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_endpoint_logs(&a.id, &req) + }) + .await?; crate::output::emit(&ctx.out, &render::EndpointLogsView(resp)) } async fn log_details(id: &str, request_id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_log_details(id, request_id).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_log_details(id, request_id) + }) + .await?; crate::output::emit(&ctx.out, &render::LogDetailsView(resp)) } @@ -336,7 +343,10 @@ async fn metrics(a: MetricsArgs, ctx: Ctx) -> Result<(), CliError> { period: a.period, metric: a.metric, }; - let resp = ctx.sdk.admin.get_endpoint_metrics(&a.id, &req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_endpoint_metrics(&a.id, &req) + }) + .await?; crate::output::emit(&ctx.out, &render::EndpointMetricsView(resp)) } diff --git a/src/commands/endpoint/ratelimit.rs b/src/commands/endpoint/ratelimit.rs index ed0c69a..8530c16 100644 --- a/src/commands/endpoint/ratelimit.rs +++ b/src/commands/endpoint/ratelimit.rs @@ -11,6 +11,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; +use crate::retry::retrying; #[derive(Debug, Subcommand)] pub enum RateLimitCmd { @@ -128,7 +129,7 @@ pub async fn run(cmd: RateLimitCmd, ctx: Ctx) -> Result<(), CliError> { } async fn get(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_rate_limits(id).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_rate_limits(id)).await?; crate::output::emit(&ctx.out, &RateLimitsView(resp)) } @@ -151,7 +152,10 @@ async fn set(a: SetArgs, ctx: Ctx) -> Result<(), CliError> { } async fn method_list(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_method_rate_limits(id).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_method_rate_limits(id) + }) + .await?; crate::output::emit(&ctx.out, &MethodRateLimitsView(resp)) } diff --git a/src/commands/endpoint/security.rs b/src/commands/endpoint/security.rs index 4892157..6ba9eba 100644 --- a/src/commands/endpoint/security.rs +++ b/src/commands/endpoint/security.rs @@ -14,6 +14,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; +use crate::retry::retrying; #[derive(Debug, Subcommand)] pub enum SecurityCmd { @@ -206,12 +207,18 @@ pub async fn run(cmd: SecurityCmd, ctx: Ctx) -> Result<(), CliError> { } async fn show(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_endpoint_security(id).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_endpoint_security(id) + }) + .await?; crate::output::emit(&ctx.out, &SecurityShowView(resp)) } async fn options_show(id: &str, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_security_options(id).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_security_options(id) + }) + .await?; crate::output::emit(&ctx.out, &SecurityOptionsView(resp)) } diff --git a/src/commands/endpoint/tag.rs b/src/commands/endpoint/tag.rs index 2a41cd9..262154a 100644 --- a/src/commands/endpoint/tag.rs +++ b/src/commands/endpoint/tag.rs @@ -58,7 +58,7 @@ pub async fn run(cmd: TagCmd, ctx: Ctx) -> Result<(), CliError> { } async fn list(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_tags().await?; + let resp = crate::retry::retrying(ctx.global.retries, || ctx.sdk.admin.list_tags()).await?; crate::output::emit(&ctx.out, &TagsView(resp)) } diff --git a/src/commands/kv/actions.rs b/src/commands/kv/actions.rs index 3cfe808..b7b0c55 100644 --- a/src/commands/kv/actions.rs +++ b/src/commands/kv/actions.rs @@ -11,6 +11,7 @@ use super::render::{ListView, ListsView, SetsView}; use super::{ListCmd, SetCmd}; use crate::context::Ctx; use crate::errors::CliError; +use crate::retry::retrying; pub(super) async fn set(cmd: SetCmd, ctx: Ctx) -> Result<(), CliError> { match cmd { @@ -30,7 +31,7 @@ pub(super) async fn set(cmd: SetCmd, ctx: Ctx) -> Result<(), CliError> { ctx.out.note(&format!("✓ Set {key:?}")); } SetCmd::Get { key } => { - let resp = ctx.sdk.kvstore.get_set(&key).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.kvstore.get_set(&key)).await?; if ctx.out.format.is_structured() { crate::output::emit(&ctx.out, &resp)?; } else { @@ -38,14 +39,11 @@ pub(super) async fn set(cmd: SetCmd, ctx: Ctx) -> Result<(), CliError> { } } SetCmd::Ls(a) => { - let resp = ctx - .sdk - .kvstore - .get_sets(&GetSetsParams { - limit: a.limit, - cursor: a.cursor, - }) - .await?; + let params = GetSetsParams { + limit: a.limit, + cursor: a.cursor, + }; + let resp = retrying(ctx.global.retries, || ctx.sdk.kvstore.get_sets(¶ms)).await?; crate::output::emit(&ctx.out, &SetsView(resp))?; } SetCmd::Delete { key } => { @@ -88,28 +86,22 @@ pub(super) async fn set(cmd: SetCmd, ctx: Ctx) -> Result<(), CliError> { pub(super) async fn list(cmd: ListCmd, ctx: Ctx) -> Result<(), CliError> { match cmd { ListCmd::Ls(a) => { - let resp = ctx - .sdk - .kvstore - .get_lists(&GetListsParams { - limit: a.limit, - cursor: a.cursor, - }) - .await?; + let params = GetListsParams { + limit: a.limit, + cursor: a.cursor, + }; + let resp = retrying(ctx.global.retries, || ctx.sdk.kvstore.get_lists(¶ms)).await?; crate::output::emit(&ctx.out, &ListsView(resp))?; } ListCmd::Get(a) => { - let resp = ctx - .sdk - .kvstore - .get_list( - &a.key, - &GetListParams { - limit: a.limit, - cursor: a.cursor, - }, - ) - .await?; + let params = GetListParams { + limit: a.limit, + cursor: a.cursor, + }; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.kvstore.get_list(&a.key, ¶ms) + }) + .await?; crate::output::emit(&ctx.out, &ListView(resp))?; } ListCmd::Create { key, items } => { @@ -133,7 +125,10 @@ pub(super) async fn list(cmd: ListCmd, ctx: Ctx) -> Result<(), CliError> { ctx.out.note(&format!("✓ Appended {item:?} to {key:?}")); } ListCmd::Contains { key, item } => { - let resp = ctx.sdk.kvstore.list_contains_item(&key, &item).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.kvstore.list_contains_item(&key, &item) + }) + .await?; if ctx.out.format.is_structured() { crate::output::emit(&ctx.out, &resp)?; } else { diff --git a/src/commands/metrics.rs b/src/commands/metrics.rs index 390e55d..f6911bf 100644 --- a/src/commands/metrics.rs +++ b/src/commands/metrics.rs @@ -7,6 +7,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::Render; +use crate::retry::retrying; #[derive(Debug, ClapArgs)] pub struct Args { @@ -60,7 +61,10 @@ async fn account(a: AccountArgs, ctx: Ctx) -> Result<(), CliError> { metric: a.metric, percentile: a.percentile, }; - let resp = ctx.sdk.admin.get_account_metrics(&req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_account_metrics(&req) + }) + .await?; crate::output::emit(&ctx.out, &AccountMetricsView(resp)) } @@ -69,7 +73,10 @@ async fn endpoint(a: EndpointArgs, ctx: Ctx) -> Result<(), CliError> { period: a.period, metric: a.metric, }; - let resp = ctx.sdk.admin.get_endpoint_metrics(&a.id, &req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_endpoint_metrics(&a.id, &req) + }) + .await?; crate::output::emit(&ctx.out, &EndpointMetricsView(resp)) } diff --git a/src/commands/stream/actions.rs b/src/commands/stream/actions.rs index 622a470..c06e908 100644 --- a/src/commands/stream/actions.rs +++ b/src/commands/stream/actions.rs @@ -12,6 +12,7 @@ use super::{CreateArgs, ListArgs, TestFilterArgs, UpdateArgs}; use crate::confirm::{decide_without_prompt, prompt_typed, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; +use crate::retry::retrying; pub(super) async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { let params = ListStreamsParams { @@ -21,7 +22,7 @@ pub(super) async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { order_by: a.order_by, order_direction: a.order_direction, }; - let resp = ctx.sdk.streams.list_streams(¶ms).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.streams.list_streams(¶ms)).await?; crate::output::emit(&ctx.out, &StreamsListView(resp)) } @@ -108,7 +109,7 @@ fn build_create_params(a: CreateArgs) -> Result { } pub(super) async fn show(id: &str, ctx: Ctx) -> Result<(), CliError> { - let s = ctx.sdk.streams.get_stream(id).await?; + let s = retrying(ctx.global.retries, || ctx.sdk.streams.get_stream(id)).await?; crate::output::emit(&ctx.out, &StreamView(s)) } @@ -201,16 +202,17 @@ pub(super) async fn test_filter(a: TestFilterArgs, ctx: Ctx) -> Result<(), CliEr filter_language: a.filter_language.map(Into::into), address_book_config: None, }; - let resp = ctx.sdk.streams.test_filter(¶ms).await?; + // POST, but read-only: it evaluates a filter against historical data and + // changes nothing, so it's safe to retry. + let resp = retrying(ctx.global.retries, || ctx.sdk.streams.test_filter(¶ms)).await?; crate::output::emit(&ctx.out, &TestFilterView(resp)) } pub(super) async fn enabled_count(stream_type: Option, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx - .sdk - .streams - .get_enabled_count(stream_type.as_deref()) - .await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.streams.get_enabled_count(stream_type.as_deref()) + }) + .await?; if ctx.out.format.is_structured() { crate::output::emit(&ctx.out, &resp) } else { diff --git a/src/commands/team.rs b/src/commands/team.rs index 7b78dfe..b9adc99 100644 --- a/src/commands/team.rs +++ b/src/commands/team.rs @@ -11,6 +11,7 @@ use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity} use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; +use crate::retry::retrying; #[derive(Debug, ClapArgs)] pub struct Args { @@ -123,7 +124,7 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { } async fn list(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_teams().await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.list_teams()).await?; crate::output::emit(&ctx.out, &TeamsView(resp)) } @@ -137,7 +138,7 @@ async fn create(name: String, ctx: Ctx) -> Result<(), CliError> { } async fn show(id: i64, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.get_team(id).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_team(id)).await?; crate::output::emit(&ctx.out, &TeamDetailView(resp)) } @@ -160,7 +161,7 @@ async fn delete(id: i64, ctx: Ctx) -> Result<(), CliError> { } async fn endpoints(id: i64, ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.admin.list_team_endpoints(id).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.list_team_endpoints(id)).await?; crate::output::emit(&ctx.out, &TeamEndpointsView(resp)) } diff --git a/src/commands/usage.rs b/src/commands/usage.rs index 48626e6..c46a482 100644 --- a/src/commands/usage.rs +++ b/src/commands/usage.rs @@ -8,6 +8,7 @@ use serde::Serialize; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; +use crate::retry::retrying; use crate::time_arg; #[derive(Debug, ClapArgs)] @@ -61,31 +62,40 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { async fn summary(r: Range, ctx: Ctx) -> Result<(), CliError> { let req = r.into_request()?; - let resp = ctx.sdk.admin.get_usage(&req).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_usage(&req)).await?; crate::output::emit(&ctx.out, &UsageSummaryView(resp)) } async fn by_endpoint(r: Range, ctx: Ctx) -> Result<(), CliError> { let req = r.into_request()?; - let resp = ctx.sdk.admin.get_usage_by_endpoint(&req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_usage_by_endpoint(&req) + }) + .await?; crate::output::emit(&ctx.out, &UsageByEndpointView(resp)) } async fn by_method(r: Range, ctx: Ctx) -> Result<(), CliError> { let req = r.into_request()?; - let resp = ctx.sdk.admin.get_usage_by_method(&req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_usage_by_method(&req) + }) + .await?; crate::output::emit(&ctx.out, &UsageByMethodView(resp)) } async fn by_chain(r: Range, ctx: Ctx) -> Result<(), CliError> { let req = r.into_request()?; - let resp = ctx.sdk.admin.get_usage_by_chain(&req).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.admin.get_usage_by_chain(&req) + }) + .await?; crate::output::emit(&ctx.out, &UsageByChainView(resp)) } async fn by_tag(r: Range, ctx: Ctx) -> Result<(), CliError> { let req = r.into_request()?; - let resp = ctx.sdk.admin.get_usage_by_tag(&req).await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.admin.get_usage_by_tag(&req)).await?; crate::output::emit(&ctx.out, &UsageByTagView(resp)) } diff --git a/src/commands/webhook/actions.rs b/src/commands/webhook/actions.rs index 51e944f..16e423e 100644 --- a/src/commands/webhook/actions.rs +++ b/src/commands/webhook/actions.rs @@ -15,18 +15,22 @@ use super::{ActivateArgs, CreateArgs, ListArgs, TemplateKind, UpdateArgs, Update use crate::confirm::{decide_without_prompt, prompt_typed, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; +use crate::retry::retrying; pub(super) async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { let params = GetWebhooksParams { limit: a.limit, offset: a.offset, }; - let resp = ctx.sdk.webhooks.list_webhooks(¶ms).await?; + let resp = retrying(ctx.global.retries, || { + ctx.sdk.webhooks.list_webhooks(¶ms) + }) + .await?; crate::output::emit(&ctx.out, &WebhooksListView(resp)) } pub(super) async fn show(id: &str, ctx: Ctx) -> Result<(), CliError> { - let w = ctx.sdk.webhooks.get_webhook(id).await?; + let w = retrying(ctx.global.retries, || ctx.sdk.webhooks.get_webhook(id)).await?; crate::output::emit(&ctx.out, &WebhookView(w)) } @@ -180,7 +184,7 @@ pub(super) async fn pause(id: &str, ctx: Ctx) -> Result<(), CliError> { } pub(super) async fn enabled_count(ctx: Ctx) -> Result<(), CliError> { - let resp = ctx.sdk.webhooks.get_enabled_count().await?; + let resp = retrying(ctx.global.retries, || ctx.sdk.webhooks.get_enabled_count()).await?; if ctx.out.format.is_structured() { crate::output::emit(&ctx.out, &resp) } else { diff --git a/src/context.rs b/src/context.rs index 8179951..336eed3 100644 --- a/src/context.rs +++ b/src/context.rs @@ -29,6 +29,9 @@ pub struct GlobalArgs { pub verbose: bool, pub no_input: bool, pub yes_count: u8, + /// Max automatic retries for read-only API calls (see `crate::retry`). + /// `Default` yields 0 (no retries) — the CLI default of 3 comes from clap. + pub retries: u32, pub base_url: Option, } diff --git a/src/lib.rs b/src/lib.rs index f6bdb4d..aade0ae 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -11,6 +11,7 @@ pub(crate) mod commands; pub(crate) mod config; pub(crate) mod confirm; pub(crate) mod context; +pub(crate) mod retry; pub(crate) mod time_arg; pub use cli::Cli; diff --git a/src/retry.rs b/src/retry.rs new file mode 100644 index 0000000..523eaa3 --- /dev/null +++ b/src/retry.rs @@ -0,0 +1,143 @@ +//! Full-jitter exponential backoff for read-only API calls. +//! +//! Mutating commands must NOT use this. The API has no idempotency keys, so a +//! retried create/update/delete can be applied twice (duplicate endpoints, +//! double tag writes, …). Reads are safe to repeat, and the backend is +//! rate-limited, so scripted reads need to survive the occasional 429. +//! +//! `Retry-After` is not honored yet: `SdkError::Api` carries only the status +//! and body, not response headers. Honoring it needs a quicknode-sdk change; +//! until then full jitter keeps concurrent callers from herding. + +use std::future::Future; +use std::time::Duration; + +use quicknode_sdk::errors::{HttpKind, SdkError}; + +const BASE_DELAY_MS: u64 = 500; +const MAX_DELAY_MS: u64 = 8_000; + +/// Run `f`, retrying up to `max_retries` extra times on transient failures +/// (timeouts, connect errors, HTTP 429/500/502/504/503). `f` is invoked fresh +/// for each attempt. `max_retries = 0` means a single attempt, no retry. +pub async fn retrying(max_retries: u32, f: F) -> Result +where + F: Fn() -> Fut, + Fut: Future>, +{ + let mut attempt: u32 = 0; + loop { + match f().await { + Ok(v) => return Ok(v), + Err(e) if attempt < max_retries && is_retryable(&e) => { + tokio::time::sleep(delay_for(attempt)).await; + attempt += 1; + } + Err(e) => return Err(e), + } + } +} + +/// Transient failures only. Auth errors, 404s, validation errors, etc. will +/// fail identically on every attempt — retrying them just adds latency. +fn is_retryable(e: &SdkError) -> bool { + match e { + SdkError::Api { status, .. } => matches!(status.as_u16(), 429 | 500 | 502 | 503 | 504), + SdkError::Http(_) => matches!(e.http_kind(), Some(HttpKind::Timeout | HttpKind::Connect)), + _ => false, + } +} + +/// Full jitter: a uniformly random delay in `[0, base * 2^attempt]`, capped. +/// Randomizing the whole interval (rather than adding a small jitter term) +/// spreads concurrent retriers across the window instead of re-synchronizing +/// them at the next power of two. +fn delay_for(attempt: u32) -> Duration { + let exp = attempt.min(10); // 2^10 * 500ms is already far past the cap + let ceiling_ms = BASE_DELAY_MS.saturating_mul(1 << exp).min(MAX_DELAY_MS); + Duration::from_millis(fastrand::u64(0..=ceiling_ms)) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::atomic::{AtomicU32, Ordering}; + + fn api_error(status: u16) -> SdkError { + SdkError::Api { + status: reqwest::StatusCode::from_u16(status).unwrap(), + body: String::new(), + } + } + + #[test] + fn retryable_statuses() { + for s in [429, 500, 502, 503, 504] { + assert!(is_retryable(&api_error(s)), "{s} should be retryable"); + } + for s in [400, 401, 403, 404, 422] { + assert!(!is_retryable(&api_error(s)), "{s} should not be retryable"); + } + } + + #[test] + fn delay_never_exceeds_cap() { + for attempt in 0..64 { + assert!(delay_for(attempt) <= Duration::from_millis(MAX_DELAY_MS)); + } + } + + #[tokio::test(start_paused = true)] + async fn retries_until_success() { + let calls = AtomicU32::new(0); + let result = retrying(3, || { + let n = calls.fetch_add(1, Ordering::SeqCst); + async move { + if n < 2 { + Err(api_error(429)) + } else { + Ok("ok") + } + } + }) + .await; + assert_eq!(result.unwrap(), "ok"); + assert_eq!(calls.load(Ordering::SeqCst), 3); + } + + #[tokio::test(start_paused = true)] + async fn exhausts_retries_and_returns_last_error() { + let calls = AtomicU32::new(0); + let result: Result<(), _> = retrying(2, || { + calls.fetch_add(1, Ordering::SeqCst); + async { Err(api_error(503)) } + }) + .await; + assert!(result.is_err()); + assert_eq!(calls.load(Ordering::SeqCst), 3); // 1 attempt + 2 retries + } + + #[tokio::test(start_paused = true)] + async fn non_retryable_fails_on_first_attempt() { + let calls = AtomicU32::new(0); + let result: Result<(), _> = retrying(3, || { + calls.fetch_add(1, Ordering::SeqCst); + async { Err(api_error(404)) } + }) + .await; + assert!(result.is_err()); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } + + #[tokio::test(start_paused = true)] + async fn zero_retries_means_single_attempt() { + let calls = AtomicU32::new(0); + let result: Result<(), _> = retrying(0, || { + calls.fetch_add(1, Ordering::SeqCst); + async { Err(api_error(429)) } + }) + .await; + assert!(result.is_err()); + assert_eq!(calls.load(Ordering::SeqCst), 1); + } +} diff --git a/tests/retry.rs b/tests/retry.rs new file mode 100644 index 0000000..b87ca76 --- /dev/null +++ b/tests/retry.rs @@ -0,0 +1,111 @@ +//! Retry behavior: read-only commands retry transient failures with backoff; +//! mutating commands never retry. + +mod common; + +use common::run_qn; +use serde_json::json; +use wiremock::matchers::{body_json, method, path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +fn endpoints_ok() -> ResponseTemplate { + ResponseTemplate::new(200).set_body_json(json!({ + "data": [], + "pagination": { "total": 0, "limit": 20, "offset": 0 } + })) +} + +#[tokio::test] +async fn read_retries_a_429_then_succeeds() { + let server = MockServer::start().await; + // First request 429s; the mock then expires and the 200 mock takes over. + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .respond_with(ResponseTemplate::new(429)) + .up_to_n_times(1) + .expect(1) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .respond_with(endpoints_ok()) + .expect(1) + .mount(&server) + .await; + + let out = run_qn(&server.uri(), &["endpoint", "list"]).await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn read_exhausts_retries_on_persistent_429() { + let server = MockServer::start().await; + // --retries 1 → exactly 2 requests (initial + 1 retry), then exit 2. + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .respond_with(ResponseTemplate::new(429)) + .expect(2) + .mount(&server) + .await; + + let out = run_qn(&server.uri(), &["--retries", "1", "endpoint", "list"]).await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn retries_0_fails_on_first_429() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v0/endpoints")) + .respond_with(ResponseTemplate::new(429)) + .expect(1) + .mount(&server) + .await; + + let out = run_qn(&server.uri(), &["--retries", "0", "endpoint", "list"]).await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn read_does_not_retry_a_404() { + let server = MockServer::start().await; + Mock::given(method("GET")) + .and(path("/v0/endpoints/ep-missing")) + .respond_with(ResponseTemplate::new(404).set_body_json(json!({"error": "not found"}))) + .expect(1) + .mount(&server) + .await; + + let out = run_qn(&server.uri(), &["endpoint", "show", "ep-missing"]).await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn create_is_never_retried_even_on_500() { + let server = MockServer::start().await; + // A mutating POST must hit the server exactly once: with no idempotency + // keys, a retried create could provision (and bill) twice. + Mock::given(method("POST")) + .and(path("/v0/endpoints")) + .and(body_json( + json!({ "chain": "ethereum", "network": "mainnet" }), + )) + .respond_with(ResponseTemplate::new(500)) + .expect(1) + .mount(&server) + .await; + + let out = run_qn( + &server.uri(), + &[ + "endpoint", + "create", + "--chain", + "ethereum", + "--network", + "mainnet", + ], + ) + .await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); +} From b985a86cf604ba08d9e529cedebf4e4ba116bdfb Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Wed, 10 Jun 2026 16:12:03 -0400 Subject: [PATCH 3/5] Remove account-wide delete-all verbs; gate remaining destructive ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qn stream delete-all and qn webhook delete-all are gone. A one-line account-wide wipe is a disproportionate blast radius for a CLI command; that operation belongs behind the API where callers script it deliberately. Newly confirmation-gated (Mild: --yes skips, TTY prompts, scripts get exit 5 before any request is sent): - endpoint bulk pause — prompt includes the endpoint count - endpoint security token delete — notes that clients lose access - endpoint rate-limit delete-override — notes the revert to plan limits bulk resume stays ungated (it restores service). Existing prompts now state the blast radius (archive is irreversible from the CLI, tag delete is account-wide). The kv-local confirm_mild helper moved to confirm::confirm_mild for reuse. Severity::Severe and prompt_typed remain for future wide-blast-radius gates. CLAUDE.md now records the severity policy; README documents the confirmation model. Tests: each gated command verifies exit 5 with zero requests reaching the mock without --yes, and exit 0 with it. --- CLAUDE.md | 8 +++- README.md | 19 ++++++-- src/cli.rs | 2 +- src/commands/endpoint/bulk.rs | 20 ++++++++ src/commands/endpoint/mod.rs | 5 +- src/commands/endpoint/ratelimit.rs | 7 +++ src/commands/endpoint/security.rs | 7 +++ src/commands/endpoint/tag.rs | 2 +- src/commands/kv/actions.rs | 5 +- src/commands/kv/mod.rs | 17 ------- src/commands/stream/actions.rs | 24 ++-------- src/commands/stream/mod.rs | 3 -- src/commands/webhook/actions.rs | 24 ++-------- src/commands/webhook/mod.rs | 3 -- src/confirm.rs | 30 +++++++++++- src/retry.rs | 14 +++--- tests/admin_extra.rs | 31 +++++++++++- tests/endpoint.rs | 77 ++++++++++++++++++++++++++++++ tests/retry.rs | 4 +- tests/streams.rs | 18 ++----- tests/webhooks.rs | 14 ++---- 21 files changed, 223 insertions(+), 111 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ee240ac..050e020 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,7 +73,13 @@ Things to verify for each new endpoint: - **Negative numbers**: any `i64` flag that accepts `-1` (`--end`, etc.) needs `#[arg(long, allow_hyphen_values = true)]` or clap will read it as another flag. - **Multi-value flags**: prefer repeatable `--method foo --method bar` (clap `Vec` with `#[arg(long = "method")]`). Optionally also accept `--methods foo,bar` via a second field with `value_delimiter = ','`. The command body extends one into the other. - **Large `Args` structs**: clippy's `large_enum_variant` will reject an enum variant whose payload is > ~200 bytes. Box it: `Create(Box)` (we hit this on `StreamCmd::Create` and `WebhookCmd::Create`). -- **Destructive operations**: route through `confirm::decide_without_prompt(Severity::Mild|Severe, ConfirmCfg)`. Mild = single `--yes` skips, TTY prompts otherwise. Severe = `--yes --yes` skips OR user types a literal word (`delete-all`). Non-TTY without enough `--yes` returns `CliError::NeedsConfirmation` (exit 5). +- **Destructive operations**: the policy is non-negotiable — + - Anything that deletes, revokes, or loosens protection (delete/archive verbs, token revocation, removing a rate-limit override, pausing many endpoints) gets **at least `Severity::Mild`**: single `--yes` skips, TTY prompts otherwise, non-TTY without `--yes` returns `CliError::NeedsConfirmation` (exit 5). Use the `confirm::confirm_mild(&ctx, msg)` helper. + - Prompts must name the resource and the blast radius: "Pause 47 endpoint(s)? They will stop serving requests", not "Are you sure?". Include counts for bulk operations. + - **Account-wide wipe verbs (delete-all style) are not offered by the CLI at all.** Don't add one; point users at the API instead. + - Commands that restore service (`resume`, `activate`) are not gated. + - `Severity::Severe` (typed-word + `--yes --yes`) exists in `confirm.rs` for future wide-blast-radius operations; nothing uses it today. + - Every gated command needs both tests: no `--yes` non-TTY ⇒ exit 5 **and zero requests reach the mock** (`.expect(0)`); `--yes` ⇒ exit 0. - **Args that span multiple subcommands**: for things like rate-limits where both account-level and method-level CRUD exists, use a single `RateLimitCmd` enum with `MethodList/MethodCreate/...` variants — don't fight clap by trying to nest a third level. ### 3. Module layout diff --git a/README.md b/README.md index 252c69d..4ce86d7 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,17 @@ CLI hands the key to the SDK explicitly; it does not read the SDK's The hidden `--base-url ` flag overrides the API host for all four sub-clients at once (used for integration tests and on-prem mirrors). +## Confirmations + +Destructive commands (`delete`, `archive`, `bulk pause`, token revocation, +removing a rate-limit override, …) prompt before acting, and the prompt states +what will happen ("Pause 3 endpoint(s)? They will stop serving requests"). +Pass `--yes`/`-y` to skip the prompt. In scripts and CI (no TTY), a gated +command without `--yes` exits with code 5 **before** any request is sent. + +The CLI deliberately has no account-wide wipe commands (no `delete-all`); +operations with that blast radius belong behind the API, not a one-liner. + ## Retries Read-only commands (`list`, `show`, `logs`, `metrics`, `usage`, …) retry @@ -220,9 +231,9 @@ exponential backoff and full jitter. The default is 3 retries; tune it with the global `--retries ` flag (`--retries 0` disables). Commands that modify resources (`create`, `update`, `delete`, `pause`, …) -**never** retry automatically: the API has no idempotency keys yet, so a -retried create could provision twice. If a mutation fails with a transient -error, check whether it took effect before re-running it. +**never** retry automatically: a retried create could provision twice. If a +mutation fails with a transient error, check whether it took effect before +re-running it. ## Exit codes @@ -233,7 +244,7 @@ error, check whether it took effect before re-running it. | 2 | API error (server returned 4xx/5xx) | | 3 | Network failure (timeout, connect, transport) | | 4 | Missing or invalid API key / config | -| 5 | Operation needs confirmation (pass `--yes` or `--yes --yes`) | +| 5 | Operation needs confirmation (pass `--yes`) | | 130 | Interrupted (SIGINT) | ## License diff --git a/src/cli.rs b/src/cli.rs index 0521324..db8ca01 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -70,7 +70,7 @@ pub struct Cli { #[arg(long, global = true, default_value_t = 3, value_name = "N")] pub retries: u32, - /// Skip confirmation prompts. Pass twice for destructive bulk operations like `stream delete-all`. + /// Skip confirmation prompts on destructive operations. #[arg(short = 'y', long = "yes", global = true, action = ArgAction::Count)] pub yes: u8, diff --git a/src/commands/endpoint/bulk.rs b/src/commands/endpoint/bulk.rs index 79128a0..0e08739 100644 --- a/src/commands/endpoint/bulk.rs +++ b/src/commands/endpoint/bulk.rs @@ -13,6 +13,7 @@ use quicknode_sdk::admin::{ }; use serde::Serialize; +use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, set_header_bold, write_table, OutputCtx, Render}; @@ -73,6 +74,25 @@ async fn set_status(a: IdsArgs, status: &str, ctx: Ctx) -> Result<(), CliError> if a.ids.is_empty() { return Err(CliError::Arg("supply at least one endpoint id".to_string())); } + // Pausing a fleet stops it serving requests — confirm with the blast + // radius spelled out. Resuming restores service, so it stays unguarded. + if status == "paused" { + let cfg = ConfirmCfg::new( + ctx.global.yes_count, + ctx.global.no_input, + ctx.out.stdout_is_tty, + ); + let proceed = match decide_without_prompt(Severity::Mild, cfg)? { + true => true, + false => prompt_yes_no(&format!( + "Pause {} endpoint(s)? They will stop serving requests", + a.ids.len() + ))?, + }; + if !proceed { + return Err(CliError::Cancelled); + } + } let req = BulkUpdateEndpointStatusRequest { ids: a.ids, status: status.to_string(), diff --git a/src/commands/endpoint/mod.rs b/src/commands/endpoint/mod.rs index 0145544..c9e3098 100644 --- a/src/commands/endpoint/mod.rs +++ b/src/commands/endpoint/mod.rs @@ -281,7 +281,10 @@ async fn archive(a: ArchiveArgs, ctx: Ctx) -> Result<(), CliError> { ); let proceed = match decide_without_prompt(Severity::Mild, cfg)? { true => true, - false => prompt_yes_no(&format!("Archive endpoint {}?", a.id))?, + false => prompt_yes_no(&format!( + "Archive endpoint {}? This cannot be undone from the CLI", + a.id + ))?, }; if !proceed { return Err(CliError::Cancelled); diff --git a/src/commands/endpoint/ratelimit.rs b/src/commands/endpoint/ratelimit.rs index 8530c16..4904618 100644 --- a/src/commands/endpoint/ratelimit.rs +++ b/src/commands/endpoint/ratelimit.rs @@ -100,6 +100,13 @@ pub async fn run(cmd: RateLimitCmd, ctx: Ctx) -> Result<(), CliError> { RateLimitCmd::Get { id } => get(&id, ctx).await, RateLimitCmd::Set(a) => set(a, ctx).await, RateLimitCmd::DeleteOverride { id, override_id } => { + crate::confirm::confirm_mild( + &ctx, + &format!( + "Delete rate-limit override {override_id} on {id}? \ + The endpoint reverts to its plan defaults" + ), + )?; ctx.sdk .admin .delete_rate_limit_override(&id, &override_id) diff --git a/src/commands/endpoint/security.rs b/src/commands/endpoint/security.rs index 6ba9eba..1a6e81f 100644 --- a/src/commands/endpoint/security.rs +++ b/src/commands/endpoint/security.rs @@ -11,6 +11,7 @@ use quicknode_sdk::admin::{ }; use serde::Serialize; +use crate::confirm::confirm_mild; use crate::context::Ctx; use crate::errors::CliError; use crate::output::{new_table, opt_cell, set_header_bold, write_table, Render}; @@ -265,6 +266,12 @@ async fn token(cmd: TokenCmd, ctx: Ctx) -> Result<(), CliError> { ctx.out.note(&format!("✓ Created token on {id}")); } TokenCmd::Delete { id, token_id } => { + confirm_mild( + &ctx, + &format!( + "Delete token {token_id} on {id}? Clients authenticating with it lose access" + ), + )?; ctx.sdk.admin.delete_token(&id, &token_id).await?; ctx.out.note(&format!("✓ Deleted token {token_id} on {id}")); } diff --git a/src/commands/endpoint/tag.rs b/src/commands/endpoint/tag.rs index 262154a..67b0773 100644 --- a/src/commands/endpoint/tag.rs +++ b/src/commands/endpoint/tag.rs @@ -79,7 +79,7 @@ async fn delete(tag_id: i32, ctx: Ctx) -> Result<(), CliError> { ); let proceed = match decide_without_prompt(Severity::Mild, cfg)? { true => true, - false => prompt_yes_no(&format!("Delete tag {tag_id}?"))?, + false => prompt_yes_no(&format!("Delete tag {tag_id} from the account?"))?, }; if !proceed { return Err(CliError::Cancelled); diff --git a/src/commands/kv/actions.rs b/src/commands/kv/actions.rs index b7b0c55..3534ebe 100644 --- a/src/commands/kv/actions.rs +++ b/src/commands/kv/actions.rs @@ -9,6 +9,7 @@ use quicknode_sdk::{ use super::render::{ListView, ListsView, SetsView}; use super::{ListCmd, SetCmd}; +use crate::confirm::confirm_mild; use crate::context::Ctx; use crate::errors::CliError; use crate::retry::retrying; @@ -47,7 +48,7 @@ pub(super) async fn set(cmd: SetCmd, ctx: Ctx) -> Result<(), CliError> { crate::output::emit(&ctx.out, &SetsView(resp))?; } SetCmd::Delete { key } => { - super::confirm_mild(&ctx, &format!("Delete set {key:?}?"))?; + confirm_mild(&ctx, &format!("Delete set {key:?}?"))?; ctx.sdk.kvstore.delete_set(&key).await?; ctx.out.note(&format!("✓ Deleted set {key:?}")); } @@ -160,7 +161,7 @@ pub(super) async fn list(cmd: ListCmd, ctx: Ctx) -> Result<(), CliError> { ctx.out.note(&format!("✓ Updated list {:?}", a.key)); } ListCmd::Delete { key } => { - super::confirm_mild(&ctx, &format!("Delete list {key:?}?"))?; + confirm_mild(&ctx, &format!("Delete list {key:?}?"))?; ctx.sdk.kvstore.delete_list(&key).await?; ctx.out.note(&format!("✓ Deleted list {key:?}")); } diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index 1ec815f..834c5f5 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -7,7 +7,6 @@ use std::io::{IsTerminal, Read}; use clap::{Args as ClapArgs, Subcommand}; -use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; @@ -114,22 +113,6 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { } } -fn confirm_mild(ctx: &Ctx, prompt: &str) -> Result<(), CliError> { - let cfg = ConfirmCfg::new( - ctx.global.yes_count, - ctx.global.no_input, - ctx.out.stdout_is_tty, - ); - let proceed = match decide_without_prompt(Severity::Mild, cfg)? { - true => true, - false => prompt_yes_no(prompt)?, - }; - if !proceed { - return Err(CliError::Cancelled); - } - Ok(()) -} - fn read_stdin() -> Result { if std::io::stdin().is_terminal() { return Err(CliError::Arg( diff --git a/src/commands/stream/actions.rs b/src/commands/stream/actions.rs index c06e908..c7e292b 100644 --- a/src/commands/stream/actions.rs +++ b/src/commands/stream/actions.rs @@ -9,7 +9,7 @@ use quicknode_sdk::streams::{ use super::render::{StreamView, StreamsListView, TestFilterView}; use super::{CreateArgs, ListArgs, TestFilterArgs, UpdateArgs}; -use crate::confirm::{decide_without_prompt, prompt_typed, prompt_yes_no, ConfirmCfg, Severity}; +use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; use crate::retry::retrying; @@ -148,26 +148,8 @@ pub(super) async fn delete(id: &str, ctx: Ctx) -> Result<(), CliError> { Ok(()) } -pub(super) async fn delete_all(ctx: Ctx) -> Result<(), CliError> { - let cfg = ConfirmCfg::new( - ctx.global.yes_count, - ctx.global.no_input, - ctx.out.stdout_is_tty, - ); - let proceed = match decide_without_prompt(Severity::Severe, cfg)? { - true => true, - false => prompt_typed( - "Type 'delete-all' to delete EVERY stream on the account", - "delete-all", - )?, - }; - if !proceed { - return Err(CliError::Cancelled); - } - ctx.sdk.streams.delete_all_streams().await?; - ctx.out.note("✓ Deleted all streams"); - Ok(()) -} +// There is intentionally no `stream delete-all` command. Account-wide wipes +// are out of scope for the CLI; use the API directly if you really need one. pub(super) async fn activate(id: &str, ctx: Ctx) -> Result<(), CliError> { ctx.sdk.streams.activate_stream(id).await?; diff --git a/src/commands/stream/mod.rs b/src/commands/stream/mod.rs index 35bffa0..4277870 100644 --- a/src/commands/stream/mod.rs +++ b/src/commands/stream/mod.rs @@ -36,8 +36,6 @@ pub enum StreamCmd { Update(UpdateArgs), /// Delete a stream. Delete { id: String }, - /// Delete every stream on the account. - DeleteAll, /// Activate (resume) a stream. Activate { id: String }, /// Pause a stream. @@ -273,7 +271,6 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { StreamCmd::Show { id } => actions::show(&id, ctx).await, StreamCmd::Update(a) => actions::update(a, ctx).await, StreamCmd::Delete { id } => actions::delete(&id, ctx).await, - StreamCmd::DeleteAll => actions::delete_all(ctx).await, StreamCmd::Activate { id } => actions::activate(&id, ctx).await, StreamCmd::Pause { id } => actions::pause(&id, ctx).await, StreamCmd::TestFilter(a) => actions::test_filter(a, ctx).await, diff --git a/src/commands/webhook/actions.rs b/src/commands/webhook/actions.rs index 16e423e..9fa81c6 100644 --- a/src/commands/webhook/actions.rs +++ b/src/commands/webhook/actions.rs @@ -12,7 +12,7 @@ use quicknode_sdk::webhooks::{ use super::render::{WebhookView, WebhooksListView}; use super::{ActivateArgs, CreateArgs, ListArgs, TemplateKind, UpdateArgs, UpdateTemplateArgs}; -use crate::confirm::{decide_without_prompt, prompt_typed, prompt_yes_no, ConfirmCfg, Severity}; +use crate::confirm::{decide_without_prompt, prompt_yes_no, ConfirmCfg, Severity}; use crate::context::Ctx; use crate::errors::CliError; use crate::retry::retrying; @@ -147,26 +147,8 @@ pub(super) async fn delete(id: &str, ctx: Ctx) -> Result<(), CliError> { Ok(()) } -pub(super) async fn delete_all(ctx: Ctx) -> Result<(), CliError> { - let cfg = ConfirmCfg::new( - ctx.global.yes_count, - ctx.global.no_input, - ctx.out.stdout_is_tty, - ); - let proceed = match decide_without_prompt(Severity::Severe, cfg)? { - true => true, - false => prompt_typed( - "Type 'delete-all' to delete EVERY webhook on the account", - "delete-all", - )?, - }; - if !proceed { - return Err(CliError::Cancelled); - } - ctx.sdk.webhooks.delete_all_webhooks().await?; - ctx.out.note("✓ Deleted all webhooks"); - Ok(()) -} +// There is intentionally no `webhook delete-all` command. Account-wide wipes +// are out of scope for the CLI; use the API directly if you really need one. pub(super) async fn activate(a: ActivateArgs, ctx: Ctx) -> Result<(), CliError> { let params = ActivateWebhookParams { diff --git a/src/commands/webhook/mod.rs b/src/commands/webhook/mod.rs index 53c4b8b..c36fc92 100644 --- a/src/commands/webhook/mod.rs +++ b/src/commands/webhook/mod.rs @@ -32,8 +32,6 @@ pub enum WebhookCmd { UpdateTemplate(Box), /// Delete a webhook. Delete { id: String }, - /// Delete every webhook on the account. - DeleteAll, /// Activate a webhook (resume delivery). Activate(ActivateArgs), /// Pause a webhook. @@ -186,7 +184,6 @@ pub async fn run(args: Args, ctx: Ctx) -> Result<(), CliError> { WebhookCmd::Update(a) => actions::update(a, ctx).await, WebhookCmd::UpdateTemplate(a) => actions::update_template(*a, ctx).await, WebhookCmd::Delete { id } => actions::delete(&id, ctx).await, - WebhookCmd::DeleteAll => actions::delete_all(ctx).await, WebhookCmd::Activate(a) => actions::activate(a, ctx).await, WebhookCmd::Pause { id } => actions::pause(&id, ctx).await, WebhookCmd::EnabledCount => actions::enabled_count(ctx).await, diff --git a/src/confirm.rs b/src/confirm.rs index f6a6f76..6c37700 100644 --- a/src/confirm.rs +++ b/src/confirm.rs @@ -2,7 +2,8 @@ //! //! Two levels: //! - **mild** (e.g. `endpoint archive`, `endpoint tag delete`): one y/N prompt; --yes skips. -//! - **severe** (e.g. `stream delete-all`): typed-word confirmation; --yes --yes (twice) skips. +//! - **severe** (reserved for future wide-blast-radius operations): typed-word +//! confirmation; --yes --yes (twice) skips. //! //! On a non-TTY, mild requires `--yes` and severe requires `--yes --yes` — never //! auto-confirm in scripts. @@ -13,6 +14,10 @@ use crate::errors::CliError; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Severity { Mild, + /// Typed-word confirmation; `--yes --yes` to skip. No command uses this + /// today — account-wide wipe verbs were removed from the CLI — but the + /// gating stays for future wide-blast-radius operations. + #[allow(dead_code)] Severe, } @@ -54,6 +59,25 @@ pub fn decide_without_prompt(severity: Severity, cfg: ConfirmCfg) -> Result Result<(), CliError> { + let cfg = ConfirmCfg::new( + ctx.global.yes_count, + ctx.global.no_input, + ctx.out.stdout_is_tty, + ); + let proceed = match decide_without_prompt(Severity::Mild, cfg)? { + true => true, + false => prompt_yes_no(message)?, + }; + if !proceed { + return Err(CliError::Cancelled); + } + Ok(()) +} + /// Interactive y/N prompt. Returns true on yes. pub fn prompt_yes_no(message: &str) -> Result { use dialoguer::Confirm; @@ -65,7 +89,9 @@ pub fn prompt_yes_no(message: &str) -> Result { } /// Interactive typed-word confirmation. Returns true if the user types -/// `expected` exactly. +/// `expected` exactly. Pairs with [`Severity::Severe`]; kept for the same +/// reason. +#[allow(dead_code)] pub fn prompt_typed(message: &str, expected: &str) -> Result { use dialoguer::Input; let typed: String = Input::new() diff --git a/src/retry.rs b/src/retry.rs index 523eaa3..77dcb2c 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -1,13 +1,13 @@ //! Full-jitter exponential backoff for read-only API calls. //! -//! Mutating commands must NOT use this. The API has no idempotency keys, so a -//! retried create/update/delete can be applied twice (duplicate endpoints, -//! double tag writes, …). Reads are safe to repeat, and the backend is -//! rate-limited, so scripted reads need to survive the occasional 429. +//! Mutating commands must NOT use this: a retried create/update/delete can be +//! applied twice (duplicate endpoints, double tag writes, …). Reads are safe +//! to repeat, and the backend is rate-limited, so scripted reads need to +//! survive the occasional 429. //! -//! `Retry-After` is not honored yet: `SdkError::Api` carries only the status -//! and body, not response headers. Honoring it needs a quicknode-sdk change; -//! until then full jitter keeps concurrent callers from herding. +//! Backoff is driven purely by full jitter (`SdkError::Api` exposes the +//! status and body, which is what the retry decision is based on); the +//! randomized window keeps concurrent callers from herding. use std::future::Future; use std::time::Duration; diff --git a/tests/admin_extra.rs b/tests/admin_extra.rs index d6df670..ceffca0 100644 --- a/tests/admin_extra.rs +++ b/tests/admin_extra.rs @@ -222,12 +222,41 @@ async fn bulk_status_paused() { .await; let out = run_qn( &server.uri(), - &["endpoint", "bulk", "pause", "ep-1", "ep-2"], + &["endpoint", "bulk", "pause", "ep-1", "ep-2", "--yes"], ) .await; assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); } +#[tokio::test] +async fn bulk_pause_without_yes_needs_confirmation_and_sends_nothing() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v0/endpoints/bulk/status")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; + let out = run_qn(&server.uri(), &["endpoint", "bulk", "pause", "ep-1"]).await; + assert_eq!(out.exit_code, 5, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn bulk_resume_does_not_need_confirmation() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/v0/endpoints/bulk/status")) + .and(body_json(json!({ "ids": ["ep-1"], "status": "active" }))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "data": { "total": 1, "updated_count": 1, "failed_count": 0, "results": [] } + }))) + .expect(1) + .mount(&server) + .await; + let out = run_qn(&server.uri(), &["endpoint", "bulk", "resume", "ep-1"]).await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + #[tokio::test] async fn bulk_tag_add() { let server = MockServer::start().await; diff --git a/tests/endpoint.rs b/tests/endpoint.rs index 50e9b01..cff12ae 100644 --- a/tests/endpoint.rs +++ b/tests/endpoint.rs @@ -403,6 +403,83 @@ async fn endpoint_security_token_create() { assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); } +#[tokio::test] +async fn endpoint_security_token_delete_with_yes() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v0/endpoints/ep-1/security/tokens/tok-1")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ "data": true }))) + .expect(1) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &[ + "endpoint", "security", "token", "delete", "ep-1", "tok-1", "--yes", + ], + ) + .await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn endpoint_security_token_delete_without_yes_sends_nothing() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v0/endpoints/ep-1/security/tokens/tok-1")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &["endpoint", "security", "token", "delete", "ep-1", "tok-1"], + ) + .await; + assert_eq!(out.exit_code, 5, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn rate_limit_delete_override_with_yes() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v0/endpoints/ep-1/rate-limits/ovr-1")) + .respond_with(ResponseTemplate::new(200)) + .expect(1) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &[ + "endpoint", + "rate-limit", + "delete-override", + "ep-1", + "ovr-1", + "--yes", + ], + ) + .await; + assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); +} + +#[tokio::test] +async fn rate_limit_delete_override_without_yes_sends_nothing() { + let server = MockServer::start().await; + Mock::given(method("DELETE")) + .and(path("/v0/endpoints/ep-1/rate-limits/ovr-1")) + .respond_with(ResponseTemplate::new(200)) + .expect(0) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &["endpoint", "rate-limit", "delete-override", "ep-1", "ovr-1"], + ) + .await; + assert_eq!(out.exit_code, 5, "stderr={}", out.stderr); +} + #[tokio::test] async fn endpoint_security_set_options_sends_partial_payload() { let server = MockServer::start().await; diff --git a/tests/retry.rs b/tests/retry.rs index b87ca76..20b79af 100644 --- a/tests/retry.rs +++ b/tests/retry.rs @@ -83,8 +83,8 @@ async fn read_does_not_retry_a_404() { #[tokio::test] async fn create_is_never_retried_even_on_500() { let server = MockServer::start().await; - // A mutating POST must hit the server exactly once: with no idempotency - // keys, a retried create could provision (and bill) twice. + // A mutating POST must hit the server exactly once: a retried create + // could provision (and bill) twice. Mock::given(method("POST")) .and(path("/v0/endpoints")) .and(body_json( diff --git a/tests/streams.rs b/tests/streams.rs index 8e31abd..ddbdc48 100644 --- a/tests/streams.rs +++ b/tests/streams.rs @@ -172,21 +172,11 @@ async fn activate_pause_delete_stream() { } #[tokio::test] -async fn delete_all_streams_needs_double_yes() { +async fn delete_all_is_not_a_command() { + // Account-wide wipes are deliberately not offered by the CLI. let server = MockServer::start().await; - Mock::given(method("DELETE")) - .and(path("/streams/rest/v1/streams")) - .respond_with(ResponseTemplate::new(200)) - .mount(&server) - .await; - - // Single --yes is not enough for severe. - let one_yes = run_qn(&server.uri(), &["stream", "delete-all", "--yes"]).await; - assert_eq!(one_yes.exit_code, 5, "stderr={}", one_yes.stderr); - - // --yes --yes proceeds. - let two_yes = run_qn(&server.uri(), &["stream", "delete-all", "--yes", "--yes"]).await; - assert_eq!(two_yes.exit_code, 0, "stderr={}", two_yes.stderr); + let out = run_qn(&server.uri(), &["stream", "delete-all", "--yes", "--yes"]).await; + assert_ne!(out.exit_code, 0); } #[tokio::test] diff --git a/tests/webhooks.rs b/tests/webhooks.rs index be7f0f2..1728d43 100644 --- a/tests/webhooks.rs +++ b/tests/webhooks.rs @@ -147,17 +147,11 @@ async fn pause_activate_delete_webhook() { } #[tokio::test] -async fn delete_all_webhooks_needs_double_yes() { +async fn delete_all_is_not_a_command() { + // Account-wide wipes are deliberately not offered by the CLI. let server = MockServer::start().await; - Mock::given(method("DELETE")) - .and(path("/webhooks/rest/v1/webhooks")) - .respond_with(ResponseTemplate::new(200)) - .mount(&server) - .await; - let one = run_qn(&server.uri(), &["webhook", "delete-all", "--yes"]).await; - assert_eq!(one.exit_code, 5); - let two = run_qn(&server.uri(), &["webhook", "delete-all", "-y", "-y"]).await; - assert_eq!(two.exit_code, 0, "stderr={}", two.stderr); + let out = run_qn(&server.uri(), &["webhook", "delete-all", "-y", "-y"]).await; + assert_ne!(out.exit_code, 0); } #[tokio::test] From 00ad0b33f18ea5e531ef06d9dc3995eca33ff240 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Wed, 10 Jun 2026 16:16:46 -0400 Subject: [PATCH 4/5] Truthful help and exit codes; grouped global flags; examples - Required flags are now clap-enforced and shown in usage lines: endpoint create (--chain/--network), endpoint update (--label), and the six stream create flags via required_unless_present(config_file). Missing-flag errors now list everything missing at once instead of failing one runtime check at a time. - clap usage errors (typo'd subcommand, missing required flag) exit 1, matching runtime argument errors, so exit 2 always and only means "the API returned an error". --help/--version still exit 0. - Global flags are grouped under a "Global options" heading in every subcommand's --help, so command-specific flags surface first. - after_help examples on the top level and on endpoint/stream/webhook/ kv/auth. - README exit-code table updated. --- README.md | 2 +- src/cli.rs | 11 +++++++++- src/commands/auth.rs | 5 +++++ src/commands/endpoint/mod.rs | 38 ++++++++++++++-------------------- src/commands/kv/mod.rs | 5 +++++ src/commands/stream/actions.rs | 31 +++++++++------------------ src/commands/stream/mod.rs | 27 +++++++++++++++++------- src/commands/webhook/mod.rs | 5 +++++ src/errors.rs | 3 ++- src/main.rs | 16 +++++++++++++- tests/common/mod.rs | 3 ++- tests/endpoint.rs | 21 +++++++++++++++---- 12 files changed, 106 insertions(+), 61 deletions(-) diff --git a/README.md b/README.md index 4ce86d7..6bffb86 100644 --- a/README.md +++ b/README.md @@ -240,7 +240,7 @@ re-running it. | Code | Meaning | |---|---| | 0 | Success | -| 1 | CLI error (bad argument, IO, decode) | +| 1 | CLI error (usage/bad argument, IO, decode) | | 2 | API error (server returned 4xx/5xx) | | 3 | Network failure (timeout, connect, transport) | | 4 | Missing or invalid API key / config | diff --git a/src/cli.rs b/src/cli.rs index db8ca01..8eb9097 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -25,7 +25,16 @@ use crate::output::Format; (--config-file path if given, else ~/.config/qn/config.toml). Run `qn auth login`\n\ to save a key the first time.", propagate_version = true, - disable_help_subcommand = true + disable_help_subcommand = true, + after_help = "Examples:\n \ + qn auth login\n \ + qn endpoint create --chain ethereum --network mainnet\n \ + qn endpoint list -o json\n \ + qn endpoint logs ep-1234 --from 1h\n \ + qn chain list", + // Group the global flags under their own heading in every subcommand's + // --help, so command-specific flags surface first under "Options". + next_help_heading = "Global options" )] pub struct Cli { /// API key. Overrides the config file. diff --git a/src/commands/auth.rs b/src/commands/auth.rs index 0e3d73f..8da4e30 100644 --- a/src/commands/auth.rs +++ b/src/commands/auth.rs @@ -16,6 +16,11 @@ use crate::context::GlobalArgs; use crate::errors::CliError; #[derive(Debug, ClapArgs)] +#[command(after_help = "Examples:\n \ + qn auth login # prompts for the key (hidden input)\n \ + qn auth login --api-key # non-interactive (e.g. CI)\n \ + qn auth whoami # verify the key against the API\n \ + qn --config-file ./ci.toml auth status")] pub struct Args { #[command(subcommand)] pub cmd: AuthCmd, diff --git a/src/commands/endpoint/mod.rs b/src/commands/endpoint/mod.rs index c9e3098..f3c93ca 100644 --- a/src/commands/endpoint/mod.rs +++ b/src/commands/endpoint/mod.rs @@ -24,6 +24,12 @@ pub use security::SecurityCmd; pub use tag::TagCmd; #[derive(Debug, ClapArgs)] +#[command(after_help = "Examples:\n \ + qn endpoint create --chain ethereum --network mainnet\n \ + qn endpoint list -o json\n \ + qn endpoint logs ep-1234 --from 1h --details\n \ + qn endpoint pause ep-1234\n \ + qn endpoint bulk pause ep-1 ep-2 --yes")] pub struct Args { #[command(subcommand)] pub cmd: EndpointCmd, @@ -123,12 +129,12 @@ pub struct ListArgs { #[derive(Debug, ClapArgs)] pub struct CreateArgs { - /// Blockchain (e.g. `ethereum`, `solana`). + /// Blockchain (e.g. `ethereum`, `solana`; run `qn chain list` to see all). #[arg(long)] - pub chain: Option, + pub chain: String, /// Network (e.g. `mainnet`). #[arg(long)] - pub network: Option, + pub network: String, } #[derive(Debug, ClapArgs)] @@ -137,7 +143,7 @@ pub struct UpdateArgs { pub id: String, /// New label. #[arg(long)] - pub label: Option, + pub label: String, } #[derive(Debug, ClapArgs)] @@ -232,22 +238,9 @@ async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { } async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { - let missing: Vec<&str> = [ - ("--chain", a.chain.is_none()), - ("--network", a.network.is_none()), - ] - .iter() - .filter_map(|(name, missing)| if *missing { Some(*name) } else { None }) - .collect(); - if !missing.is_empty() { - return Err(CliError::Arg(format!( - "'endpoint create' requires {}. Run 'qn chain list' to see available chains.", - missing.join(" and "), - ))); - } let req = CreateEndpointRequest { - chain: a.chain, - network: a.network, + chain: Some(a.chain), + network: Some(a.network), }; let resp = ctx.sdk.admin.create_endpoint(&req).await?; ctx.out @@ -264,10 +257,9 @@ async fn show(id: &str, ctx: Ctx) -> Result<(), CliError> { } async fn update(a: UpdateArgs, ctx: Ctx) -> Result<(), CliError> { - if a.label.is_none() { - return Err(CliError::Arg("'endpoint update' requires --label.".into())); - } - let req = UpdateEndpointRequest { label: a.label }; + let req = UpdateEndpointRequest { + label: Some(a.label), + }; ctx.sdk.admin.update_endpoint(&a.id, &req).await?; ctx.out.note(&format!("✓ Updated endpoint {}", a.id)); Ok(()) diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index 834c5f5..c8c8459 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -11,6 +11,11 @@ use crate::context::Ctx; use crate::errors::CliError; #[derive(Debug, ClapArgs)] +#[command(after_help = "Examples:\n \ + qn kv set put mykey myvalue\n \ + qn kv set get mykey\n \ + qn kv list create mylist item1 item2\n \ + qn kv list contains mylist item1")] pub struct Args { #[command(subcommand)] pub cmd: KvCmd, diff --git a/src/commands/stream/actions.rs b/src/commands/stream/actions.rs index c7e292b..2c3d460 100644 --- a/src/commands/stream/actions.rs +++ b/src/commands/stream/actions.rs @@ -39,27 +39,16 @@ pub(super) async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { } fn build_create_params(a: CreateArgs) -> Result { - let name = a - .name - .ok_or_else(|| CliError::Arg("--name is required".into()))?; - let network = a - .network - .ok_or_else(|| CliError::Arg("--network is required".into()))?; - let dataset = a - .dataset - .ok_or_else(|| CliError::Arg("--dataset is required".into()))?; - let start = a - .start - .ok_or_else(|| CliError::Arg("--start is required".into()))?; - let end = a - .end - .ok_or_else(|| CliError::Arg("--end is required (-1 for continuous)".into()))?; - let region = a - .region - .ok_or_else(|| CliError::Arg("--region is required".into()))?; - let url = a.webhook.ok_or_else(|| { - CliError::Arg("--webhook is required (or use --config-file for other destinations)".into()) - })?; + // These flags are `required_unless_present = "config_file"` in clap, and + // this function is only reached when --config-file was NOT supplied. + const ENFORCED: &str = "enforced by clap unless --config-file is present"; + let name = a.name.expect(ENFORCED); + let network = a.network.expect(ENFORCED); + let dataset = a.dataset.expect(ENFORCED); + let start = a.start.expect(ENFORCED); + let end = a.end.expect(ENFORCED); + let region = a.region.expect(ENFORCED); + let url = a.webhook.expect(ENFORCED); let filter_function = match (a.filter, a.filter_file) { (Some(s), None) => Some(STANDARD.encode(s)), diff --git a/src/commands/stream/mod.rs b/src/commands/stream/mod.rs index 4277870..e5aa383 100644 --- a/src/commands/stream/mod.rs +++ b/src/commands/stream/mod.rs @@ -18,6 +18,12 @@ use crate::context::Ctx; use crate::errors::CliError; #[derive(Debug, ClapArgs)] +#[command(after_help = "Examples:\n \ + qn stream list --limit 20\n \ + qn stream create --name blocks --network ethereum-mainnet --dataset block \\\n \ + --start 24691804 --end=-1 --region usa-east --webhook https://hook.example.com\n \ + qn stream create --config-file stream.json\n \ + qn stream pause s-1234")] pub struct Args { #[command(subcommand)] pub cmd: StreamCmd, @@ -72,28 +78,33 @@ pub struct CreateArgs { pub config_file: Option, /// Stream name. - #[arg(long)] + #[arg(long, required_unless_present = "config_file")] pub name: Option, /// Network (e.g. `ethereum-mainnet`). - #[arg(long)] + #[arg(long, required_unless_present = "config_file")] pub network: Option, /// Dataset (snake_case). - #[arg(long, value_enum)] + #[arg(long, value_enum, required_unless_present = "config_file")] pub dataset: Option, /// Start block. - #[arg(long)] + #[arg(long, required_unless_present = "config_file")] pub start: Option, /// End block (`-1` for continuous). - #[arg(long, allow_hyphen_values = true)] + #[arg( + long, + allow_hyphen_values = true, + required_unless_present = "config_file" + )] pub end: Option, /// Region. - #[arg(long, value_enum)] + #[arg(long, value_enum, required_unless_present = "config_file")] pub region: Option, /// Billing plan slug (optional). #[arg(long)] pub plan: Option, - /// Webhook URL for the primary destination. - #[arg(long)] + /// Webhook URL for the primary destination (use `--config-file` for other + /// destination types). + #[arg(long, required_unless_present = "config_file")] pub webhook: Option, /// Webhook security token (32-byte minimum). Optional. #[arg(long)] diff --git a/src/commands/webhook/mod.rs b/src/commands/webhook/mod.rs index c36fc92..6f0edd2 100644 --- a/src/commands/webhook/mod.rs +++ b/src/commands/webhook/mod.rs @@ -12,6 +12,11 @@ use crate::context::Ctx; use crate::errors::CliError; #[derive(Debug, ClapArgs)] +#[command(after_help = "Examples:\n \ + qn webhook list\n \ + qn webhook create --name alerts --network ethereum-mainnet \\\n \ + --url https://hook.example.com --template evmWalletFilter --wallet 0xabc\n \ + qn webhook pause wh-1234")] pub struct Args { #[command(subcommand)] pub cmd: WebhookCmd, diff --git a/src/errors.rs b/src/errors.rs index 59eb792..2b43a1a 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -53,7 +53,8 @@ pub enum CliError { /// Maps a [`CliError`] to a process exit code per the plan. /// /// - 0: success (never produced here) -/// - 1: generic CLI failure (arg parse, IO, decode) +/// - 1: generic CLI failure (arg parse, IO, decode). clap usage errors are +/// mapped to 1 in main.rs too, so 2 always and only means an API error. /// - 2: SdkError::Api (server returned a non-2xx) /// - 3: SdkError::Http (network failure) /// - 4: NoApiKey / BadConfig diff --git a/src/main.rs b/src/main.rs index 49cab38..f993dbc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -8,7 +8,21 @@ use qn::errors::{exit_code_for, render}; #[tokio::main] async fn main() -> ExitCode { - let cli = Cli::parse(); + // Map clap usage errors to exit 1 — the same bucket as runtime argument + // errors — so exit 2 always and only means "the API returned an error". + // (clap's own Error::exit would use 2 for both.) + let cli = match Cli::try_parse() { + Ok(cli) => cli, + Err(e) => { + // --help/--version print to stdout and are not errors. + let _ = e.print(); + return if e.use_stderr() { + ExitCode::from(1) + } else { + ExitCode::SUCCESS + }; + } + }; let verbose = cli.verbose; match cli.run().await { Ok(()) => ExitCode::SUCCESS, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 8618171..d43f698 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -54,7 +54,8 @@ async fn run_qn_inner(base_url: &str, extra_args: &[&str], inject_key: bool) -> let cli = match Cli::try_parse_from(&argv) { Ok(c) => c, Err(e) => { - let exit = if e.use_stderr() { 2 } else { 0 }; + // Mirrors main.rs: usage errors are exit 1, help/version exit 0. + let exit = if e.use_stderr() { 1 } else { 0 }; return RunOutput { stdout: String::new(), stderr: e.to_string(), diff --git a/tests/endpoint.rs b/tests/endpoint.rs index cff12ae..10a0a41 100644 --- a/tests/endpoint.rs +++ b/tests/endpoint.rs @@ -566,12 +566,12 @@ async fn endpoint_ratelimit_set_omits_unset_fields() { assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); } -// ---- Stage B: required-args pre-flight checks ---- // +// ---- Required-args enforcement (clap-level, exits 1 before any HTTP) ---- // #[tokio::test] async fn endpoint_create_no_flags_fails_before_api_call() { // No mocks mounted; if the CLI tries to make a request, wiremock would 404. - // We assert that the pre-flight check fires *before* any HTTP call. + // clap rejects the invocation *before* any HTTP call. let server = MockServer::start().await; let out = run_qn(&server.uri(), &["endpoint", "create"]).await; assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); @@ -593,8 +593,8 @@ async fn endpoint_create_only_chain_fails_before_api_call() { .await; assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); assert!( - out.stderr.contains("--network") && !out.stderr.contains("--chain "), - "should call out --network specifically; stderr={}", + out.stderr.contains("--network "), + "should call out --network; stderr={}", out.stderr ); assert_eq!(server.received_requests().await.unwrap().len(), 0); @@ -609,6 +609,19 @@ async fn endpoint_update_no_flags_fails_before_api_call() { assert_eq!(server.received_requests().await.unwrap().len(), 0); } +#[tokio::test] +async fn stream_create_missing_flags_fails_before_api_call() { + let server = MockServer::start().await; + let out = run_qn(&server.uri(), &["stream", "create", "--name", "x"]).await; + assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); + assert!( + out.stderr.contains("--network") && out.stderr.contains("--webhook"), + "stderr={}", + out.stderr + ); + assert_eq!(server.received_requests().await.unwrap().len(), 0); +} + #[tokio::test] async fn endpoint_security_set_options_no_flags_fails_before_api_call() { let server = MockServer::start().await; From b9926529d2262574651f7290f28637e5d7947802 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Wed, 10 Jun 2026 16:39:07 -0400 Subject: [PATCH 5/5] Review fixes: rename stream --config-file, truthful retry docs - qn stream create's JSON-params flag is now --stream-config-file, so it can no longer shadow the global --config-file (CLI config TOML) when placed after the subcommand. Help text on both flags cross-references the distinction. - Retry docs (README + --retries help) now state the exact retried statuses (429/500/502/503/504) instead of claiming all 5xx, and the README notes that stream test-filter retries despite being a POST. - README environment section no longer claims NO_COLOR/TERM are the only honored variables (XDG_CONFIG_HOME/HOME locate the config file). - Trimmed an SDK-internals aside from the retry.rs module doc. --- README.md | 17 ++++++++++------- src/cli.rs | 4 ++-- src/commands/stream/actions.rs | 9 +++++---- src/commands/stream/mod.rs | 33 ++++++++++++++++++--------------- src/retry.rs | 5 ++--- 5 files changed, 37 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 6bffb86..f0dbba4 100644 --- a/README.md +++ b/README.md @@ -204,10 +204,11 @@ qn completions powershell > qn.ps1 ## Configuration via environment `qn` reads no API credentials from the environment (see -[Authentication](#authentication) for why). It honors the conventional -output-related variables only: `NO_COLOR` and `TERM=dumb` disable color. The -CLI hands the key to the SDK explicitly; it does not read the SDK's -`QN_SDK__*` environment namespace. +[Authentication](#authentication) for why). The conventional variables are +honored: `NO_COLOR` and `TERM=dumb` disable color, and +`XDG_CONFIG_HOME`/`HOME` locate the default config file. The CLI hands the +key to the SDK explicitly; it does not read the SDK's `QN_SDK__*` environment +namespace. The hidden `--base-url ` flag overrides the API host for all four sub-clients at once (used for integration tests and on-prem mirrors). @@ -226,9 +227,11 @@ operations with that blast radius belong behind the API, not a one-liner. ## Retries Read-only commands (`list`, `show`, `logs`, `metrics`, `usage`, …) retry -transient failures — HTTP 429/5xx, timeouts, connection errors — with -exponential backoff and full jitter. The default is 3 retries; tune it with -the global `--retries ` flag (`--retries 0` disables). +transient failures — HTTP 429, 500, 502, 503, 504, timeouts, and connection +errors — with exponential backoff and full jitter. The default is 3 retries; +tune it with the global `--retries ` flag (`--retries 0` disables). +`stream test-filter` retries too: it sends a POST, but only evaluates a +filter against historical data and changes nothing. Commands that modify resources (`create`, `update`, `delete`, `pause`, …) **never** retry automatically: a retried create could provision twice. If a diff --git a/src/cli.rs b/src/cli.rs index 8eb9097..0b15241 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -74,8 +74,8 @@ pub struct Cli { pub no_input: bool, /// Max automatic retries for read-only commands on transient failures - /// (HTTP 429/5xx, timeouts). Uses exponential backoff with jitter. - /// 0 disables retries. Commands that modify resources never retry. + /// (HTTP 429/500/502/503/504, timeouts). Uses exponential backoff with + /// jitter. 0 disables retries. Commands that modify resources never retry. #[arg(long, global = true, default_value_t = 3, value_name = "N")] pub retries: u32, diff --git a/src/commands/stream/actions.rs b/src/commands/stream/actions.rs index 2c3d460..420f318 100644 --- a/src/commands/stream/actions.rs +++ b/src/commands/stream/actions.rs @@ -27,7 +27,7 @@ pub(super) async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { } pub(super) async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { - let params = if let Some(path) = a.config_file { + let params = if let Some(path) = a.stream_config_file { let text = std::fs::read_to_string(&path)?; serde_json::from_str::(&text)? } else { @@ -39,9 +39,10 @@ pub(super) async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { } fn build_create_params(a: CreateArgs) -> Result { - // These flags are `required_unless_present = "config_file"` in clap, and - // this function is only reached when --config-file was NOT supplied. - const ENFORCED: &str = "enforced by clap unless --config-file is present"; + // These flags are `required_unless_present = "stream_config_file"` in + // clap, and this function is only reached when --stream-config-file was + // NOT supplied. + const ENFORCED: &str = "enforced by clap unless --stream-config-file is present"; let name = a.name.expect(ENFORCED); let network = a.network.expect(ENFORCED); let dataset = a.dataset.expect(ENFORCED); diff --git a/src/commands/stream/mod.rs b/src/commands/stream/mod.rs index e5aa383..53489c4 100644 --- a/src/commands/stream/mod.rs +++ b/src/commands/stream/mod.rs @@ -2,9 +2,11 @@ //! //! Stream `create` is highly configurable. To keep the CLI usable we expose //! the common fields directly (name, network, dataset, range, region, plan, -//! webhook url) and provide `--config-file` to load a full JSON +//! webhook url) and provide `--stream-config-file` to load a full JSON //! `CreateStreamParams` from disk for advanced cases (S3/Azure/Postgres/Kafka -//! destinations, filter functions, extra destinations, etc.). +//! destinations, filter functions, extra destinations, etc.). It is named +//! distinctly from the global `--config-file` (CLI auth/config TOML) so the +//! two can never be confused. mod actions; mod render; @@ -22,7 +24,7 @@ use crate::errors::CliError; qn stream list --limit 20\n \ qn stream create --name blocks --network ethereum-mainnet --dataset block \\\n \ --start 24691804 --end=-1 --region usa-east --webhook https://hook.example.com\n \ - qn stream create --config-file stream.json\n \ + qn stream create --stream-config-file stream.json\n \ qn stream pause s-1234")] pub struct Args { #[command(subcommand)] @@ -34,7 +36,7 @@ pub enum StreamCmd { /// List streams on the account. #[command(visible_alias = "ls")] List(ListArgs), - /// Create a stream (webhook destination). For non-webhook destinations, use --config-file. + /// Create a stream (webhook destination). For non-webhook destinations, use --stream-config-file. Create(Box), /// Show a stream's full configuration and current state. Show { id: String }, @@ -73,38 +75,39 @@ pub struct ListArgs { #[derive(Debug, ClapArgs)] pub struct CreateArgs { /// Load full `CreateStreamParams` from a JSON file. When supplied, all - /// other --flags are ignored. + /// other --flags are ignored. (Distinct from the global `--config-file`, + /// which selects the CLI's own config TOML.) #[arg(long, conflicts_with_all = ["name", "network", "dataset", "start", "end", "region", "plan", "webhook"])] - pub config_file: Option, + pub stream_config_file: Option, /// Stream name. - #[arg(long, required_unless_present = "config_file")] + #[arg(long, required_unless_present = "stream_config_file")] pub name: Option, /// Network (e.g. `ethereum-mainnet`). - #[arg(long, required_unless_present = "config_file")] + #[arg(long, required_unless_present = "stream_config_file")] pub network: Option, /// Dataset (snake_case). - #[arg(long, value_enum, required_unless_present = "config_file")] + #[arg(long, value_enum, required_unless_present = "stream_config_file")] pub dataset: Option, /// Start block. - #[arg(long, required_unless_present = "config_file")] + #[arg(long, required_unless_present = "stream_config_file")] pub start: Option, /// End block (`-1` for continuous). #[arg( long, allow_hyphen_values = true, - required_unless_present = "config_file" + required_unless_present = "stream_config_file" )] pub end: Option, /// Region. - #[arg(long, value_enum, required_unless_present = "config_file")] + #[arg(long, value_enum, required_unless_present = "stream_config_file")] pub region: Option, /// Billing plan slug (optional). #[arg(long)] pub plan: Option, - /// Webhook URL for the primary destination (use `--config-file` for other - /// destination types). - #[arg(long, required_unless_present = "config_file")] + /// Webhook URL for the primary destination (use `--stream-config-file` + /// for other destination types). + #[arg(long, required_unless_present = "stream_config_file")] pub webhook: Option, /// Webhook security token (32-byte minimum). Optional. #[arg(long)] diff --git a/src/retry.rs b/src/retry.rs index 77dcb2c..7cb5bcc 100644 --- a/src/retry.rs +++ b/src/retry.rs @@ -5,9 +5,8 @@ //! to repeat, and the backend is rate-limited, so scripted reads need to //! survive the occasional 429. //! -//! Backoff is driven purely by full jitter (`SdkError::Api` exposes the -//! status and body, which is what the retry decision is based on); the -//! randomized window keeps concurrent callers from herding. +//! Backoff is full jitter: the randomized window keeps concurrent callers +//! from herding. use std::future::Future; use std::time::Duration;