fix: address comments

Author: SoulPancake

openfga_sdk/sync/api_client.py

@@ -109,6 +109,7 @@ def __exit__(self, exc_type, exc_value, traceback):
         self.close()
 
     def close(self):
+        self.rest_client.close()
         if self._pool:
             self._pool.close()
             self._pool.join()

openfga_sdk/sync/rest.py

@@ -493,7 +493,13 @@ def request(
             # Log the response body
             logger.debug("response body: %s", wrapped_response.data.decode("utf-8"))
 
-        # Handle any errors that may have occurred
-        self.handle_response_exception(raw_response)
+        # Handle any errors that may have occurred. If an exception is raised,
+        # ensure the underlying response is closed so the connection is not
+        # leaked from the pool.
+        try:
+            self.handle_response_exception(raw_response)
+        except Exception:
+            raw_response.close()
+            raise
 
         return wrapped_response or raw_response

test/sync/rest_test.py

@@ -731,3 +731,132 @@ def test_ssl_context_with_all_ssl_options(mock_pool_manager, mock_create_context
     call_kwargs = mock_pool_manager.call_args[1]
     assert call_kwargs["ssl_context"] == mock_ssl_context
     assert call_kwargs["maxsize"] == 8
+
+
+def test_request_does_not_clear_pool_after_request():
+    """Ensure request() does not call close()/pool_manager.clear() after each request,
+    so pooled connections are preserved for reuse."""
+    mock_config = MagicMock(
+        spec=[
+            "verify_ssl",
+            "ssl_ca_cert",
+            "cert_file",
+            "key_file",
+            "assert_hostname",
+            "retries",
+            "socket_options",
+            "connection_pool_maxsize",
+            "timeout_millisec",
+            "proxy",
+            "proxy_headers",
+        ]
+    )
+    mock_config.ssl_ca_cert = None
+    mock_config.cert_file = None
+    mock_config.key_file = None
+    mock_config.verify_ssl = True
+    mock_config.connection_pool_maxsize = 4
+    mock_config.timeout_millisec = 5000
+    mock_config.proxy = None
+    mock_config.proxy_headers = None
+
+    client = RESTClientObject(configuration=mock_config)
+    mock_pool_manager = MagicMock()
+    client.pool_manager = mock_pool_manager
+
+    mock_raw_response = MagicMock()
+    mock_raw_response.status = 200
+    mock_raw_response.reason = "OK"
+    mock_raw_response.data = b'{"ok":true}'
+
+    mock_pool_manager.request.return_value = mock_raw_response
+
+    # Make multiple requests
+    client.request(method="GET", url="http://example.com", _preload_content=True)
+    client.request(method="GET", url="http://example.com", _preload_content=True)
+
+    # pool_manager.clear() should never have been called
+    mock_pool_manager.clear.assert_not_called()
+
+
+def test_request_closes_response_on_error():
+    """Ensure that if handle_response_exception raises, the raw response is closed
+    so the connection is not leaked from the pool."""
+    mock_config = MagicMock(
+        spec=[
+            "verify_ssl",
+            "ssl_ca_cert",
+            "cert_file",
+            "key_file",
+            "assert_hostname",
+            "retries",
+            "socket_options",
+            "connection_pool_maxsize",
+            "timeout_millisec",
+            "proxy",
+            "proxy_headers",
+        ]
+    )
+    mock_config.ssl_ca_cert = None
+    mock_config.cert_file = None
+    mock_config.key_file = None
+    mock_config.verify_ssl = True
+    mock_config.connection_pool_maxsize = 4
+    mock_config.timeout_millisec = 5000
+    mock_config.proxy = None
+    mock_config.proxy_headers = None
+
+    client = RESTClientObject(configuration=mock_config)
+    mock_pool_manager = MagicMock()
+    client.pool_manager = mock_pool_manager
+
+    mock_raw_response = MagicMock()
+    mock_raw_response.status = 500
+    mock_raw_response.reason = "Internal Server Error"
+    mock_raw_response.data = b'{"error":"something went wrong"}'
+    mock_raw_response.getheaders.return_value = {}
+
+    mock_pool_manager.request.return_value = mock_raw_response
+
+    with pytest.raises(ServiceException):
+        client.request(method="GET", url="http://example.com", _preload_content=True)
+
+    # Verify raw_response.close() was called to release the connection
+    mock_raw_response.close.assert_called_once()
+
+
+def test_api_client_close_calls_rest_client_close():
+    """Ensure the sync ApiClient.close() delegates to rest_client.close()
+    so pooled connections are cleaned up at shutdown."""
+    from openfga_sdk.configuration import Configuration
+    from openfga_sdk.sync.api_client import ApiClient
+
+    configuration = Configuration(
+        api_url="http://api.fga.example",
+    )
+
+    api_client = ApiClient(configuration)
+    mock_rest_client = MagicMock()
+    api_client.rest_client = mock_rest_client
+
+    api_client.close()
+
+    mock_rest_client.close.assert_called_once()
+
+
+def test_api_client_context_manager_calls_close():
+    """Ensure the sync ApiClient context manager calls close() on exit,
+    which in turn cleans up the REST client's connection pool."""
+    from openfga_sdk.configuration import Configuration
+    from openfga_sdk.sync.api_client import ApiClient
+
+    configuration = Configuration(
+        api_url="http://api.fga.example",
+    )
+
+    with ApiClient(configuration) as api_client:
+        mock_rest_client = MagicMock()
+        api_client.rest_client = mock_rest_client
+
+    # After exiting the context manager, close() should have been called
+    mock_rest_client.close.assert_called_once()