diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 3adacb92f..84322f028 100644 --- a/shotgun_api3/shotgun.py +++ b/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): diff --git a/tests/base.py b/tests/base.py index 01b39bd15..28547cbeb 100644 --- a/tests/base.py +++ b/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. diff --git a/tests/test_client.py b/tests/test_client.py index 9a6b0f9b8..6275e4d28 100644 --- a/tests/test_client.py +++ b/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"""