From 33672a12eabae2bd2b541d5b963277ab3fb490bb Mon Sep 17 00:00:00 2001 From: ldm0 Date: Tue, 22 Sep 2026 14:59:51 +0800 Subject: [PATCH] fix(fetch): include underlying curl errors in failure reports Preserve the full error chain in network failure reasons and the CLI fallback so curl error codes and detailed TLS, DNS, and connection failures remain visible. Keep the concise two-line CLI presentation and typed readiness timeout handling. Test TLS failures across all fetch transports, network error classifications, and CLI connection failures. Validated with cargo fmt --all, workspace Clippy with all targets and features, and cargo nextest run --no-fail-fast (18444 passed). Refs #701 --- moli-fetch/src/network_fetch_result.rs | 59 +++++++++++++++++++++++-- moli-fetch/src/tests/tls_credentials.rs | 45 ++++++++++++++++++- moli/src/app.rs | 8 ++-- moli/tests/fetch_cli.rs | 18 ++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/moli-fetch/src/network_fetch_result.rs b/moli-fetch/src/network_fetch_result.rs index f85f89c5b..ded5de096 100644 --- a/moli-fetch/src/network_fetch_result.rs +++ b/moli-fetch/src/network_fetch_result.rs @@ -433,7 +433,7 @@ impl NetworkFetchFailureContext { observation_journal: NetworkObservationJournal, ) -> anyhow::Error { let network_error_text = crate::error::browser_network_error_text(&source); - let reason = source.to_string(); + let reason = format!("{source:#}"); source.context(Self { observation_journal, network_error_text, @@ -448,7 +448,7 @@ impl NetworkFetchFailureContext { request_context: NetworkFetchFailureRequestContext, ) -> anyhow::Error { let network_error_text = crate::error::browser_network_error_text(&source); - let reason = source.to_string(); + let reason = format!("{source:#}"); source.context(Self { observation_journal, network_error_text, @@ -465,8 +465,9 @@ impl NetworkFetchFailureContext { self.network_error_text } - /// Returns the single human-readable transport or policy reason captured - /// before this machine-readable context was attached. + /// Returns the human-readable transport or policy error chain captured + /// before this machine-readable context was attached, including the + /// underlying curl error code and detailed error buffer when available. pub fn reason(&self) -> &str { &self.reason } @@ -586,6 +587,56 @@ impl NetworkFetchResult { mod tests { use super::*; + #[test] + fn network_failure_reason_preserves_curl_code_and_details() { + for (code, detail, network_error_text) in [ + ( + curl_sys::CURLE_PEER_FAILED_VERIFICATION, + "SSL certificate problem: unable to get local issuer certificate", + "net::ERR_CERT_AUTHORITY_INVALID", + ), + ( + curl_sys::CURLE_COULDNT_RESOLVE_HOST, + "Could not resolve host: example.test", + "net::ERR_NAME_NOT_RESOLVED", + ), + ( + curl_sys::CURLE_COULDNT_CONNECT, + "Failed to connect to example.test: Connection refused", + "net::ERR_CONNECTION_REFUSED", + ), + ] { + for with_request_context in [false, true] { + let mut curl_error = curl::Error::new(code); + curl_error.set_extra(detail.to_owned()); + let source = anyhow::Error::new(curl_error).context("curl request failed"); + let journal = NetworkObservationJournal::default(); + let error = if with_request_context { + NetworkFetchFailureContext::attach_with_request_context( + source, + journal, + NetworkFetchFailureRequestContext::new( + Url::parse("https://example.test/").unwrap(), + "GET".to_owned(), + None, + Vec::new().into(), + Vec::new(), + ), + ) + } else { + NetworkFetchFailureContext::attach(source, journal) + }; + let failure = error.downcast_ref::().unwrap(); + let reason = failure.reason(); + assert!(reason.starts_with("curl request failed: "), "{reason}"); + assert!(reason.contains(&format!("[{code}]")), "{reason}"); + assert!(reason.contains(detail), "{reason}"); + assert_eq!(failure.network_error_text(), network_error_text); + assert_eq!(error.downcast_ref::().unwrap().code(), code); + } + } + } + #[test] fn network_fetch_failure_context_preserves_source_without_repeating_it() { let source = std::io::Error::other("transport failure sentinel"); diff --git a/moli-fetch/src/tests/tls_credentials.rs b/moli-fetch/src/tests/tls_credentials.rs index bc947e7a3..34e019e99 100644 --- a/moli-fetch/src/tests/tls_credentials.rs +++ b/moli-fetch/src/tests/tls_credentials.rs @@ -26,7 +26,7 @@ use url::Url; use crate::{ FetchCancelHandle, FetchClient, FetchClientHandle, FetchConfig, RedirectInfo, RedirectSource, - Request, RequestCredentialsMode, RequestMode, + Request, RequestCredentialsMode, RequestMode, network_fetch_result::NetworkObservationRecorder, }; use super::support::unique_test_cache_dir; @@ -315,6 +315,49 @@ impl Transport { } } +#[tokio::test] +async fn tls_trust_failure_reports_curl_details_in_every_transport() -> Result<()> { + let credentials = TlsCredentials::new()?; + let server = TlsServer::spawn(&credentials).await?; + let mut config = FetchConfig::default(); + config.set_http_proxy(Some(String::new())); + config.set_request_timeout_ms(5_000); + // The generated CA is intentionally absent from the client's trust store. + for transport in Transport::ALL { + let client = FetchClient::new(&config, new_shared_browser_cookie_store()); + let request = Request::get(server.url.as_str())? + .with_network_observation_recorder(NetworkObservationRecorder::default()); + let error = transport.fetch(&client, request).await.unwrap_err(); + let curl_error = error.downcast_ref::().unwrap(); + assert!(curl_error.is_peer_failed_verification(), "{error:#}"); + let detail = curl_error + .extra_description() + .expect("libcurl should preserve its TLS error buffer"); + assert!(!detail.is_empty()); + let reason = match transport { + // The callback API returns the original error chain, which the + // CLI's fallback formatting must also preserve. + Transport::Buffered => format!("{error:#}"), + Transport::Html | Transport::Raw => { + let failure = error + .downcast_ref::() + .unwrap(); + assert_eq!( + failure.network_error_text(), + "net::ERR_CERT_AUTHORITY_INVALID" + ); + failure.reason().to_owned() + } + }; + assert!(reason.contains("curl request failed"), "{reason}"); + assert!(reason.contains("[60]"), "{reason}"); + assert!(reason.contains(detail), "{reason}"); + assert!(client.shutdown().is_clean()); + } + assert!(server.requests.lock().is_empty()); + Ok(()) +} + #[tokio::test] async fn https_proxy_uses_shared_dns_and_proxy_hostname_tls() -> Result<()> { let credentials = TlsCredentials::new()?; diff --git a/moli/src/app.rs b/moli/src/app.rs index a3f4a5133..6942d97fa 100644 --- a/moli/src/app.rs +++ b/moli/src/app.rs @@ -249,7 +249,7 @@ fn with_fetch_context(error: anyhow::Error, url: &str) -> anyhow::Error { } else if let Some(timeout) = error.downcast_ref::() { timeout.to_string() } else { - error.to_string() + format!("{error:#}") }; with_fetch_context_reason(error, url, reason) } @@ -314,9 +314,9 @@ mod tests { use std::time::Duration; #[test] - fn fetch_report_has_one_reason_line_without_rendering_the_source_chain() { + fn fetch_report_has_one_reason_line_including_the_source_chain() { let error = with_fetch_context( - anyhow::anyhow!("first failure line\nsecond failure line"), + anyhow::anyhow!("first failure line\nsecond failure line").context("request failed"), "https://example.test/", ); let mut report = Vec::new(); @@ -326,7 +326,7 @@ mod tests { assert_eq!( report, - "Error: failed to fetch `https://example.test/`\nReason: first failure line second failure line\n" + "Error: failed to fetch `https://example.test/`\nReason: request failed: first failure line second failure line\n" ); assert!(!report.contains("Caused by:")); } diff --git a/moli/tests/fetch_cli.rs b/moli/tests/fetch_cli.rs index eb2b79af1..154c8e436 100644 --- a/moli/tests/fetch_cli.rs +++ b/moli/tests/fetch_cli.rs @@ -3899,6 +3899,24 @@ fn cli_dump_json_keeps_transport_failures_as_process_errors() -> Result<()> { Ok(()) } +#[test] +fn cli_connection_failure_reports_curl_code_and_cause() -> Result<()> { + let listener = std::net::TcpListener::bind("127.0.0.1:0")?; + let address = listener.local_addr()?; + drop(listener); + let url = format!("http://{address}/connection-refused"); + + let output = run_fetch_cli(&url)?; + let stdout = clean_output(&output.stdout); + let stderr = clean_output(&output.stderr); + assert!(!output.status.success(), "stdout={stdout}\nstderr={stderr}"); + assert!(stdout.is_empty(), "stdout={stdout}"); + assert_single_fetch_failure_reason(&stderr, &url, "curl request failed"); + assert!(stderr.contains("[7]"), "stderr={stderr}"); + assert!(stderr.contains("Failed to connect"), "stderr={stderr}"); + Ok(()) +} + #[test] fn cli_timeout_reports_waiting_for_response_headers_as_one_reason() -> Result<()> { let runtime = tokio::runtime::Runtime::new()?;