Skip to content

Commit 50a5f32

Browse files
committed
Fix #2723: Retry on HTTP 400 failedPrecondition
1 parent 0026615 commit 50a5f32

File tree

2 files changed

+55
-8
lines changed

2 files changed

+55
-8
lines changed

googleapiclient/http.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,10 @@ def _should_retry_response(resp_status, content):
9797
if resp_status == _TOO_MANY_REQUESTS:
9898
return True
9999

100-
# For 403 errors, we have to check for the `reason` in the response to
100+
# For 403 and 400 errors, we have to check for the `reason` in the response to
101101
# determine if we should retry.
102-
if resp_status == http_client.FORBIDDEN:
103-
# If there's no details about the 403 type, don't retry.
102+
if resp_status in [http_client.FORBIDDEN, http_client.BAD_REQUEST]:
103+
# If there's no details about the error, don't retry.
104104
if not content:
105105
return False
106106

@@ -137,11 +137,16 @@ def _should_retry_response(resp_status, content):
137137
LOGGER.warning("Invalid JSON content from response: %s", content)
138138
return False
139139

140-
LOGGER.warning('Encountered 403 Forbidden with reason "%s"', reason)
141-
142-
# Only retry on rate limit related failures.
143-
if reason in ("userRateLimitExceeded", "rateLimitExceeded"):
144-
return True
140+
if resp_status == http_client.FORBIDDEN:
141+
LOGGER.warning('Encountered 403 Forbidden with reason "%s"', reason)
142+
# Only retry on rate limit related failures.
143+
if reason in ("userRateLimitExceeded", "rateLimitExceeded"):
144+
return True
145+
elif resp_status == http_client.BAD_REQUEST:
146+
LOGGER.warning('Encountered 400 Bad Request with reason "%s"', reason)
147+
# Only retry on precondition failures.
148+
if reason in ("failedPrecondition", "preconditionFailed"):
149+
return True
145150

146151
# Everything else is a success or non-retriable so break.
147152
return False

tests/test_http.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,20 @@ def test_media_io_base_download_unknown_media_size(self):
878878
}
879879
}"""
880880

881+
FAILED_PRECONDITION_RESPONSE = """{
882+
"error": {
883+
"errors": [
884+
{
885+
"domain": "global",
886+
"reason": "failedPrecondition",
887+
"message": "Precondition Failed"
888+
}
889+
],
890+
"code": 400,
891+
"message": "Precondition Failed"
892+
}
893+
}"""
894+
881895

882896
NOT_CONFIGURED_RESPONSE = """{
883897
"error": {
@@ -1056,6 +1070,34 @@ def test_no_retry_succeeds(self):
10561070

10571071
self.assertEqual(0, len(sleeptimes))
10581072

1073+
def test_retry_400_failed_precondition(self):
1074+
num_retries = 2
1075+
resp_seq = [
1076+
({"status": "400"}, FAILED_PRECONDITION_RESPONSE),
1077+
({"status": "200"}, "{}")
1078+
]
1079+
http = HttpMockSequence(resp_seq)
1080+
model = JsonModel()
1081+
uri = "https://www.googleapis.com/someapi/v1/collection/?foo=bar"
1082+
method = "POST"
1083+
request = HttpRequest(
1084+
http,
1085+
model.response,
1086+
uri,
1087+
method=method,
1088+
body="{}",
1089+
headers={"content-type": "application/json"},
1090+
)
1091+
1092+
sleeptimes = []
1093+
request._sleep = lambda x: sleeptimes.append(x)
1094+
request._rand = lambda: 10
1095+
1096+
request.execute(num_retries=num_retries)
1097+
1098+
self.assertEqual(1, len(sleeptimes))
1099+
self.assertEqual(10 * 2 ** 1, sleeptimes[0])
1100+
10591101
def test_no_retry_fails_fast(self):
10601102
http = HttpMockSequence([({"status": "500"}, ""), ({"status": "200"}, "{}")])
10611103
model = JsonModel()

0 commit comments

Comments
 (0)