fix: Resolve issue where num_retries would have no effect

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):

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"}, "{}"))