From 0cc2d47148922e2b7ba0de05e42caf771d1b33a1 Mon Sep 17 00:00:00 2001 From: Jannik Peters Date: Mon, 2 Mar 2026 15:55:35 +0100 Subject: [PATCH 1/2] Improve CLI error messages on HTTP/TCP errors --- crates/cli/src/client.rs | 49 +++++++++++++++++++++++++++------ crates/cli/src/commands/zone.rs | 10 +++---- 2 files changed, 46 insertions(+), 13 deletions(-) diff --git a/crates/cli/src/client.rs b/crates/cli/src/client.rs index 2c2937d6..77f63ca1 100644 --- a/crates/cli/src/client.rs +++ b/crates/cli/src/client.rs @@ -1,3 +1,4 @@ +use std::error::Error; use std::time::Duration; use reqwest::{IntoUrl, Method, RequestBuilder}; @@ -42,15 +43,47 @@ impl CascadeApiClient { } } +/// Format HTTP errors with message based on error type, and chain error +/// descriptions together instead of simply printing the Debug representation +/// (which is confusing for users). pub fn format_http_error(err: reqwest::Error) -> String { - if err.is_decode() { - // Use the debug representation of decoding errors otherwise the cause - // of the decoding failure, e.g. the underlying Serde error, gets lost - // and makes determining why the response couldn't be decoded a game - // of divide and conquer removing response fields one by one until the - // offending field is determined. - format!("HTTP request failed: {err:?}") + let mut message = String::new(); + + if err.is_timeout() { + // "Returns true if the error is related to a timeout." [1] + return String::from("HTTP connection timed out"); + } + + // [1]: https://docs.rs/reqwest/latest/reqwest/struct.Error.html + if err.is_connect() { + // "Returns true if the error is related to connect" [1] + message.push_str("HTTP connection failed"); + } else if err.is_decode() { + // "Returns true if the error is related to decoding the response’s body" [1] + // Originally, we used the debug representation to be able to see all + // fields related to the error and make finding the offending field + // easier. This was confusing for users. Now we print the "source()" + // of the error below, which contains the relevant information. + message.push_str("HTTP response decoding failed"); } else { - format!("HTTP request failed: {err}") + // Covers unknown errors, non-OK HTTP status codes, errors "related to + // the request" [1], errors "related to the request or response body" + // [1], errors "from a type Builder" [1], errors "from + // a RedirectPolicy." [1], errors "related to a protocol upgrade + // request" [1] + message.push_str("HTTP request failed"); } + + // Chain error sources together to capture all relevant error parts. E.g.: + // "client error (Connect): tcp connect error: Connection refused (os error 111)" + // instead of just "client error (Connect)"; + // and "client error (SendRequest): connection closed before message completed" + // instead of just "client error (SendRequest)" + let mut we = err.source(); + while let Some(e) = we { + message.push_str(&format!(": {e}")); + we = e.source(); + } + + message } diff --git a/crates/cli/src/commands/zone.rs b/crates/cli/src/commands/zone.rs index d496531f..bc47060a 100644 --- a/crates/cli/src/commands/zone.rs +++ b/crates/cli/src/commands/zone.rs @@ -288,7 +288,7 @@ impl Zone { .send() .and_then(|r| r.json()) .await - .map_err(|e| format!("HTTP request failed: {e:?}"))?; + .map_err(format_http_error)?; match result { Ok(ZoneReviewOutput {}) => { @@ -326,7 +326,7 @@ impl Zone { .send() .and_then(|r| r.json()) .await - .map_err(|e| format!("HTTP request failed: {e:?}"))?; + .map_err(format_http_error)?; match result { Ok(ZoneReviewOutput {}) => { @@ -348,7 +348,7 @@ impl Zone { .send() .and_then(|r| r.json()) .await - .map_err(|e| format!("HTTP request failed: {e:?}"))?; + .map_err(format_http_error)?; match response { Ok(status) => Self::print_zone_status(client, status, detailed).await, @@ -364,7 +364,7 @@ impl Zone { .send() .and_then(|r| r.json()) .await - .map_err(|e| format!("HTTP request failed: {e:?}"))?; + .map_err(format_http_error)?; match response { Ok(response) => { @@ -481,7 +481,7 @@ impl Zone { .send() .and_then(|r| r.json()) .await - .map_err(|e| format!("HTTP request failed: {e:?}"))?; + .map_err(format_http_error)?; let policy = response.map_err(|_| { format!( From 43d5cee4db7221e974ec6922e98c0e0f8e5aa818 Mon Sep 17 00:00:00 2001 From: Jannik Peters Date: Mon, 2 Mar 2026 16:06:04 +0100 Subject: [PATCH 2/2] Document reason for special timeout message --- crates/cli/src/client.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/cli/src/client.rs b/crates/cli/src/client.rs index 77f63ca1..2fdb290e 100644 --- a/crates/cli/src/client.rs +++ b/crates/cli/src/client.rs @@ -49,6 +49,8 @@ impl CascadeApiClient { pub fn format_http_error(err: reqwest::Error) -> String { let mut message = String::new(); + // Returning a shortened timed out message to not have a redundant text + // like: "... HTTP connection timed out: operation timed out" if err.is_timeout() { // "Returns true if the error is related to a timeout." [1] return String::from("HTTP connection timed out");