SG-35165 Retry on URLError (#342)

* Retry on URLError

* Fix test

* Reduce attempt

* Use constant

Author: carlos-villavicencio-adsk

shotgun_api3/shotgun.py

@@ -500,6 +500,8 @@ class Shotgun(object):
         r"(\D?([01]\d|2[0-3])\D?([0-5]\d)\D?([0-5]\d)?\D?(\d{3})?)?$")
 
     _MULTIPART_UPLOAD_CHUNK_SIZE = 20000000
+    MAX_ATTEMPTS = 3    # Retries on failure
+    BACKOFF = 0.75      # Seconds to wait before retry, times the attempt number
 
     def __init__(self,
                  base_url,
@@ -3431,10 +3433,7 @@ def _call_rpc(self, method, params, include_auth_params=True, first=False):
             req_headers["locale"] = "auto"
 
         attempt = 1
-        max_attempts = 4 # Three retries on failure
-        backoff = 0.75 # Seconds to wait before retry, times the attempt number
-
-        while attempt <= max_attempts:
+        while attempt <= self.MAX_ATTEMPTS:
             http_status, resp_headers, body = self._make_call(
                 "POST",
                 self.config.api_path,
@@ -3452,9 +3451,9 @@ def _call_rpc(self, method, params, include_auth_params=True, first=False):
                 # We've seen some rare instances of PTR returning 502 for issues that
                 # appear to be caused by something internal to PTR. We're going to
                 # allow for limited retries for those specifically.
-                if attempt != max_attempts and e.errcode in [502, 504]:
+                if attempt != self.MAX_ATTEMPTS and e.errcode in [502, 504]:
                     LOG.debug("Got a 502 or 504 response. Waiting and retrying...")
-                    time.sleep(float(attempt) * backoff)
+                    time.sleep(float(attempt) * self.BACKOFF)
                     attempt += 1
                     continue
                 elif e.errcode == 403:
@@ -4143,28 +4142,31 @@ def _upload_data_to_storage(self, data, content_type, size, storage_url):
         request.get_method = lambda: "PUT"
 
         attempt = 1
-        max_attempts = 4  # Three retries on failure
-        backoff = 0.75  # Seconds to wait before retry, times the attempt number
-
-        while attempt <= max_attempts:
+        while attempt <= self.MAX_ATTEMPTS:
             try:
                 result = self._make_upload_request(request, opener)
 
                 LOG.debug("Completed request to %s" % request.get_method())
 
             except urllib.error.HTTPError as e:
-                if attempt != max_attempts and e.code in [500, 503]:
+                if attempt != self.MAX_ATTEMPTS and e.code in [500, 503]:
                     LOG.debug("Got a %s response. Waiting and retrying..." % e.code)
-                    time.sleep(float(attempt) * backoff)
+                    time.sleep(float(attempt) * self.BACKOFF)
                     attempt += 1
                     continue
                 elif e.code in [500, 503]:
                     raise ShotgunError("Got a %s response when uploading to %s: %s" % (e.code, storage_url, e))
                 else:
                     raise ShotgunError("Unanticipated error occurred uploading to %s: %s" % (storage_url, e))
-
+            except urllib.error.URLError as e:
+                LOG.debug("Got a '%s' response. Waiting and retrying..." % e)
+                time.sleep(float(attempt) * self.BACKOFF)
+                attempt += 1
+                continue
             else:
                 break
+        else:
+            raise ShotgunError("Max attemps limit reached.")
 
         etag = result.info()["Etag"]
         LOG.debug("Part upload completed successfully.")
@@ -4250,19 +4252,28 @@ def _send_form(self, url, params):
 
         opener = self._build_opener(FormPostHandler)
 
-        # Perform the request
-        try:
-            resp = opener.open(url, params)
-            result = resp.read()
-            # response headers are in str(resp.info()).splitlines()
-        except urllib.error.HTTPError as e:
-            if e.code == 500:
-                raise ShotgunError("Server encountered an internal error. "
-                                   "\n%s\n(%s)\n%s\n\n" % (url, self._sanitize_auth_params(params), e))
-            else:
-                raise ShotgunError("Unanticipated error occurred %s" % (e))
+        attempt = 1
+        while attempt <= self.MAX_ATTEMPTS:
+            # Perform the request
+            try:
+                resp = opener.open(url, params)
+                result = resp.read()
+                # response headers are in str(resp.info()).splitlines()
+            except urllib.error.URLError as e:
+                LOG.debug("Got a %s response. Waiting and retrying..." % e)
+                time.sleep(float(attempt) * self.BACKOFF)
+                attempt += 1
+                continue
+            except urllib.error.HTTPError as e:
+                if e.code == 500:
+                    raise ShotgunError("Server encountered an internal error. "
+                                    "\n%s\n(%s)\n%s\n\n" % (url, self._sanitize_auth_params(params), e))
+                else:
+                    raise ShotgunError("Unanticipated error occurred %s" % (e))
 
-        return six.ensure_text(result)
+            return six.ensure_text(result)
+        else:
+            raise ShotgunError("Max attemps limit reached.")
 
 
 class CACertsHTTPSConnection(http_client.HTTPConnection):

tests/base.py

@@ -150,6 +150,8 @@ def _setup_mock(self, s3_status_code_error=503):
         # create the server caps directly to say we have the correct version
         self.sg._server_caps = ServerCapabilities(self.sg.config.server,
                                                   {"version": [2, 4, 0]})
+        # prevent waiting for backoff
+        self.sg.BACKOFF = 0
 
     def _mock_http(self, data, headers=None, status=None):
         """Setup a mock response from the PTR server.

tests/test_client.py

@@ -438,9 +438,9 @@ def test_call_rpc(self):
         self._mock_http(d, status=(502, "bad gateway"))
         self.assertRaises(api.ProtocolError, self.sg._call_rpc, "list", a)
         self.assertEqual(
-            4,
+            self.sg.MAX_ATTEMPTS,
             self.sg._http_request.call_count,
-            "Call is repeated up to 3 times",
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times",
         )
 
         # 504
@@ -449,9 +449,9 @@ def test_call_rpc(self):
         self._mock_http(d, status=(504, "gateway timeout"))
         self.assertRaises(api.ProtocolError, self.sg._call_rpc, "list", a)
         self.assertEqual(
-            4,
+            self.sg.MAX_ATTEMPTS,
             self.sg._http_request.call_count,
-            "Call is repeated up to 3 times",
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times",
         )
 
     def test_upload_s3_503(self):
@@ -462,7 +462,6 @@ def test_upload_s3_503(self):
         storage_url = "http://foo.com/"
         path = os.path.abspath(os.path.expanduser(
             os.path.join(this_dir, "sg_logo.jpg")))
-        max_attempts = 4  # Max retries to S3 server attempts
         # Expected HTTPError exception error message
         expected = "The server is currently down or to busy to reply." \
                    "Please try again later."
@@ -474,8 +473,8 @@ def test_upload_s3_503(self):
             self.sg._upload_file_to_storage(path, storage_url)
         # Test the max retries attempt
         self.assertTrue(
-            max_attempts == self.sg._make_upload_request.call_count,
-            "Call is repeated up to 3 times")
+            self.sg.MAX_ATTEMPTS == self.sg._make_upload_request.call_count,
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times")
 
     def test_upload_s3_500(self):
         """
@@ -486,7 +485,6 @@ def test_upload_s3_500(self):
         storage_url = "http://foo.com/"
         path = os.path.abspath(os.path.expanduser(
             os.path.join(this_dir, "sg_logo.jpg")))
-        max_attempts = 4  # Max retries to S3 server attempts
         # Expected HTTPError exception error message
         expected = "The server is currently down or to busy to reply." \
                    "Please try again later."
@@ -498,8 +496,70 @@ def test_upload_s3_500(self):
             self.sg._upload_file_to_storage(path, storage_url)
         # Test the max retries attempt
         self.assertTrue(
-            max_attempts == self.sg._make_upload_request.call_count,
-            "Call is repeated up to 3 times")
+            self.sg.MAX_ATTEMPTS == self.sg._make_upload_request.call_count,
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times")
+    
+    def test_upload_s3_urlerror__get_attachment_upload_info(self):
+        """
+        Test URLError response is retried when invoking _send_form
+        """
+        mock_opener = mock.Mock()
+        mock_opener.return_value.open.side_effect = urllib.error.URLError(
+            "[WinError 10054] An existing connection was forcibly closed by the remote host"
+        )
+        self.sg._build_opener = mock_opener
+        this_dir, _ = os.path.split(__file__)
+        path = os.path.abspath(
+            os.path.expanduser(os.path.join(this_dir, "sg_logo.jpg"))
+        )
+
+        with self.assertRaises(api.ShotgunError) as cm:
+            self.sg._get_attachment_upload_info(False, path, False)
+
+        # Test the max retries attempt
+        self.assertEqual(
+            self.sg.MAX_ATTEMPTS,
+            mock_opener.return_value.open.call_count,
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times"
+        )
+
+        # Test the exception message
+        the_exception = cm.exception
+        self.assertEqual(str(the_exception), "Max attemps limit reached.")
+
+    def test_upload_s3_urlerror__upload_to_storage(self):
+        """
+        Test URLError response is retried when uploading to S3.
+        """
+        self.sg._make_upload_request = mock.Mock(
+            spec=api.Shotgun._make_upload_request,
+            side_effect=urllib.error.URLError(
+                "[Errno 104] Connection reset by peer",
+            ),
+        )
+
+        this_dir, _ = os.path.split(__file__)
+        storage_url = "http://foo.com/"
+        path = os.path.abspath(
+            os.path.expanduser(os.path.join(this_dir, "sg_logo.jpg"))
+        )
+
+        # Test the Internal function that is used to upload each
+        # data part in the context of multi-part uploads to S3, we
+        # simulate the HTTPError exception raised with 503 status errors
+        with self.assertRaises(api.ShotgunError) as cm:
+            self.sg._upload_file_to_storage(path, storage_url)
+
+        # Test the max retries attempt
+        self.assertEqual(
+            self.sg.MAX_ATTEMPTS,
+            self.sg._make_upload_request.call_count,
+            f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times"
+        )
+
+        # Test the exception message
+        the_exception = cm.exception
+        self.assertEqual(str(the_exception), "Max attemps limit reached.")
 
     def test_transform_data(self):
         """Outbound data is transformed"""