Repository: encode/httpx (python)

Issue:
Stricter enforcement around client scoping.
Raised via: https://github.com/encode/httpx/issues/1478#issuecomment-828955977

There are really two sensible ways to use a client instance wrt. scoping the lifetime...

Implicitly opened clients...

```python
client = httpx.Client()
...
client.close()  # Optional in the sync case. Issues a warning if omitted in the async case.
```

Explicitly opened clients...

```python
with httpx.Client() as client:
    ...
```

However we currently don't guard against unexpected usages, which would likely indicate a user error, and could lead to unexpected behaviours.

* Using a client within a `with` scope multiple times in parallel.
* Using a client making plain implicitly opening requests, and *then* opening it within a `with` scope.
* Using a client within a `with` scope, but calling an explicit `close()`.
* Using a client within a `with` scope multiple times sequentially. We could *potentially* allow this? Possibly a footgun? Not sure?

We could guard against these cases and more strictly enforce correct client scoping.

For instance, the user-code error that caused https://github.com/encode/httpx/issues/1478#issuecomment-828955977 would be resolved if we added some `RuntimeError` cases around weird client state transitions on `__enter__`/`__exit__`/`__aenter__`/`__aexit__`.

So:

* On `__enter__`/`__aenter__` raise a `RuntimeError` if the client was not in an `UNOPENED` state.
* On `__exit__`/`__aexit__` raise a `RuntimeError` if the client was not in a `CLOSED` state.


Pull request:
Stricter enforcement of client lifespan styles.
When using a client, the expectation is that users should use one of the two following styles...

```python
with httpx.Client() as client:
    ...
```

Or...

```python
client = httpx.Client()
...
```

This pull request introduces a little bit of guarding around the opening of a `client` with block in order to enforce that unexpected usages mixing the two styles don't occur, so that we can't...

* Reopen a client once it's been closed.
* Enter into a client `with` block more than once.

Nailing down the possible usages here should just help guard against odd unexpected states.

Closes #1615

Patch:
diff --git a/httpx/_client.py b/httpx/_client.py
index 9fd96c4..212222a 100644
--- a/httpx/_client.py
+++ b/httpx/_client.py
@@ -1232,6 +1232,13 @@ class Client(BaseClient):
                     transport.close()
 
     def __enter__(self: T) -> T:
+        if self._state != ClientState.UNOPENED:
+            msg = {
+                ClientState.OPENED: "Cannot open a client instance more than once.",
+                ClientState.CLOSED: "Cannot reopen a client instance, once it has been closed.",
+            }[self._state]
+            raise RuntimeError(msg)
+
         self._state = ClientState.OPENED
 
         self._transport.__enter__()
@@ -1941,6 +1948,13 @@ class AsyncClient(BaseClient):
                     await proxy.aclose()
 
     async def __aenter__(self: U) -> U:
+        if self._state != ClientState.UNOPENED:
+            msg = {
+                ClientState.OPENED: "Cannot open a client instance more than once.",
+                ClientState.CLOSED: "Cannot reopen a client instance, once it has been closed.",
+            }[self._state]
+            raise RuntimeError(msg)
+
         self._state = ClientState.OPENED
 
         await self._transport.__aenter__()


Test patch:
diff --git a/tests/client/test_async_client.py b/tests/client/test_async_client.py
index 0f83edd..1761287 100644
--- a/tests/client/test_async_client.py
+++ b/tests/client/test_async_client.py
@@ -265,9 +265,15 @@ async def test_client_closed_state_using_implicit_open():
     await client.aclose()
 
     assert client.is_closed
+    # Once we're close we cannot make any more requests.
     with pytest.raises(RuntimeError):
         await client.get("http://example.com")
 
+    # Once we're closed we cannot reopen the client.
+    with pytest.raises(RuntimeError):
+        async with client:
+            pass  # pragma: nocover
+
 
 @pytest.mark.usefixtures("async_environment")
 async def test_client_closed_state_using_with_block():
diff --git a/tests/client/test_client.py b/tests/client/test_client.py
index 01d0de8..783d6d4 100644
--- a/tests/client/test_client.py
+++ b/tests/client/test_client.py
@@ -305,9 +305,16 @@ def test_client_closed_state_using_implicit_open():
     client.close()
 
     assert client.is_closed
+
+    # Once we're close we cannot make any more requests.
     with pytest.raises(RuntimeError):
         client.get("http://example.com")
 
+    # Once we're closed we cannot reopen the client.
+    with pytest.raises(RuntimeError):
+        with client:
+            pass  # pragma: nocover
+
 
 def test_client_closed_state_using_with_block():
     with httpx.Client(transport=httpx.MockTransport(hello_world)) as client: