refactor(client): simplify cleanup (#318)

This removes Client.__del__, but users are not expected to call this directly.

pyproject.toml

@@ -81,7 +81,7 @@ typecheck = { chain = [
 ]}
 "typecheck:pyright" = "pyright"
 "typecheck:verify-types" = "pyright --verifytypes modern_treasury --ignoreexternal"
-"typecheck:mypy" = "mypy --enable-incomplete-feature=Unpack ."
+"typecheck:mypy" = "mypy ."
 
 [build-system]
 requires = ["hatchling"]

src/modern_treasury/_base_client.py

@@ -5,6 +5,7 @@
 import time
 import uuid
 import email
+import asyncio
 import inspect
 import logging
 import platform
@@ -672,9 +673,16 @@ def _idempotency_key(self) -> str:
         return f"stainless-python-retry-{uuid.uuid4()}"
 
 
+class SyncHttpxClientWrapper(httpx.Client):
+    def __del__(self) -> None:
+        try:
+            self.close()
+        except Exception:
+            pass
+
+
 class SyncAPIClient(BaseClient[httpx.Client, Stream[Any]]):
     _client: httpx.Client
-    _has_custom_http_client: bool
     _default_stream_cls: type[Stream[Any]] | None = None
 
     def __init__(
@@ -747,15 +755,14 @@ def __init__(
             custom_headers=custom_headers,
             _strict_response_validation=_strict_response_validation,
         )
-        self._client = http_client or httpx.Client(
+        self._client = http_client or SyncHttpxClientWrapper(
             base_url=base_url,
             # cast to a valid type because mypy doesn't understand our type narrowing
             timeout=cast(Timeout, timeout),
             proxies=proxies,
             transport=transport,
             limits=limits,
         )
-        self._has_custom_http_client = bool(http_client)
 
     def is_closed(self) -> bool:
         return self._client.is_closed
@@ -1135,9 +1142,17 @@ def get_api_list(
         return self._request_api_list(model, page, opts)
 
 
+class AsyncHttpxClientWrapper(httpx.AsyncClient):
+    def __del__(self) -> None:
+        try:
+            # TODO(someday): support non asyncio runtimes here
+            asyncio.get_running_loop().create_task(self.aclose())
+        except Exception:
+            pass
+
+
 class AsyncAPIClient(BaseClient[httpx.AsyncClient, AsyncStream[Any]]):
     _client: httpx.AsyncClient
-    _has_custom_http_client: bool
     _default_stream_cls: type[AsyncStream[Any]] | None = None
 
     def __init__(
@@ -1210,15 +1225,14 @@ def __init__(
             custom_headers=custom_headers,
             _strict_response_validation=_strict_response_validation,
         )
-        self._client = http_client or httpx.AsyncClient(
+        self._client = http_client or AsyncHttpxClientWrapper(
             base_url=base_url,
             # cast to a valid type because mypy doesn't understand our type narrowing
             timeout=cast(Timeout, timeout),
             proxies=proxies,
             transport=transport,
             limits=limits,
         )
-        self._has_custom_http_client = bool(http_client)
 
     def is_closed(self) -> bool:
         return self._client.is_closed

src/modern_treasury/_client.py

@@ -4,7 +4,6 @@
 
 import os
 import base64
-import asyncio
 from typing import Any, Union, Mapping
 from typing_extensions import Self, override
 
@@ -37,6 +36,8 @@
     DEFAULT_MAX_RETRIES,
     SyncAPIClient,
     AsyncAPIClient,
+    SyncHttpxClientWrapper,
+    AsyncHttpxClientWrapper,
     make_request_options,
 )
 
@@ -273,7 +274,7 @@ def copy(
             if http_client is not None:
                 raise ValueError("The 'http_client' argument is mutually exclusive with 'connection_pool_limits'")
 
-            if self._has_custom_http_client:
+            if not isinstance(self._client, SyncHttpxClientWrapper):
                 raise ValueError(
                     "A custom HTTP client has been set and is mutually exclusive with the 'connection_pool_limits' argument"
                 )
@@ -305,16 +306,6 @@ def copy(
     # client.with_options(timeout=10).foo.create(...)
     with_options = copy
 
-    def __del__(self) -> None:
-        if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
-            # this can happen if the '__init__' method raised an error
-            return
-
-        if self._has_custom_http_client:
-            return
-
-        self.close()
-
     def ping(
         self,
         *,
@@ -591,7 +582,7 @@ def copy(
             if http_client is not None:
                 raise ValueError("The 'http_client' argument is mutually exclusive with 'connection_pool_limits'")
 
-            if self._has_custom_http_client:
+            if not isinstance(self._client, AsyncHttpxClientWrapper):
                 raise ValueError(
                     "A custom HTTP client has been set and is mutually exclusive with the 'connection_pool_limits' argument"
                 )
@@ -623,19 +614,6 @@ def copy(
     # client.with_options(timeout=10).foo.create(...)
     with_options = copy
 
-    def __del__(self) -> None:
-        if not hasattr(self, "_has_custom_http_client") or not hasattr(self, "close"):
-            # this can happen if the '__init__' method raised an error
-            return
-
-        if self._has_custom_http_client:
-            return
-
-        try:
-            asyncio.get_running_loop().create_task(self.close())
-        except Exception:
-            pass
-
     async def ping(
         self,
         *,

tests/test_client.py

@@ -801,16 +801,6 @@ def test_proxies_option_mutually_exclusive_with_http_client(self) -> None:
                         http_client=http_client,
                     )
 
-    def test_client_del(self) -> None:
-        client = ModernTreasury(
-            base_url=base_url, api_key=api_key, organization_id=organization_id, _strict_response_validation=True
-        )
-        assert not client.is_closed()
-
-        client.__del__()
-
-        assert client.is_closed()
-
     def test_copied_client_does_not_close_http(self) -> None:
         client = ModernTreasury(
             base_url=base_url, api_key=api_key, organization_id=organization_id, _strict_response_validation=True
@@ -820,9 +810,8 @@ def test_copied_client_does_not_close_http(self) -> None:
         copied = client.copy()
         assert copied is not client
 
-        copied.__del__()
+        del copied
 
-        assert not copied.is_closed()
         assert not client.is_closed()
 
     def test_client_context_manager(self) -> None:
@@ -1712,17 +1701,6 @@ async def test_proxies_option_mutually_exclusive_with_http_client(self) -> None:
                         http_client=http_client,
                     )
 
-    async def test_client_del(self) -> None:
-        client = AsyncModernTreasury(
-            base_url=base_url, api_key=api_key, organization_id=organization_id, _strict_response_validation=True
-        )
-        assert not client.is_closed()
-
-        client.__del__()
-
-        await asyncio.sleep(0.2)
-        assert client.is_closed()
-
     async def test_copied_client_does_not_close_http(self) -> None:
         client = AsyncModernTreasury(
             base_url=base_url, api_key=api_key, organization_id=organization_id, _strict_response_validation=True
@@ -1732,10 +1710,9 @@ async def test_copied_client_does_not_close_http(self) -> None:
         copied = client.copy()
         assert copied is not client
 
-        copied.__del__()
+        del copied
 
         await asyncio.sleep(0.2)
-        assert not copied.is_closed()
         assert not client.is_closed()
 
     async def test_client_context_manager(self) -> None: