fix(ecstore): retry transient walk dir stream errors (#3194)

* fix(ecstore): retry transient walk dir stream errors

Retry RemoteDisk walk_dir once when the first failure looks transport-related so restart-time HttpReader stream errors do not immediately count as hard listing failures.

Improve HttpReader request and stream error messages with method and URL context, and add regression coverage for retry recovery and diagnostics.

* fix(ecstore): avoid retry after partial walk dir stream

Stop retrying walk_dir after copy_stream_with_buffer has already written partial bytes into the destination writer.

Keep the retry only for open_walk_dir failures and add a regression test that proves partial stream failures are returned without issuing a second stream.
This commit is contained in:
houseme
2026-06-03 21:38:22 +08:00
committed by GitHub
parent ce6fcf39b1
commit 20bb5dc4a2
2 changed files with 233 additions and 20 deletions
+29 -7
View File
@@ -248,22 +248,24 @@ impl HttpReader {
let resp = request.send().await.map_err(|e| {
record_internode_error(track_internode_metrics, internode_operation);
Error::other(format!("HttpReader HTTP request error: {e}"))
Error::other(format!("HttpReader HTTP request error for {method} {url}: {e}"))
})?;
if resp.status().is_success().not() {
record_internode_error(track_internode_metrics, internode_operation);
return Err(Error::other(format!(
"HttpReader HTTP request failed with non-200 status {}",
resp.status()
"HttpReader HTTP request failed for {method} {url} with non-200 status {}",
resp.status(),
)));
}
record_internode_outgoing_request(track_internode_metrics, internode_operation);
let stream_error_url = url.clone();
let stream_error_method = method.clone();
let stream = resp.bytes_stream().map_err(move |e| {
record_internode_error(track_internode_metrics, internode_operation);
Error::other(format!("HttpReader stream error: {e}"))
Error::other(format!("HttpReader stream error for {stream_error_method} {stream_error_url}: {e}"))
});
Ok(Self {
@@ -837,10 +839,13 @@ mod tests {
assert_eq!(&first, b"hello");
let mut next = [0u8; 1];
let err = tokio::time::timeout(Duration::from_secs(1), reader.read(&mut next))
let read_result = tokio::time::timeout(Duration::from_secs(1), reader.read(&mut next))
.await
.expect("stall timeout should wake reader")
.expect_err("reader should return a timeout error");
.expect("stall timeout should wake reader");
let err = match read_result {
Ok(_) => panic!("reader should return a timeout error"),
Err(err) => err,
};
assert_eq!(err.kind(), io::ErrorKind::TimedOut);
handle.abort();
@@ -898,6 +903,23 @@ mod tests {
handle.abort();
}
#[tokio::test]
async fn http_reader_request_error_includes_method_and_url() {
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let addr = listener.local_addr().unwrap();
drop(listener);
let url = format!("http://{addr}/stream");
let err = match HttpReader::new(url.clone(), Method::GET, HeaderMap::new(), None).await {
Ok(_) => panic!("closed listener should trigger request error"),
Err(err) => err,
};
let err_text = err.to_string();
assert!(err_text.contains("HttpReader HTTP request error for GET"));
assert!(err_text.contains(&url));
}
#[test]
fn loopback_urls_bypass_proxy_selection() {
assert!(should_bypass_proxy_for_url("http://127.0.0.1:9000/stream"));