From 896a2a7fa71749a8a4dcb9b38e5f5c424251cd91 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Thu, 4 Jun 2026 12:17:38 -0400 Subject: [PATCH 1/3] Parse API error bodies into clean bullets with did-you-mean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The CLI previously rendered 400 responses as 'API returned HTTP 400.' with the raw JSON dump hidden behind --verbose. The server already returns structured validation errors; this surfaces them. errors::render_api_error now: - Walks JSON bodies for error/errors/message/messages keys, accepting strings, arrays of strings, arrays of objects, and nested objects. - Treats 400/422 as 'invalid request.' with one bullet per extracted message. - For 'X must be one of: …' bullets, runs Levenshtein against argv and appends a did-you-mean when the best match is within distance 3 and shares a 3-char prefix. - Truncates long enum lists to 5 + '(N more)'. - Maps known field names (network, chain) to follow-up commands. - Falls back to printing the raw body when JSON parsing fails. - Verbose still dumps the full body. Adds render_with_argv so the integration-test harness can use the simulated argv instead of the test binary's process argv. 17 new unit tests + 2 wiremock cases in tests/webhooks.rs. --- src/errors.rs | 488 ++++++++++++++++++++++++++++++++++++++++++-- tests/common/mod.rs | 7 +- tests/webhooks.rs | 106 ++++++++++ 3 files changed, 580 insertions(+), 21 deletions(-) diff --git a/src/errors.rs b/src/errors.rs index 080cafd..8aabf95 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,8 +1,10 @@ //! Top-level error type for the CLI and the SDK→user message mapping. +use std::collections::BTreeSet; use std::path::PathBuf; use quicknode_sdk::errors::{HttpKind, SdkError}; +use serde_json::Value; use thiserror::Error; #[derive(Debug, Error)] @@ -69,28 +71,21 @@ pub fn exit_code_for(err: &CliError) -> i32 { } } -/// Renders the error to a human-friendly single line (or two for multi-line bodies). +/// Renders the error to a human-friendly message using the real process argv +/// for did-you-mean suggestions. Use [`render_with_argv`] from tests where the +/// simulated argv differs from the process argv. /// /// Verbose mode appends the underlying body / source where available. pub fn render(err: &CliError, verbose: bool) -> String { + let argv: Vec = std::env::args().skip(1).collect(); + render_with_argv(err, verbose, &argv) +} + +/// Like [`render`] but uses the supplied argv values for did-you-mean lookup. +pub fn render_with_argv(err: &CliError, verbose: bool, argv: &[String]) -> String { match err { CliError::Sdk(SdkError::Api { status, body }) => { - let code = status.as_u16(); - let base = match code { - 401 | 403 => "unauthorized. Check your API key with 'qn auth whoami'.".to_string(), - 404 => "not found.".to_string(), - 422 => "the API rejected the request as invalid.".to_string(), - 429 => "rate limited by the Quicknode API. Try again shortly.".to_string(), - 500..=599 => format!( - "Quicknode API is having issues (HTTP {code}). Try again or check status.quicknode.com." - ), - _ => format!("API returned HTTP {code}."), - }; - if verbose && !body.is_empty() { - format!("Error: {base}\n{body}") - } else { - format!("Error: {base}") - } + render_api_error(status.as_u16(), body, verbose, argv) } CliError::Sdk(sdk @ SdkError::Http(_)) => { let msg = match sdk.http_kind() { @@ -133,18 +128,317 @@ pub fn render(err: &CliError, verbose: bool) -> String { } } +/// Status codes have a small set of canonical user-facing messages. Validation +/// (400/422) gets the structured body treatment from `parse_api_body`. +fn render_api_error(code: u16, body: &str, verbose: bool, argv: &[String]) -> String { + let headline = match code { + 400 | 422 => "invalid request.".to_string(), + 401 | 403 => "unauthorized. Check your API key with 'qn auth whoami'.".to_string(), + 404 => "not found.".to_string(), + 429 => "rate limited by the Quicknode API. Try again shortly.".to_string(), + 500..=599 => format!( + "Quicknode API is having issues (HTTP {code}). Try again or check status.quicknode.com." + ), + _ => format!("API returned HTTP {code}."), + }; + + // For non-validation status codes, body is mostly noise (server stack traces, + // HTML error pages, etc). Only mine it for validation-class errors. + let parsed = if matches!(code, 400 | 422) { + parse_api_body(body, argv) + } else { + ParsedApiBody::default() + }; + + let mut out = format!("Error: {headline}"); + + if !parsed.bullets.is_empty() { + for bullet in &parsed.bullets { + out.push_str("\n • "); + out.push_str(bullet); + } + } else if matches!(code, 400 | 422) && !body.is_empty() && !verbose { + // We tried to parse and got nothing useful; surface the raw body so + // the user isn't left with a bare "invalid request." line. + out.push('\n'); + out.push_str(body.trim()); + } + + for hint in &parsed.hints { + out.push('\n'); + out.push_str(hint); + } + + if verbose && !body.is_empty() { + out.push('\n'); + out.push_str(body); + } else if matches!(code, 400 | 422) && !parsed.bullets.is_empty() && !body.is_empty() { + out.push_str("\nRe-run with --verbose for the full response body."); + } + + out +} + +#[derive(Default)] +struct ParsedApiBody { + bullets: Vec, + hints: Vec, +} + +/// Parses a JSON-shaped API error body, extracting human-readable messages and +/// (when the body contains "must be one of …" enum lists) appending +/// did-you-mean suggestions against the user's argv. +fn parse_api_body(body: &str, argv: &[String]) -> ParsedApiBody { + let mut out = ParsedApiBody::default(); + if body.is_empty() { + return out; + } + let Ok(value) = serde_json::from_str::(body) else { + return out; + }; + + let mut raw_strings: Vec = Vec::new(); + collect_error_strings(&value, &mut raw_strings); + if raw_strings.is_empty() { + return out; + } + + let mut seen: BTreeSet = BTreeSet::new(); + let mut fields_hinted: BTreeSet = BTreeSet::new(); + + for s in raw_strings { + let trimmed = s.trim().to_string(); + if trimmed.is_empty() || !seen.insert(trimmed.clone()) { + continue; + } + if is_generic_label(&trimmed) { + // Skip "Bad Request" / "Unauthorized" — these duplicate the headline. + continue; + } + let bullet = decorate_with_suggestion(&trimmed, argv, &mut fields_hinted); + out.bullets.push(bullet); + } + + for field in &fields_hinted { + if let Some(hint) = field_hint(field) { + out.hints.push(hint.to_string()); + } + } + + out +} + +/// Recursively walk a JSON value, pulling strings out of any key named +/// `error`, `errors`, `message`, or `messages`. Accepts strings, arrays of +/// strings, arrays of objects (recurse), and nested objects (recurse). +fn collect_error_strings(value: &Value, out: &mut Vec) { + const KEYS: &[&str] = &["errors", "error", "messages", "message"]; + match value { + Value::Object(map) => { + for key in KEYS { + if let Some(v) = map.get(*key) { + collect_strings_from(v, out); + } + } + // Also recurse into other object values so we can find nested + // error/message keys (e.g. NestJS wraps under `message.message`). + for (k, v) in map { + if !KEYS.contains(&k.as_str()) { + collect_error_strings(v, out); + } + } + } + Value::Array(arr) => { + for v in arr { + collect_error_strings(v, out); + } + } + _ => {} + } +} + +/// Helper: when we hit one of the error keys, accept multiple shapes. +fn collect_strings_from(value: &Value, out: &mut Vec) { + match value { + Value::String(s) => out.push(s.clone()), + Value::Array(arr) => { + for v in arr { + match v { + Value::String(s) => out.push(s.clone()), + Value::Object(_) => collect_error_strings(v, out), + _ => {} + } + } + } + Value::Object(_) => collect_error_strings(value, out), + _ => {} + } +} + +/// Returns the bullet text, possibly with a `did you mean '…'?` suffix and a +/// `(N more)` truncation marker. Also records which fields had enum lists so +/// the caller can attach helper hints. +fn decorate_with_suggestion( + raw: &str, + argv: &[String], + fields_hinted: &mut BTreeSet, +) -> String { + let Some((field, candidates)) = parse_must_be_one_of(raw) else { + return raw.to_string(); + }; + + fields_hinted.insert(field.clone()); + + // Find the argv value that's closest to any candidate, then attach DYM if + // the best match is within threshold. + let best = best_suggestion(argv, &candidates); + + let display = truncate_candidate_list(&candidates, 5); + let mut bullet = format!("{field} must be one of: {display}"); + if let Some((user_value, suggestion)) = best { + bullet.push_str(&format!( + " — did you mean '{suggestion}' (you passed '{user_value}')?" + )); + } + bullet +} + +/// Parse `" must be one of [the following values:] X, Y, Z"`. +/// Returns the field name and the candidate list. +fn parse_must_be_one_of(s: &str) -> Option<(String, Vec)> { + let (field_part, rest) = s.split_once(" must be one of")?; + let field = field_part.trim(); + if field.is_empty() + || !field + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '.') + { + return None; + } + // After "must be one of" we accept any of: " the following values: X, Y", + // ": X, Y", or " X, Y" (rare but possible). + let list_part = rest + .strip_prefix(" the following values: ") + .or_else(|| rest.strip_prefix(": ")) + .or_else(|| rest.strip_prefix(' ')) + .unwrap_or(rest) + .trim_end_matches('.'); + let candidates: Vec = list_part + .split(", ") + .map(|c| c.trim().to_string()) + .filter(|c| !c.is_empty()) + .collect(); + if candidates.len() < 2 { + return None; + } + Some((field.to_string(), candidates)) +} + +/// Find the (argv-value, candidate) pair with smallest Levenshtein distance, +/// gated on: distance ≤ 3 AND ≥ 3 leading chars shared with the candidate. +fn best_suggestion(argv: &[String], candidates: &[String]) -> Option<(String, String)> { + let mut best: Option<(usize, String, String)> = None; + for arg in argv { + // Skip flags themselves and obviously-non-value tokens. + if arg.starts_with('-') || arg.is_empty() || arg.len() < 2 { + continue; + } + for cand in candidates { + let d = levenshtein(arg, cand); + if d > 3 { + continue; + } + if shared_prefix_len(arg, cand) < 3 { + continue; + } + match best.as_ref() { + None => best = Some((d, arg.clone(), cand.clone())), + Some((cur, _, _)) if d < *cur => best = Some((d, arg.clone(), cand.clone())), + _ => {} + } + } + } + best.map(|(_, a, c)| (a, c)) +} + +fn shared_prefix_len(a: &str, b: &str) -> usize { + a.chars().zip(b.chars()).take_while(|(x, y)| x == y).count() +} + +/// Classic O(n*m) Levenshtein distance. n,m are tiny here (≤ ~40 chars), so +/// this is fine. +fn levenshtein(a: &str, b: &str) -> usize { + let a: Vec = a.chars().collect(); + let b: Vec = b.chars().collect(); + if a.is_empty() { + return b.len(); + } + if b.is_empty() { + return a.len(); + } + let mut prev: Vec = (0..=b.len()).collect(); + let mut curr = vec![0usize; b.len() + 1]; + for (i, ca) in a.iter().enumerate() { + curr[0] = i + 1; + for (j, cb) in b.iter().enumerate() { + let cost = if ca == cb { 0 } else { 1 }; + curr[j + 1] = (curr[j] + 1).min(prev[j + 1] + 1).min(prev[j] + cost); + } + std::mem::swap(&mut prev, &mut curr); + } + prev[b.len()] +} + +fn truncate_candidate_list(candidates: &[String], keep: usize) -> String { + if candidates.len() <= keep { + return candidates.join(", "); + } + let shown = candidates[..keep].join(", "); + let extra = candidates.len() - keep; + format!("{shown} ({extra} more)") +} + +/// Maps known server-side field names to a follow-up command the user can run +/// to discover valid values. +/// Skip standard HTTP status-phrase strings that duplicate the headline. +fn is_generic_label(s: &str) -> bool { + matches!( + s, + "Bad Request" + | "Unauthorized" + | "Forbidden" + | "Not Found" + | "Unprocessable Entity" + | "Too Many Requests" + | "Internal Server Error" + | "Service Unavailable" + ) +} + +fn field_hint(field: &str) -> Option<&'static str> { + match field { + "network" => Some("Run 'qn chain list' to see supported networks."), + "chain" => Some("Run 'qn chain list' to see supported chains."), + _ => None, + } +} + #[cfg(test)] mod tests { use super::*; use quicknode_sdk::errors::SdkError; - fn api_err(code: u16) -> CliError { + fn api_err_with(code: u16, body: &str) -> CliError { CliError::Sdk(SdkError::Api { status: reqwest::StatusCode::from_u16(code).unwrap(), - body: "{\"message\":\"boom\"}".to_string(), + body: body.to_string(), }) } + fn api_err(code: u16) -> CliError { + api_err_with(code, "{\"message\":\"boom\"}") + } + #[test] fn exit_code_api_is_2() { assert_eq!(exit_code_for(&api_err(404)), 2); @@ -189,4 +483,160 @@ mod tests { let msg = render(&api_err(404), false); assert!(!msg.contains("boom"), "got: {msg}"); } + + // ---- body parsing ---- + + #[test] + fn nestjs_shape_extracts_bullets() { + let body = r#"{"statusCode":400,"message":{"message":["network must be one of the following values: ethereum-mainnet, ethereum-sepolia, solana-mainnet","status must be one of the following values: active, paused, terminated"],"error":"Bad Request"}}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.starts_with("Error: invalid request."), "got: {msg}"); + assert!(msg.contains("• network must be one of:"), "got: {msg}"); + assert!(msg.contains("• status must be one of:"), "got: {msg}"); + } + + #[test] + fn admin_shape_extracts_error_string() { + let body = r#"{"data":null,"error":"undefined method `chain' for nil"}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("undefined method"), "got: {msg}"); + } + + #[test] + fn empty_body_400_falls_through() { + let msg = render(&api_err_with(400, ""), false); + assert_eq!(msg, "Error: invalid request."); + } + + #[test] + fn garbage_non_json_body_falls_back_to_raw() { + let body = "oops"; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("oops"), "got: {msg}"); + } + + #[test] + fn generic_errors_array_of_strings() { + let body = r#"{"errors":["first thing wrong","second thing wrong"]}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("• first thing wrong"), "got: {msg}"); + assert!(msg.contains("• second thing wrong"), "got: {msg}"); + } + + #[test] + fn generic_errors_array_of_objects() { + let body = r#"{"errors":[{"message":"thing one"},{"message":"thing two"}]}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("• thing one"), "got: {msg}"); + assert!(msg.contains("• thing two"), "got: {msg}"); + } + + #[test] + fn dedupes_repeated_strings() { + let body = r#"{"error":"same thing","message":"same thing"}"#; + let msg = render(&api_err_with(400, body), false); + let count = msg.matches("same thing").count(); + assert_eq!(count, 1, "expected dedupe, got: {msg}"); + } + + #[test] + fn truncates_long_enum_list() { + // 10 candidates, only the first 5 should render inline. + let body = r#"{"message":"x must be one of a, b, c, d, e, f, g, h, i, j"}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("a, b, c, d, e (5 more)"), "got: {msg}"); + } + + #[test] + fn field_hint_appended_for_network() { + let body = r#"{"message":"network must be one of: ethereum-mainnet, solana-mainnet"}"#; + let msg = render(&api_err_with(400, body), false); + assert!(msg.contains("qn chain list"), "got: {msg}"); + } + + #[test] + fn verbose_appends_full_body() { + let body = r#"{"message":["network must be one of: a, b, c"]}"#; + let msg = render(&api_err_with(400, body), true); + assert!(msg.contains(body), "got: {msg}"); + } + + #[test] + fn levenshtein_basic() { + assert_eq!(levenshtein("", ""), 0); + assert_eq!(levenshtein("a", ""), 1); + assert_eq!(levenshtein("", "abc"), 3); + assert_eq!(levenshtein("kitten", "sitting"), 3); + assert_eq!(levenshtein("ethereum-mainnet", "ethereum-mainnet"), 0); + assert_eq!(levenshtein("ethereum-mainnetsds", "ethereum-mainnet"), 3); + } + + #[test] + fn parse_must_be_one_of_happy_path() { + let (f, c) = + parse_must_be_one_of("network must be one of the following values: a, b, c").unwrap(); + assert_eq!(f, "network"); + assert_eq!(c, vec!["a", "b", "c"]); + } + + #[test] + fn parse_must_be_one_of_no_following_values_prefix() { + let (f, c) = parse_must_be_one_of("status must be one of active, paused").unwrap(); + assert_eq!(f, "status"); + assert_eq!(c, vec!["active", "paused"]); + } + + #[test] + fn parse_must_be_one_of_rejects_unrelated_strings() { + assert!(parse_must_be_one_of("some random error").is_none()); + } + + #[test] + fn truncate_candidate_list_under_keep_returns_all() { + assert_eq!( + truncate_candidate_list(&["a".into(), "b".into()], 5), + "a, b" + ); + } + + #[test] + fn best_suggestion_picks_closest_within_threshold() { + let candidates: Vec = + vec!["ethereum-mainnet", "ethereum-sepolia", "solana-mainnet"] + .into_iter() + .map(String::from) + .collect(); + let argv = vec!["ethereum-mainnetsds".to_string()]; + let suggestion = best_suggestion(&argv, &candidates); + assert_eq!( + suggestion, + Some(("ethereum-mainnetsds".into(), "ethereum-mainnet".into())) + ); + } + + #[test] + fn best_suggestion_returns_none_if_too_far() { + let candidates: Vec = vec!["ethereum-mainnet"] + .into_iter() + .map(String::from) + .collect(); + let argv = vec!["sfjla".to_string()]; + assert_eq!(best_suggestion(&argv, &candidates), None); + } + + #[test] + fn best_suggestion_ignores_flag_tokens() { + let candidates: Vec = vec!["chain"].into_iter().map(String::from).collect(); + let argv = vec!["--chain".to_string()]; + // "--chain" starts with "-", should be skipped. + assert_eq!(best_suggestion(&argv, &candidates), None); + } + + #[test] + fn renders_5xx_skips_body_parsing() { + // We don't want stack-trace HTML on a 500 to be parsed as bullets. + let body = r#"{"message":"internal error"}"#; + let msg = render(&api_err_with(500, body), false); + assert!(!msg.contains("•"), "got: {msg}"); + } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index 6367170..a66f64c 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -11,7 +11,7 @@ use clap::Parser; use qn::cli::Cli; -use qn::errors::{exit_code_for, render}; +use qn::errors::{exit_code_for, render_with_argv}; /// Result of dispatching a CLI invocation in-process. pub struct RunOutput { @@ -52,6 +52,9 @@ pub async fn run_qn(base_url: &str, extra_args: &[&str]) -> RunOutput { }; let verbose = cli.verbose; + // Use the simulated argv (skip the leading "qn" token) so did-you-mean + // suggestions can see what the test "user" passed. + let argv_for_render: Vec = argv.iter().skip(1).cloned().collect(); match cli.run().await { Ok(()) => RunOutput { stdout: String::new(), @@ -60,7 +63,7 @@ pub async fn run_qn(base_url: &str, extra_args: &[&str]) -> RunOutput { }, Err(e) => RunOutput { stdout: String::new(), - stderr: render(&e, verbose), + stderr: render_with_argv(&e, verbose, &argv_for_render), exit_code: exit_code_for(&e), }, } diff --git a/tests/webhooks.rs b/tests/webhooks.rs index 20067c1..be7f0f2 100644 --- a/tests/webhooks.rs +++ b/tests/webhooks.rs @@ -196,3 +196,109 @@ async fn webhook_create_missing_wallets_errors() { assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); assert!(out.stderr.contains("--wallet"), "stderr={}", out.stderr); } + +// ---- 400 error rendering ---- // + +/// A close-to-real NestJS validator 400 from the webhooks service. +fn nestjs_network_400() -> serde_json::Value { + json!({ + "statusCode": 400, + "timestamp": "2026-06-04T14:45:19.326Z", + "path": "/webhooks/rest/v1/webhooks/template/evmWalletFilter", + "message": { + "message": [ + "network must be one of the following values: ethereum-mainnet, ethereum-sepolia, solana-mainnet, solana-devnet, base-mainnet, base-sepolia, polygon-mainnet, polygon-amoy" + ], + "error": "Bad Request", + "statusCode": 400 + } + }) +} + +#[tokio::test] +async fn create_webhook_400_renders_bullets_with_typo_suggestion() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/webhooks/rest/v1/webhooks/template/evmWalletFilter")) + .respond_with(ResponseTemplate::new(400).set_body_json(nestjs_network_400())) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &[ + "webhook", + "create", + "--name", + "w1", + "--network", + "ethereum-mainnetsds", + "--url", + "https://hook.example/x", + "--template", + "evm-wallet", + "--wallet", + "0xabc", + ], + ) + .await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); + assert!( + out.stderr.contains("invalid request"), + "stderr={}", + out.stderr + ); + assert!( + out.stderr.contains("network must be one of"), + "stderr={}", + out.stderr + ); + assert!( + out.stderr.contains("did you mean 'ethereum-mainnet'"), + "stderr={}", + out.stderr + ); + assert!( + out.stderr.contains("qn chain list"), + "stderr={}", + out.stderr + ); +} + +#[tokio::test] +async fn create_webhook_400_far_typo_no_suggestion() { + let server = MockServer::start().await; + Mock::given(method("POST")) + .and(path("/webhooks/rest/v1/webhooks/template/evmWalletFilter")) + .respond_with(ResponseTemplate::new(400).set_body_json(nestjs_network_400())) + .mount(&server) + .await; + let out = run_qn( + &server.uri(), + &[ + "webhook", + "create", + "--name", + "w1", + "--network", + "sfjla", + "--url", + "https://hook.example/x", + "--template", + "evm-wallet", + "--wallet", + "0xabc", + ], + ) + .await; + assert_eq!(out.exit_code, 2, "stderr={}", out.stderr); + assert!( + out.stderr.contains("network must be one of"), + "stderr={}", + out.stderr + ); + assert!( + !out.stderr.contains("did you mean"), + "should not suggest for 'sfjla'; stderr={}", + out.stderr + ); +} From 3f635ce6283207f6727d293cce2d765a087a6c8d Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Thu, 4 Jun 2026 12:17:47 -0400 Subject: [PATCH 2/3] Add required-args pre-flight checks for four create/update commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the 'empty-body 400' gap audited across the create/update surface: where the user supplies zero (or trivially insufficient) flags, the server today returns a 400 with no useful body — the new Stage A renderer can't help. These commands now fail fast client-side with a CliError::Arg naming the missing flags. - endpoint create: require at least --chain or --network - endpoint update: require --label - endpoint security set-options: require at least one toggle - team set-endpoints: require at least one endpoint id webhook create's required fields are already enforced by clap. Four integration tests assert exit code 1 and zero HTTP traffic. --- src/commands/endpoint/mod.rs | 10 ++++++++ src/commands/endpoint/security.rs | 17 ++++++++++++++ src/commands/team.rs | 5 ++++ tests/admin_extra.rs | 9 +++++++ tests/endpoint.rs | 39 +++++++++++++++++++++++++++++++ 5 files changed, 80 insertions(+) diff --git a/src/commands/endpoint/mod.rs b/src/commands/endpoint/mod.rs index 2fe045f..05a9242 100644 --- a/src/commands/endpoint/mod.rs +++ b/src/commands/endpoint/mod.rs @@ -224,6 +224,13 @@ async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { } async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { + if a.chain.is_none() && a.network.is_none() { + return Err(CliError::Arg( + "'endpoint create' requires at least one of --chain or --network. \ + Run 'qn chain list' to see available chains." + .into(), + )); + } let req = CreateEndpointRequest { chain: a.chain, network: a.network, @@ -243,6 +250,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 }; ctx.sdk.admin.update_endpoint(&a.id, &req).await?; ctx.out.note(&format!("✓ Updated endpoint {}", a.id)); diff --git a/src/commands/endpoint/security.rs b/src/commands/endpoint/security.rs index 289889a..4892157 100644 --- a/src/commands/endpoint/security.rs +++ b/src/commands/endpoint/security.rs @@ -216,6 +216,23 @@ async fn options_show(id: &str, ctx: Ctx) -> Result<(), CliError> { } async fn set_options(a: SetOptionsArgs, ctx: Ctx) -> Result<(), CliError> { + if a.tokens.is_none() + && a.referrers.is_none() + && a.jwts.is_none() + && a.ips.is_none() + && a.domain_masks.is_none() + && a.hsts.is_none() + && a.cors.is_none() + && a.request_filters.is_none() + && a.ip_custom_header.is_none() + { + return Err(CliError::Arg( + "'endpoint security set-options' requires at least one of: \ + --tokens, --referrers, --jwts, --ips, --domain-masks, --hsts, \ + --cors, --request-filters, --ip-custom-header." + .into(), + )); + } let options = SecurityOptionsUpdate { tokens: a.tokens.map(|t| t.as_str().to_string()), referrers: a.referrers.map(|t| t.as_str().to_string()), diff --git a/src/commands/team.rs b/src/commands/team.rs index 0637ef2..7b78dfe 100644 --- a/src/commands/team.rs +++ b/src/commands/team.rs @@ -165,6 +165,11 @@ async fn endpoints(id: i64, ctx: Ctx) -> Result<(), CliError> { } async fn set_endpoints(a: SetEndpointsArgs, ctx: Ctx) -> Result<(), CliError> { + if a.endpoint_ids.is_empty() { + return Err(CliError::Arg( + "'team set-endpoints' requires at least one endpoint id.".into(), + )); + } let req = UpdateTeamEndpointsRequest { endpoint_ids: a.endpoint_ids.clone(), }; diff --git a/tests/admin_extra.rs b/tests/admin_extra.rs index d049416..fdccbb0 100644 --- a/tests/admin_extra.rs +++ b/tests/admin_extra.rs @@ -237,3 +237,12 @@ async fn bulk_tag_add() { .await; assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); } + +#[tokio::test] +async fn team_set_endpoints_no_ids_fails_before_api_call() { + let server = MockServer::start().await; + let out = run_qn(&server.uri(), &["team", "set-endpoints", "7"]).await; + assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); + assert!(out.stderr.contains("endpoint id"), "stderr={}", out.stderr); + assert_eq!(server.received_requests().await.unwrap().len(), 0); +} diff --git a/tests/endpoint.rs b/tests/endpoint.rs index 1f06ec8..8937520 100644 --- a/tests/endpoint.rs +++ b/tests/endpoint.rs @@ -491,3 +491,42 @@ async fn endpoint_ratelimit_set_omits_unset_fields() { .await; assert_eq!(out.exit_code, 0, "stderr={}", out.stderr); } + +// ---- Stage B: required-args pre-flight checks ---- // + +#[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. + let server = MockServer::start().await; + let out = run_qn(&server.uri(), &["endpoint", "create"]).await; + assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); + assert!( + out.stderr.contains("--chain") && out.stderr.contains("--network"), + "stderr={}", + out.stderr + ); + assert_eq!(server.received_requests().await.unwrap().len(), 0); +} + +#[tokio::test] +async fn endpoint_update_no_flags_fails_before_api_call() { + let server = MockServer::start().await; + let out = run_qn(&server.uri(), &["endpoint", "update", "ep-1"]).await; + assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); + assert!(out.stderr.contains("--label"), "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; + let out = run_qn( + &server.uri(), + &["endpoint", "security", "set-options", "ep-1"], + ) + .await; + assert_eq!(out.exit_code, 1, "stderr={}", out.stderr); + assert!(out.stderr.contains("--tokens"), "stderr={}", out.stderr); + assert_eq!(server.received_requests().await.unwrap().len(), 0); +} From d74116f160004f1cf3e6cfe15f1edd7099d03d20 Mon Sep 17 00:00:00 2001 From: John Mitsch Date: Thu, 4 Jun 2026 12:19:39 -0400 Subject: [PATCH 3/3] endpoint create: require both --chain and --network The server requires both fields, not 'at least one'. Tighten the pre-flight to name whichever flag (or both) the user omitted instead of letting --chain-only through to a 400. --- src/commands/endpoint/mod.rs | 18 ++++++++++++------ tests/endpoint.rs | 17 +++++++++++++++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/commands/endpoint/mod.rs b/src/commands/endpoint/mod.rs index 05a9242..69ec8f3 100644 --- a/src/commands/endpoint/mod.rs +++ b/src/commands/endpoint/mod.rs @@ -224,12 +224,18 @@ async fn list(a: ListArgs, ctx: Ctx) -> Result<(), CliError> { } async fn create(a: CreateArgs, ctx: Ctx) -> Result<(), CliError> { - if a.chain.is_none() && a.network.is_none() { - return Err(CliError::Arg( - "'endpoint create' requires at least one of --chain or --network. \ - Run 'qn chain list' to see available chains." - .into(), - )); + 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, diff --git a/tests/endpoint.rs b/tests/endpoint.rs index 8937520..3549a72 100644 --- a/tests/endpoint.rs +++ b/tests/endpoint.rs @@ -509,6 +509,23 @@ async fn endpoint_create_no_flags_fails_before_api_call() { assert_eq!(server.received_requests().await.unwrap().len(), 0); } +#[tokio::test] +async fn endpoint_create_only_chain_fails_before_api_call() { + let server = MockServer::start().await; + let out = run_qn( + &server.uri(), + &["endpoint", "create", "--chain", "ethereum"], + ) + .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 + ); + assert_eq!(server.received_requests().await.unwrap().len(), 0); +} + #[tokio::test] async fn endpoint_update_no_flags_fails_before_api_call() { let server = MockServer::start().await;