diff --git a/googleapiclient/http.py b/googleapiclient/http.py index e78f70f1ab9..5d4227fb888 100644 --- a/googleapiclient/http.py +++ b/googleapiclient/http.py @@ -94,6 +94,8 @@ def _should_retry_response(resp_status, content): Returns: True if the response should be retried, otherwise False. """ + reason = None + # Retry on 5xx errors. if resp_status >= 500: return True @@ -113,9 +115,22 @@ def _should_retry_response(resp_status, content): try: data = json.loads(content.decode("utf-8")) if isinstance(data, dict): - reason = data["error"].get("status") - if reason is None: - reason = data["error"]["errors"][0]["reason"] + # There are many variations of the error json so we need + # to determine the keyword which has the error detail. Make sure + # that the order of the keywords below isn't changed as it can + # break user code. If the "errors" key exists, we must use that + # first. + # See Issue #1243 + # https://github.com/googleapis/google-api-python-client/issues/1243 + error_detail_keyword = next((kw for kw in ["errors", "status", "message"] if kw in data["error"]), "") + + if error_detail_keyword: + reason = data["error"][error_detail_keyword] + + if isinstance(reason, list) and len(reason) > 0: + reason = reason[0] + if "reason" in reason: + reason = reason["reason"] else: reason = data[0]["error"]["errors"]["reason"] except (UnicodeDecodeError, ValueError, KeyError): diff --git a/tests/test_http.py b/tests/test_http.py index 700a48512c6..4e613589499 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -401,7 +401,7 @@ def test_media_io_base_next_chunk_retries(self): ({"status": "500"}, ""), ({"status": "503"}, ""), ({"status": "200", "location": "location"}, ""), - ({"status": "403"}, USER_RATE_LIMIT_EXCEEDED_RESPONSE), + ({"status": "403"}, USER_RATE_LIMIT_EXCEEDED_RESPONSE_NO_STATUS), ({"status": "403"}, RATE_LIMIT_EXCEEDED_RESPONSE), ({"status": "429"}, ""), ({"status": "200"}, "{}"), @@ -834,7 +834,7 @@ def test_media_io_base_download_unknown_media_size(self): --batch_foobarbaz--""" -USER_RATE_LIMIT_EXCEEDED_RESPONSE = """{ +USER_RATE_LIMIT_EXCEEDED_RESPONSE_NO_STATUS = """{ "error": { "errors": [ { @@ -848,6 +848,20 @@ def test_media_io_base_download_unknown_media_size(self): } }""" +USER_RATE_LIMIT_EXCEEDED_RESPONSE_WITH_STATUS = """{ + "error": { + "errors": [ + { + "domain": "usageLimits", + "reason": "userRateLimitExceeded", + "message": "User Rate Limit Exceeded" + } + ], + "code": 403, + "message": "User Rate Limit Exceeded", + "status": "PERMISSION_DENIED" + } +}""" RATE_LIMIT_EXCEEDED_RESPONSE = """{ "error": { @@ -981,10 +995,11 @@ def test_retry_connection_errors_resumable(self): self.assertEqual({u"foo": u"bar"}, response) def test_retry(self): - num_retries = 5 - resp_seq = [({"status": "500"}, "")] * (num_retries - 3) + num_retries = 6 + resp_seq = [({"status": "500"}, "")] * (num_retries - 4) resp_seq.append(({"status": "403"}, RATE_LIMIT_EXCEEDED_RESPONSE)) - resp_seq.append(({"status": "403"}, USER_RATE_LIMIT_EXCEEDED_RESPONSE)) + resp_seq.append(({"status": "403"}, USER_RATE_LIMIT_EXCEEDED_RESPONSE_NO_STATUS)) + resp_seq.append(({"status": "403"}, USER_RATE_LIMIT_EXCEEDED_RESPONSE_WITH_STATUS)) resp_seq.append(({"status": "429"}, "")) resp_seq.append(({"status": "200"}, "{}"))