Skip to content

Commit

Permalink
fix(raw): Allow retrying request while decoding response failed (apac…
Browse files Browse the repository at this point in the history
…he#4612)

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

Signed-off-by: Xuanwo <[email protected]>

* inline some code

Signed-off-by: Xuanwo <[email protected]>

* Polish code

Signed-off-by: Xuanwo <[email protected]>

---------

Signed-off-by: Xuanwo <[email protected]>
  • Loading branch information
Xuanwo authored and George-Miao committed Jun 5, 2024
1 parent 14193ca commit 8210583
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 20 deletions.
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

0 comments on commit 8210583

Please sign in to comment.