From 82105830e7a621c54cf51dbf1c3619e5f12f99f1 Mon Sep 17 00:00:00 2001
From: Xuanwo <github@xuanwo.io>
Date: Tue, 14 May 2024 17:59:20 +0800
Subject: [PATCH] fix(raw): Allow retrying request while decoding response
 failed (#4612)

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

Signed-off-by: Xuanwo <github@xuanwo.io>

* inline some code

Signed-off-by: Xuanwo <github@xuanwo.io>

* Polish code

Signed-off-by: Xuanwo <github@xuanwo.io>

---------

Signed-off-by: Xuanwo <github@xuanwo.io>
---
 core/src/raw/http_util/client.rs | 35 ++++++++++++++------------------
 core/src/types/error.rs          | 10 +++++++++
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/core/src/raw/http_util/client.rs b/core/src/raw/http_util/client.rs
index fea7ac036561..1842e1a3db43 100644
--- a/core/src/raw/http_util/client.rs
+++ b/core/src/raw/http_util/client.rs
@@ -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.
@@ -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)
             })?;
 
@@ -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()
+}
diff --git a/core/src/types/error.rs b/core/src/types/error.rs
index 856dfc0b3ef5..7ac8baa2fcd7 100644
--- a/core/src/types/error.rs
+++ b/core/src/types/error.rs
@@ -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.