From 4f7319c75abdce89a9fe16d427b6c33d80ca5782 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 21 Oct 2026 03:56:24 +0000 Subject: [PATCH 1/2] Keep transport cause in registry lookup errors A failed upstream registry lookup quoted only reqwest's top-level text, "error sending request for url (...)", which drops the part that says what went wrong (refused, reset, DNS, TLS, timeout). The nightly poetry hosted-rollback flake in #1650 could not be classified for that reason, and a user whose hosted rollback is refused gets the same unhelpful message. Format the send and body-read errors in UpstreamClient::get_json and registry_fetch::download with api::client::network_error_detail, the helper the patch API client already uses. It appends the cause chain and redacts URLs. Add a test that a refused connection's error now says "refused". Refs #1650 Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012VpH5sUqHcYE49yNX1k2Fw --- crates/socket-patch-core/src/api/client.rs | 2 +- .../src/patch/redirect/upstream/client.rs | 25 +++++++++++++++++-- .../src/vendor/registry_fetch.rs | 3 ++- 3 files changed, 26 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/api/client.rs b/crates/socket-patch-core/src/api/client.rs index 502998b7c..aca4ca2a6 100644 --- a/crates/socket-patch-core/src/api/client.rs +++ b/crates/socket-patch-core/src/api/client.rs @@ -38,7 +38,7 @@ static MULTI_ORG_SHOWN: AtomicBool = AtomicBool::new(false); /// at "error sending request for url (...)", dropping the part a user can /// act on ("Connection refused", a DNS or TLS failure). Causes already /// spelled out by an outer message are skipped. -fn network_error_detail(e: &reqwest::Error) -> String { +pub(crate) fn network_error_detail(e: &reqwest::Error) -> String { // reqwest's text names the request URL: a grant URL, or one with // userinfo from `--api-url` / a proxy, is quoted redacted. let mut msg = redact_urls_in(&e.to_string()).into_owned(); diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs index 9313d6f51..b37efb669 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs @@ -8,6 +8,7 @@ use std::collections::HashMap; use serde_json::Value; use tokio::sync::{Mutex, Semaphore, SemaphorePermit}; +use crate::api::client::network_error_detail; use crate::vendor::registry_fetch::{ build_registry_client, npm_registry_base, pypi_json_api_base, RegistryClient, }; @@ -249,7 +250,7 @@ impl UpstreamClient { let resp = request .send() .await - .map_err(|e| format!("GET {url}: {e}"))?; + .map_err(|e| format!("GET {url}: {}", network_error_detail(&e)))?; let status = resp.status(); if !status.is_success() { return Err(format!("GET {url}: HTTP {status}")); @@ -257,7 +258,7 @@ impl UpstreamClient { let text = resp .text() .await - .map_err(|e| format!("reading {url}: {e}"))?; + .map_err(|e| format!("reading {url}: {}", network_error_detail(&e)))?; serde_json::from_str(&text).map_err(|e| format!("{url} is not JSON: {e}")) } @@ -902,6 +903,26 @@ mod tests { let h1 = go_mod_h1(b"module example.com/m\n"); assert!(h1.starts_with("h1:") && h1.ends_with('='), "{h1}"); } + /// #1650: a failed lookup names its transport cause, not just reqwest's + /// "error sending request for url (...)", so a refusal (and a CI flake) + /// says whether the registry was unreachable, reset or timed out. + #[tokio::test] + async fn a_failed_lookup_keeps_the_transport_cause() { + // A port that was just free: connecting is refused, deterministically. + let port = { + let l = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + l.local_addr().unwrap().port() + }; + let url = format!("http://127.0.0.1:{port}/pypi/six/1.16.0/json"); + let client = UpstreamClient::new(false); + let json = client.get_json(&url).await.expect_err("nothing listens"); + let bytes = client.download(&url).await.expect_err("nothing listens"); + for err in [json, bytes] { + assert!(err.starts_with(&format!("GET {url}: ")), "{err}"); + assert!(err.to_lowercase().contains("refused"), "{err}"); + } + } + // The npm cache key reads `SOCKET_NPM_REGISTRY`, which serial tests set. #[tokio::test] #[serial_test::serial] diff --git a/crates/socket-patch-core/src/vendor/registry_fetch.rs b/crates/socket-patch-core/src/vendor/registry_fetch.rs index 91cabf5ff..3fe17d9da 100644 --- a/crates/socket-patch-core/src/vendor/registry_fetch.rs +++ b/crates/socket-patch-core/src/vendor/registry_fetch.rs @@ -6,6 +6,7 @@ use base64::Engine as _; use sha1::Sha1; use sha2::{Digest, Sha256, Sha384, Sha512}; +use crate::api::client::network_error_detail; use crate::api::retry::ApiTimeouts; use crate::constants::USER_AGENT; use crate::patch::apply::is_safe_relative_subpath; @@ -957,7 +958,7 @@ async fn download_unredacted(client: &reqwest::Client, url: &str) -> Result Date: Wed, 21 Oct 2026 21:19:01 +0000 Subject: [PATCH 2/2] Hold the port in the cause-chain test The test dropped its TcpListener before connecting, so another process could bind the freed port and the two requests could see a different failure or a real response. Serve the port from a held loopback listener that reads each request head and hangs up, bound each request with a 30 s deadline, and assert hyper's "connection closed before message completed" cause instead of "refused". Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_012VpH5sUqHcYE49yNX1k2Fw --- .../src/patch/redirect/upstream/client.rs | 44 +++++++++++++++---- 1 file changed, 35 insertions(+), 9 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs index b37efb669..c1caa748b 100644 --- a/crates/socket-patch-core/src/patch/redirect/upstream/client.rs +++ b/crates/socket-patch-core/src/patch/redirect/upstream/client.rs @@ -908,18 +908,44 @@ mod tests { /// says whether the registry was unreachable, reset or timed out. #[tokio::test] async fn a_failed_lookup_keeps_the_transport_cause() { - // A port that was just free: connecting is refused, deterministically. - let port = { - let l = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - l.local_addr().unwrap().port() - }; - let url = format!("http://127.0.0.1:{port}/pypi/six/1.16.0/json"); + use tokio::io::AsyncReadExt; + + // A server that holds its port for the whole test (so nothing else + // can bind it) and hangs up on every request after reading its head: + // the client sees the connection close before any response. + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!( + "http://{}/pypi/six/1.16.0/json", + listener.local_addr().unwrap() + ); + tokio::spawn(async move { + while let Ok((mut sock, _)) = listener.accept().await { + let mut head = Vec::new(); + let mut chunk = [0u8; 1024]; + while !head.windows(4).any(|w| w == b"\r\n\r\n") { + match sock.read(&mut chunk).await { + Ok(0) | Err(_) => break, + Ok(n) => head.extend_from_slice(&chunk[..n]), + } + } + } + }); let client = UpstreamClient::new(false); - let json = client.get_json(&url).await.expect_err("nothing listens"); - let bytes = client.download(&url).await.expect_err("nothing listens"); + let deadline = std::time::Duration::from_secs(30); + let json = tokio::time::timeout(deadline, client.get_json(&url)) + .await + .expect("the lookup ends once the server hangs up") + .expect_err("the server never answers"); + let bytes = tokio::time::timeout(deadline, client.download(&url)) + .await + .expect("the download ends once the server hangs up") + .expect_err("the server never answers"); for err in [json, bytes] { assert!(err.starts_with(&format!("GET {url}: ")), "{err}"); - assert!(err.to_lowercase().contains("refused"), "{err}"); + assert!( + err.contains("connection closed before message completed"), + "the cause chain must be kept: {err}" + ); } }