Skip to content

Commit 3837689

Browse files
committed
Auto merge of #11878 - ehuss:header-diagnostics, r=weihanglo
Add more information to HTTP errors to help with debugging. This adds some extra information to the HTTP error message about some headers and the remote IP address that could potentially be useful in diagnosing issues. Closes #8691
2 parents 96f8d6c + 84bcf0e commit 3837689

File tree

6 files changed

+268
-32
lines changed

6 files changed

+268
-32
lines changed

src/cargo/core/package.rs

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use crate::core::{Dependency, Manifest, PackageId, SourceId, Target};
2626
use crate::core::{SourceMap, Summary, Workspace};
2727
use crate::ops;
2828
use crate::util::config::PackageCacheLock;
29-
use crate::util::errors::{CargoResult, HttpNotSuccessful};
29+
use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS};
3030
use crate::util::interning::InternedString;
3131
use crate::util::network::retry::{Retry, RetryResult};
3232
use crate::util::network::sleep::SleepTracker;
@@ -379,6 +379,9 @@ struct Download<'cfg> {
379379
/// Actual downloaded data, updated throughout the lifetime of this download.
380380
data: RefCell<Vec<u8>>,
381381

382+
/// HTTP headers for debugging.
383+
headers: RefCell<Vec<String>>,
384+
382385
/// The URL that we're downloading from, cached here for error messages and
383386
/// reenqueuing.
384387
url: String,
@@ -762,6 +765,19 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
762765
});
763766
Ok(buf.len())
764767
})?;
768+
handle.header_function(move |data| {
769+
tls::with(|downloads| {
770+
if let Some(downloads) = downloads {
771+
// Headers contain trailing \r\n, trim them to make it easier
772+
// to work with.
773+
let h = String::from_utf8_lossy(data).trim().to_string();
774+
if DEBUG_HEADERS.iter().any(|p| h.starts_with(p)) {
775+
downloads.pending[&token].0.headers.borrow_mut().push(h);
776+
}
777+
}
778+
});
779+
true
780+
})?;
765781

766782
handle.progress(true)?;
767783
handle.progress_function(move |dl_total, dl_cur, _, _| {
@@ -787,6 +803,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
787803
let dl = Download {
788804
token,
789805
data: RefCell::new(Vec::new()),
806+
headers: RefCell::new(Vec::new()),
790807
id,
791808
url,
792809
descriptor,
@@ -826,6 +843,7 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
826843
.remove(&token)
827844
.expect("got a token for a non-in-progress transfer");
828845
let data = mem::take(&mut *dl.data.borrow_mut());
846+
let headers = mem::take(&mut *dl.headers.borrow_mut());
829847
let mut handle = self.set.multi.remove(handle)?;
830848
self.pending_ids.remove(&dl.id);
831849

@@ -862,12 +880,12 @@ impl<'a, 'cfg> Downloads<'a, 'cfg> {
862880

863881
let code = handle.response_code()?;
864882
if code != 200 && code != 0 {
865-
let url = handle.effective_url()?.unwrap_or(url);
866-
return Err(HttpNotSuccessful {
867-
code,
868-
url: url.to_string(),
869-
body: data,
870-
}
883+
return Err(HttpNotSuccessful::new_from_handle(
884+
&mut handle,
885+
&url,
886+
data,
887+
headers,
888+
)
871889
.into());
872890
}
873891
Ok(data)

src/cargo/sources/registry/http_remote.rs

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::ops::{self};
77
use crate::sources::registry::download;
88
use crate::sources::registry::MaybeLock;
99
use crate::sources::registry::{LoadResponse, RegistryConfig, RegistryData};
10-
use crate::util::errors::{CargoResult, HttpNotSuccessful};
10+
use crate::util::errors::{CargoResult, HttpNotSuccessful, DEBUG_HEADERS};
1111
use crate::util::network::retry::{Retry, RetryResult};
1212
use crate::util::network::sleep::SleepTracker;
1313
use crate::util::{auth, Config, Filesystem, IntoUrl, Progress, ProgressStyle};
@@ -142,6 +142,7 @@ struct Headers {
142142
last_modified: Option<String>,
143143
etag: Option<String>,
144144
www_authenticate: Vec<String>,
145+
others: Vec<String>,
145146
}
146147

147148
enum StatusCode {
@@ -287,13 +288,13 @@ impl<'cfg> HttpRegistry<'cfg> {
287288
304 => StatusCode::NotModified,
288289
401 => StatusCode::Unauthorized,
289290
404 | 410 | 451 => StatusCode::NotFound,
290-
code => {
291-
let url = handle.effective_url()?.unwrap_or(&url);
292-
return Err(HttpNotSuccessful {
293-
code,
294-
url: url.to_owned(),
295-
body: data,
296-
}
291+
_ => {
292+
return Err(HttpNotSuccessful::new_from_handle(
293+
&mut handle,
294+
&url,
295+
data,
296+
download.header_map.take().others,
297+
)
297298
.into());
298299
}
299300
};
@@ -546,6 +547,8 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
546547
code: 401,
547548
body: result.data,
548549
url: self.full_url(path),
550+
ip: None,
551+
headers: result.header_map.others,
549552
}
550553
.into());
551554
if self.auth_required {
@@ -665,7 +668,11 @@ impl<'cfg> RegistryData for HttpRegistry<'cfg> {
665668
LAST_MODIFIED => header_map.last_modified = Some(value.to_string()),
666669
ETAG => header_map.etag = Some(value.to_string()),
667670
WWW_AUTHENTICATE => header_map.www_authenticate.push(value.to_string()),
668-
_ => {}
671+
_ => {
672+
if DEBUG_HEADERS.iter().any(|prefix| tag.starts_with(prefix)) {
673+
header_map.others.push(format!("{tag}: {value}"));
674+
}
675+
}
669676
}
670677
}
671678
});

src/cargo/util/errors.rs

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,100 @@
11
#![allow(unknown_lints)]
22

33
use anyhow::Error;
4-
use std::fmt;
4+
use curl::easy::Easy;
5+
use std::fmt::{self, Write};
56
use std::path::PathBuf;
67

78
use super::truncate_with_ellipsis;
89

910
pub type CargoResult<T> = anyhow::Result<T>;
1011

12+
/// These are headers that are included in error messages to help with
13+
/// diagnosing issues.
14+
pub const DEBUG_HEADERS: &[&str] = &[
15+
// This is the unique ID that identifies the request in CloudFront which
16+
// can be used for looking at the AWS logs.
17+
"x-amz-cf-id",
18+
// This is the CloudFront POP (Point of Presence) that identifies the
19+
// region where the request was routed. This can help identify if an issue
20+
// is region-specific.
21+
"x-amz-cf-pop",
22+
// The unique token used for troubleshooting S3 requests via AWS logs or support.
23+
"x-amz-request-id",
24+
// Another token used in conjunction with x-amz-request-id.
25+
"x-amz-id-2",
26+
// Whether or not there was a cache hit or miss (both CloudFront and Fastly).
27+
"x-cache",
28+
// The cache server that processed the request (Fastly).
29+
"x-served-by",
30+
];
31+
1132
#[derive(Debug)]
1233
pub struct HttpNotSuccessful {
1334
pub code: u32,
1435
pub url: String,
36+
pub ip: Option<String>,
1537
pub body: Vec<u8>,
38+
pub headers: Vec<String>,
1639
}
1740

18-
impl fmt::Display for HttpNotSuccessful {
19-
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
41+
impl HttpNotSuccessful {
42+
pub fn new_from_handle(
43+
handle: &mut Easy,
44+
initial_url: &str,
45+
body: Vec<u8>,
46+
headers: Vec<String>,
47+
) -> HttpNotSuccessful {
48+
let ip = handle.primary_ip().ok().flatten().map(|s| s.to_string());
49+
let url = handle
50+
.effective_url()
51+
.ok()
52+
.flatten()
53+
.unwrap_or(initial_url)
54+
.to_string();
55+
HttpNotSuccessful {
56+
code: handle.response_code().unwrap_or(0),
57+
url,
58+
ip,
59+
body,
60+
headers,
61+
}
62+
}
63+
64+
/// Renders the error in a compact form.
65+
pub fn display_short(&self) -> String {
66+
self.render(false)
67+
}
68+
69+
fn render(&self, show_headers: bool) -> String {
70+
let mut result = String::new();
2071
let body = std::str::from_utf8(&self.body)
2172
.map(|s| truncate_with_ellipsis(s, 512))
2273
.unwrap_or_else(|_| format!("[{} non-utf8 bytes]", self.body.len()));
2374

2475
write!(
25-
f,
26-
"failed to get successful HTTP response from `{}`, got {}\nbody:\n{body}",
27-
self.url, self.code
76+
result,
77+
"failed to get successful HTTP response from `{}`",
78+
self.url
2879
)
80+
.unwrap();
81+
if let Some(ip) = &self.ip {
82+
write!(result, " ({ip})").unwrap();
83+
}
84+
write!(result, ", got {}\n", self.code).unwrap();
85+
if show_headers {
86+
if !self.headers.is_empty() {
87+
write!(result, "debug headers:\n{}\n", self.headers.join("\n")).unwrap();
88+
}
89+
}
90+
write!(result, "body:\n{body}").unwrap();
91+
result
92+
}
93+
}
94+
95+
impl fmt::Display for HttpNotSuccessful {
96+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
97+
f.write_str(&self.render(true))
2998
}
3099
}
31100

src/cargo/util/network/retry.rs

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,13 @@ impl<'a> Retry<'a> {
4444
pub fn r#try<T>(&mut self, f: impl FnOnce() -> CargoResult<T>) -> RetryResult<T> {
4545
match f() {
4646
Err(ref e) if maybe_spurious(e) && self.retries < self.max_retries => {
47+
let err_msg = e
48+
.downcast_ref::<HttpNotSuccessful>()
49+
.map(|http_err| http_err.display_short())
50+
.unwrap_or_else(|| e.root_cause().to_string());
4751
let msg = format!(
48-
"spurious network error ({} tries remaining): {}",
52+
"spurious network error ({} tries remaining): {err_msg}",
4953
self.max_retries - self.retries,
50-
e.root_cause(),
5154
);
5255
if let Err(e) = self.config.shell().warn(msg) {
5356
return RetryResult::Err(e);
@@ -150,13 +153,17 @@ fn with_retry_repeats_the_call_then_works() {
150153
let error1 = HttpNotSuccessful {
151154
code: 501,
152155
url: "Uri".to_string(),
156+
ip: None,
153157
body: Vec::new(),
158+
headers: Vec::new(),
154159
}
155160
.into();
156161
let error2 = HttpNotSuccessful {
157162
code: 502,
158163
url: "Uri".to_string(),
164+
ip: None,
159165
body: Vec::new(),
166+
headers: Vec::new(),
160167
}
161168
.into();
162169
let mut results: Vec<CargoResult<()>> = vec![Ok(()), Err(error1), Err(error2)];
@@ -175,13 +182,17 @@ fn with_retry_finds_nested_spurious_errors() {
175182
let error1 = anyhow::Error::from(HttpNotSuccessful {
176183
code: 501,
177184
url: "Uri".to_string(),
185+
ip: None,
178186
body: Vec::new(),
187+
headers: Vec::new(),
179188
});
180189
let error1 = anyhow::Error::from(error1.context("A non-spurious wrapping err"));
181190
let error2 = anyhow::Error::from(HttpNotSuccessful {
182191
code: 502,
183192
url: "Uri".to_string(),
193+
ip: None,
184194
body: Vec::new(),
195+
headers: Vec::new(),
185196
});
186197
let error2 = anyhow::Error::from(error2.context("A second chained error"));
187198
let mut results: Vec<CargoResult<()>> = vec![Ok(()), Err(error1), Err(error2)];
@@ -199,7 +210,9 @@ fn default_retry_schedule() {
199210
Err(anyhow::Error::from(HttpNotSuccessful {
200211
code: 500,
201212
url: "Uri".to_string(),
213+
ip: None,
202214
body: Vec::new(),
215+
headers: Vec::new(),
203216
}))
204217
};
205218
let config = Config::default().unwrap();

0 commit comments

Comments
 (0)