From 806eab98001eb5fb1819f42a995555483f7ef140 Mon Sep 17 00:00:00 2001 From: codeXsidd Date: Sat, 21 Mar 2026 10:09:43 +0530 Subject: [PATCH 1/4] fix(http): retry on HTTP 400 failedPrecondition Resolves #2723 --- googleapiclient/http.py | 21 +++++++++++++-------- tests/test_http.py | 42 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 8 deletions(-) diff --git a/googleapiclient/http.py b/googleapiclient/http.py index 187f6f5dac8..a55f77c7cc0 100644 --- a/googleapiclient/http.py +++ b/googleapiclient/http.py @@ -97,10 +97,10 @@ def _should_retry_response(resp_status, content): if resp_status == _TOO_MANY_REQUESTS: return True - # For 403 errors, we have to check for the `reason` in the response to + # For 403 and 400 errors, we have to check for the `reason` in the response to # determine if we should retry. - if resp_status == http_client.FORBIDDEN: - # If there's no details about the 403 type, don't retry. + if resp_status in [http_client.FORBIDDEN, http_client.BAD_REQUEST]: + # If there's no details about the error, don't retry. if not content: return False @@ -137,11 +137,16 @@ def _should_retry_response(resp_status, content): LOGGER.warning("Invalid JSON content from response: %s", content) return False - LOGGER.warning('Encountered 403 Forbidden with reason "%s"', reason) - - # Only retry on rate limit related failures. - if reason in ("userRateLimitExceeded", "rateLimitExceeded"): - return True + if resp_status == http_client.FORBIDDEN: + LOGGER.warning('Encountered 403 Forbidden with reason "%s"', reason) + # Only retry on rate limit related failures. + if reason in ("userRateLimitExceeded", "rateLimitExceeded"): + return True + elif resp_status == http_client.BAD_REQUEST: + LOGGER.warning('Encountered 400 Bad Request with reason "%s"', reason) + # Only retry on precondition failures. + if reason in ("failedPrecondition", "preconditionFailed"): + return True # Everything else is a success or non-retriable so break. return False diff --git a/tests/test_http.py b/tests/test_http.py index 42110adfab1..4e3fff14333 100644 --- a/tests/test_http.py +++ b/tests/test_http.py @@ -878,6 +878,20 @@ def test_media_io_base_download_unknown_media_size(self): } }""" +FAILED_PRECONDITION_RESPONSE = """{ + "error": { + "errors": [ + { + "domain": "global", + "reason": "failedPrecondition", + "message": "Precondition Failed" + } + ], + "code": 400, + "message": "Precondition Failed" + } +}""" + NOT_CONFIGURED_RESPONSE = """{ "error": { @@ -1056,6 +1070,34 @@ def test_no_retry_succeeds(self): self.assertEqual(0, len(sleeptimes)) + def test_retry_400_failed_precondition(self): + num_retries = 2 + resp_seq = [ + ({"status": "400"}, FAILED_PRECONDITION_RESPONSE), + ({"status": "200"}, "{}") + ] + http = HttpMockSequence(resp_seq) + model = JsonModel() + uri = "https://www.googleapis.com/someapi/v1/collection/?foo=bar" + method = "POST" + request = HttpRequest( + http, + model.response, + uri, + method=method, + body="{}", + headers={"content-type": "application/json"}, + ) + + sleeptimes = [] + request._sleep = lambda x: sleeptimes.append(x) + request._rand = lambda: 10 + + request.execute(num_retries=num_retries) + + self.assertEqual(1, len(sleeptimes)) + self.assertEqual(10 * 2 ** 1, sleeptimes[0]) + def test_no_retry_fails_fast(self): http = HttpMockSequence([({"status": "500"}, ""), ({"status": "200"}, "{}")]) model = JsonModel() From f3603a7fa59f3507c0f1bee56397fcfb813aba3a Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 15 Jun 2026 15:06:10 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- googleapiclient/http.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/googleapiclient/http.py b/googleapiclient/http.py index a55f77c7cc0..d69c212f5d0 100644 --- a/googleapiclient/http.py +++ b/googleapiclient/http.py @@ -137,15 +137,20 @@ def _should_retry_response(resp_status, content): LOGGER.warning("Invalid JSON content from response: %s", content) return False - if resp_status == http_client.FORBIDDEN: - LOGGER.warning('Encountered 403 Forbidden with reason "%s"', reason) - # Only retry on rate limit related failures. - if reason in ("userRateLimitExceeded", "rateLimitExceeded"): - return True - elif resp_status == http_client.BAD_REQUEST: - LOGGER.warning('Encountered 400 Bad Request with reason "%s"', reason) - # Only retry on precondition failures. - if reason in ("failedPrecondition", "preconditionFailed"): + RETRYABLE_INFO = { + http_client.FORBIDDEN: { + "reasons": ("userRateLimitExceeded", "rateLimitExceeded"), + "message": 'Encountered 403 Forbidden with reason "%s"', + }, + http_client.BAD_REQUEST: { + "reasons": ("failedPrecondition", "preconditionFailed"), + "message": 'Encountered 400 Bad Request with reason "%s"', + }, + } + if resp_status in RETRYABLE_INFO: + info = RETRYABLE_INFO[resp_status] + LOGGER.warning(info["message"], reason) + if reason in info["reasons"]: return True # Everything else is a success or non-retriable so break. From cc60f41640b439a58ffc1f29e86106ebfb86ec44 Mon Sep 17 00:00:00 2001 From: ohmayr Date: Mon, 15 Jun 2026 15:12:34 -0700 Subject: [PATCH 3/4] Update http.py --- googleapiclient/http.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/googleapiclient/http.py b/googleapiclient/http.py index d69c212f5d0..35075a1122c 100644 --- a/googleapiclient/http.py +++ b/googleapiclient/http.py @@ -147,11 +147,10 @@ def _should_retry_response(resp_status, content): "message": 'Encountered 400 Bad Request with reason "%s"', }, } - if resp_status in RETRYABLE_INFO: - info = RETRYABLE_INFO[resp_status] - LOGGER.warning(info["message"], reason) - if reason in info["reasons"]: - return True + info = RETRYABLE_INFO[resp_status] + LOGGER.warning(info["message"], reason) + if reason in info["reasons"]: + return True # Everything else is a success or non-retriable so break. return False From d4aed74123cea07df967a23f27874a33a1811ca6 Mon Sep 17 00:00:00 2001 From: codeXsidd Date: Thu, 22 Oct 2026 15:37:15 +0530 Subject: [PATCH 4/4] fix: restore and correct test_discovery_http_is_closed (#2757) The test was accidentally dead code since Sept 2020 (commit 98888da) because it was defined in a duplicate class Discovery(unittest.TestCase) block at line 498, which Python silently overwrote with the active class definition at the bottom of the file. The test was therefore never collected or executed by pytest. Additionally, the original test body was syntactically broken: it called http.close.assert_called_once() on a plain HttpMock whose close attribute is a regular function, not a mock object, so the method call would raise AttributeError if ever run. Changes: - Remove the duplicate/shadowed class Discovery block that contained only the broken test skeleton. - Add a corrected est_discovery_http_is_closed inside the active Discovery class (the one that is actually collected by pytest). - Use @mock.patch('httplib2.Http') to intercept the internally-created HTTP client that uild() creates when no http= argument is passed. - Use @mock.patch('google.auth.default') to avoid requiring real Application Default Credentials in CI. - Assert mock_http.close.assert_called_once() to verify that uild() closes the temporary discovery HTTP client after use, preventing socket/connection leaks. --- tests/test_discovery.py | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/tests/test_discovery.py b/tests/test_discovery.py index 6912783451f..9c08c43908d 100644 --- a/tests/test_discovery.py +++ b/tests/test_discovery.py @@ -495,12 +495,6 @@ def test_ResourceMethodParameters_zoo_animals_patch(self): self.assertEqual(parameters.enum_params, {}) -class Discovery(unittest.TestCase): - def test_discovery_http_is_closed(self): - http = HttpMock(datafile("malformed.json"), {"status": "200"}) - service = build("plus", "v1", credentials=mock.sentinel.credentials) - http.close.assert_called_once() - class DiscoveryErrors(unittest.TestCase): def test_tests_should_be_run_with_strict_positional_enforcement(self): @@ -1549,6 +1543,32 @@ def test_file_based_cache(self): class Discovery(unittest.TestCase): + @mock.patch("google.auth.default") + @mock.patch("httplib2.Http") + def test_discovery_http_is_closed(self, mock_http_class, mock_auth_default): + """Verify that the temporary HTTP client created by build() to fetch + the discovery document is closed after initialization, preventing + connection/socket leaks (googleapis/google-api-python-client#2757). + + When no http= is passed, build() internally creates a httplib2.Http() + instance to fetch the discovery document and must close it afterwards + to avoid connection/socket leaks. + """ + mock_http = mock_http_class.return_value + mock_http.request.return_value = ( + httplib2.Response({"status": "200"}), + read_datafile("plus.json", "rb"), + ) + mock_credentials = mock.create_autospec( + google.auth.credentials.Credentials, instance=True + ) + mock_credentials.universe_domain = "googleapis.com" + mock_auth_default.return_value = (mock_credentials, "test-project") + + build("plus", "v1", static_discovery=False) + + mock_http.close.assert_called_once() + def test_method_error_checking(self): self.http = HttpMock(datafile("plus.json"), {"status": "200"}) plus = build("plus", "v1", http=self.http, static_discovery=False)