Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(raw): Allow retrying request while decoding response failed #4612

Merged
merged 3 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 15 additions & 20 deletions core/src/raw/http_util/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,28 +106,11 @@ impl HttpClient {
}

let mut resp = req_builder.send().await.map_err(|err| {
let is_temporary = !(
// Builder related error should not be retried.
err.is_builder() ||
// Error returned by RedirectPolicy.
//
// Don't retry error if we redirect too many.
err.is_redirect() ||
// We never use `Response::error_for_status`, just don't allow retry.
//
// Status should be checked by our services.
err.is_status()
);

let mut oerr = Error::new(ErrorKind::Unexpected, "send http request")
Error::new(ErrorKind::Unexpected, "send http request")
.with_operation("http_util::Client::send")
.with_context("url", uri.to_string())
.set_source(err);
if is_temporary {
oerr = oerr.set_temporary();
}

oerr
.with_temporary(is_temporary_error(&err))
.set_source(err)
})?;

// Get content length from header so that we can check it.
Expand Down Expand Up @@ -162,7 +145,9 @@ impl HttpClient {
.await
.map_err(|err| {
Error::new(ErrorKind::Unexpected, "read data from http response")
.with_operation("http_util::Client::send")
.with_context("url", uri.to_string())
.with_temporary(is_temporary_error(&err))
.set_source(err)
})?;

Expand Down Expand Up @@ -194,3 +179,13 @@ fn check(expect: u64, actual: u64) -> Result<()> {
.set_temporary()),
}
}

#[inline]
fn is_temporary_error(err: &reqwest::Error) -> bool {
// error sending request
err.is_request()||
// request or response body error
err.is_body() ||
// error decoding response body, for example, connection reset.
err.is_decode()
}
10 changes: 10 additions & 0 deletions core/src/types/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,16 @@ impl Error {
self
}

/// Set temporary status for error by given temporary.
///
/// By set temporary, we indicate this error is retryable.
pub(crate) fn with_temporary(mut self, temporary: bool) -> Self {
if temporary {
self.status = ErrorStatus::Temporary;
}
self
}

/// Set persistent status for error.
///
/// By setting persistent, we indicate the retry should be stopped.
Expand Down
Loading