Skip to content

Commit

Permalink
ObjectStore: make error msg thrown from retry more detailed (#5012)
Browse files Browse the repository at this point in the history
* optimize error msg for better debugging.

* fix unit test.

* fix fmt.
  • Loading branch information
Rachelint authored Nov 1, 2023
1 parent ec788e1 commit 7873500
Showing 1 changed file with 19 additions and 6 deletions.
25 changes: 19 additions & 6 deletions object_store/src/client/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ pub enum Error {
body: Option<String>,
},

#[snafu(display("Error after {retries} retries: {source}"))]
#[snafu(display("Error after {retries} retries in {elapsed:?}, max_retries:{max_retries}, retry_timeout:{retry_timeout:?}, source:{source}"))]
Reqwest {
retries: usize,
max_retries: usize,
elapsed: Duration,
retry_timeout: Duration,
source: reqwest::Error,
},
}
Expand Down Expand Up @@ -198,7 +201,6 @@ impl RetryExt for reqwest::RequestBuilder {
}
Err(e) => {
let status = r.status();

if retries == max_retries
|| now.elapsed() > retry_timeout
|| !status.is_server_error() {
Expand All @@ -214,12 +216,18 @@ impl RetryExt for reqwest::RequestBuilder {
Err(e) => {
Error::Reqwest {
retries,
max_retries,
elapsed: now.elapsed(),
retry_timeout,
source: e,
}
}
}
false => Error::Reqwest {
retries,
max_retries,
elapsed: now.elapsed(),
retry_timeout,
source: e,
}
});
Expand Down Expand Up @@ -248,6 +256,9 @@ impl RetryExt for reqwest::RequestBuilder {

return Err(Error::Reqwest {
retries,
max_retries,
elapsed: now.elapsed(),
retry_timeout,
source: e,
})
}
Expand Down Expand Up @@ -408,9 +419,8 @@ mod tests {

let e = do_request().await.unwrap_err().to_string();
assert!(
e.starts_with(
"Error after 2 retries: HTTP status server error (502 Bad Gateway) for url"
),
e.contains("Error after 2 retries in") &&
e.contains("max_retries:2, retry_timeout:1000s, source:HTTP status server error (502 Bad Gateway) for url"),
"{e}"
);

Expand All @@ -425,7 +435,10 @@ mod tests {
}
let e = do_request().await.unwrap_err().to_string();
assert!(
e.starts_with("Error after 2 retries: error sending request for url"),
e.contains("Error after 2 retries in")
&& e.contains(
"max_retries:2, retry_timeout:1000s, source:error sending request for url"
),
"{e}"
);

Expand Down

0 comments on commit 7873500

Please sign in to comment.