fix: restore and correct test_discovery_http_is_closed (issue #2757)

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,10 +137,19 @@ 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"):
+        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"',
+            },
+        }
+        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.

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)

tests/test_http.py

@@ -880,6 +880,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": {
@@ -1058,6 +1072,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()