wrap requests.Response to ResponseInfo with middleware for reusing need_retry

Author: lihsai0

qiniu/http/client.py

@@ -13,6 +13,10 @@ def __init__(self, middlewares=None, send_opts=None):
         self.middlewares = [] if middlewares is None else middlewares
         self.send_opts = {} if send_opts is None else send_opts
 
+    def _wrap_send(self, req, **kwargs):
+        resp = self.session.send(req.prepare(), **kwargs)
+        return ResponseInfo(resp, None)
+
     def send_request(self, request, middlewares=None, **kwargs):
         """
 
@@ -55,23 +59,20 @@ def send_request(self, request, middlewares=None, **kwargs):
         try:
             handle = compose_middleware(
                 mw_ls,
-                lambda req: self.session.send(req.prepare(), **kwargs)
+                lambda req: self._wrap_send(req, **kwargs)
             )
-            resp = handle(request)
+            resp_info = handle(request)
         except Exception as e:
             return None, ResponseInfo(None, e)
 
-        # wrap resp
-        resp_info = ResponseInfo(resp)
+        # if ok try dump response info to dict from json
         if not resp_info.ok:
             return None, resp_info
 
-        # try dump response info to dict from json
-        resp.encoding = "utf-8"
         try:
-            ret = resp.json()
+            ret = resp_info.json()
         except ValueError:
-            logging.debug("response body decode error: %s" % resp.text)
+            logging.debug("response body decode error: %s" % resp_info.text_body)
             ret = {}
         return ret, resp_info
 

qiniu/http/middleware/base.py

@@ -6,10 +6,10 @@ def compose_middleware(middlewares, handle):
     """
     Args:
         middlewares (list[Middleware]): Middlewares
-        handle ((requests.Request) -> requests.Response): The send request handle
+        handle ((requests.Request) -> qiniu.http.response.ResponseInfo): The send request handle
 
     Returns:
-        (requests.Request) -> requests.Response: Composed handle
+        (requests.Request) -> qiniu.http.response.ResponseInfo: Composed handle
 
     """
     middlewares.reverse()
@@ -27,7 +27,7 @@ def __call__(self, request, nxt):
         """
         Args:
             request (requests.Request):
-            nxt ((requests.Request) -> requests.Response):
+            nxt ((requests.Request) -> qiniu.http.response.ResponseInfo):
 
         Returns:
             requests.Response:

qiniu/http/middleware/retry_domains.py

@@ -34,6 +34,9 @@ def _get_changed_url(url, domain):
         backup_netloc += domain
         if url_parse_result.port is not None:
             backup_netloc += ':' + str(url_parse_result.port)
+
+        # the _replace is a public method. start with `_` just to prevent conflicts with field names
+        # see namedtuple docs
         url_parse_result = url_parse_result._replace(
             netloc=backup_netloc
         )
@@ -54,25 +57,25 @@ def _should_retry(self, resp, req):
         if callable(self.retry_condition):
             return self.retry_condition(resp, req)
 
-        return resp is None or not resp.ok
+        return resp is None or resp.need_retry()
 
     def __call__(self, request, nxt):
-        resp, err = None, None
+        resp_info, err = None, None
         url_parse_result = urlparse(request.url)
 
         for backup_domain in [str(url_parse_result.hostname)] + self.backup_domains:
             request.url = RetryDomainsMiddleware._get_changed_url(request.url, backup_domain)
             self.retried_times = 0
 
             while self.retried_times < self.max_retry_times:
-                resp, err = RetryDomainsMiddleware._try_nxt(request, nxt)
+                resp_info, err = RetryDomainsMiddleware._try_nxt(request, nxt)
                 self.retried_times += 1
-                if not self._should_retry(resp, request):
+                if not self._should_retry(resp_info, request):
                     if err is not None:
                         raise err
-                    return resp
+                    return resp_info
 
         if err is not None:
             raise err
 
-        return resp
+        return resp_info

qiniu/http/response.py

@@ -46,16 +46,25 @@ def ok(self):
         return self.status_code // 100 == 2
 
     def need_retry(self):
-        if self.__response is None or self.req_id is None:
-            return True
-        code = self.status_code
-        if (code // 100 == 5 and code != 579) or code == 996:
-            return True
-        return False
+        if 0 < self.status_code < 500:
+            return False
+        # https://developer.qiniu.com/fusion/kb/1352/the-http-request-return-a-status-code
+        if self.status_code in [
+            501, 509, 573, 579, 608, 612, 614, 616, 618, 630, 631, 632, 640, 701
+        ]:
+            return False
+        return True
 
     def connect_failed(self):
         return self.__response is None or self.req_id is None
 
+    def json(self):
+        try:
+            self.__response.encoding = "utf-8"
+            return self.__response.json()
+        except Exception:
+            return {}
+
     def __str__(self):
         if is_py2:
             return ', '.join(

qiniu/region.py

@@ -231,22 +231,12 @@ def bucket_hosts(self, ak, bucket):
         uc_backup_retry_times = get_default('default_uc_backup_retry_times')
         url = "{0}/v4/query?ak={1}&bucket={2}".format(uc_host, ak, bucket)
 
-        def retry_condition(resp, _):
-            if resp is None:
-                return True
-            if resp.status_code in [612, 631]:
-                # 612 is app / accesskey is not found
-                # 631 is no such bucket
-                return False
-            return not resp.ok
-
         ret, _resp = qn_http_client.get(
             url,
             middlewares=[
                 RetryDomainsMiddleware(
                     backup_domains=uc_backup_hosts,
                     max_retry_times=uc_backup_retry_times,
-                    retry_condition=retry_condition
                 )
             ]
         )

test_qiniu.py

@@ -96,12 +96,36 @@ def __call__(self, request, nxt):
 
 
 class HttpTest(unittest.TestCase):
+    def test_response_need_retry(self):
+        _ret, resp_info = qn_http_client.get('https://qiniu.com/index.html')
+
+        resp_info.req_id = 'mocked-req-id'
+        resp_info.error = None
+
+        def gen_case(code):
+            if 0 < code < 500:
+                return code, False
+            if code in [
+                501, 509, 573, 579, 608, 612, 614, 616, 618, 630, 631, 632, 640, 701
+            ]:
+                return code, False
+            return code, True
+
+        cases = [
+            gen_case(i) for i in range(-1, 800)
+        ]
+
+        for test_code, should_retry in cases:
+            resp_info.status_code = test_code
+            assert_msg = '{0} should{1} retry'.format(test_code, '' if should_retry else ' NOT')
+            assert resp_info.need_retry() == should_retry, assert_msg
+
     def test_middlewares(self):
         rec_ls = []
         mw_a = MiddlewareRecorder(rec_ls, 'A')
         mw_b = MiddlewareRecorder(rec_ls, 'B')
         qn_http_client.get(
-            "https://qiniu.com/index.html",
+            'https://qiniu.com/index.html',
             middlewares=[
                 mw_a,
                 mw_b