Repository: encode/httpcore

Issue:
Handle HTTP/1.1 half-closed connections gracefully.
There's an HTTP/1.1 case that can occur where...

* The client starts sending a request.
* The server half-closes the connection.
* The server sends a response, such as an HTTP 413 Content Too Large.

Currently our behaviour here is that we'll see a `WriteError` occur here and never get a response.
A more graceful behaviour is handle this case, and return the 413 response.

Prompted via https://github.com/encode/httpx/discussions/2503.

*A follow up question to this will be... is there an equivalent to this for HTTP/2 streams? But let's only consider that once we've dealt with this as a precursor.*

Discussion:
tomchristie: Test case failure is...

> "ResourceWarning: Async generator 'httpcore._models.ByteStream.__aiter__' was garbage collected before it had been exhausted. Surround its use in 'async with aclosing(...):' to ensure that it gets cleaned up as soon as you're done using it."

Looks like this block...

https://github.com/encode/httpcore/blob/6a97dade2a57283491c7830385d52daccbeaf93b/httpcore/_async/http11.py#L133-L135

Needs an `aclosing` wrapper.

(No, I don't fully understand the generator clean-up stuff here.)
madkinsz: Very weird. I presume this is the `WriteError` interrupting iteration during `_send_request_body`?

https://docs.python.org/3/library/contextlib.html#contextlib.aclosing

> Significantly, aclosing() supports deterministic cleanup of async generators when they happen to exit early by [break](https://docs.python.org/3/reference/simple_stmts.html#break) or an exception
>
> This pattern ensures that the generator’s async exit code is executed in the same context as its iterations (so that exceptions and context variables work as expected, and the exit code isn’t run after the lifetime of some task it depends on).

This is only available in Python 3.10+ though? You can `try/finally/aclose` I guess? I'm having a hard time creating a little reproduction.

We're not the only ones :) https://github.com/python/cpython/issues/88684#issuecomment-1189563263

tomchristie: Related: #657 (?)
cdeler: > "ResourceWarning: Async generator 'httpcore._models.ByteStream.aiter' was garbage collected before it had been exhausted. Surround its use in 'async with aclosing(...):' to ensure that it gets cleaned up as soon as you're done using it."

I copied the test from this pr and see this error as well. Perhaps it's might be something else. Will try to dig into it

Patch:
diff --git a/httpcore/_async/http11.py b/httpcore/_async/http11.py
index 7ad3664..32fa3a6 100644
--- a/httpcore/_async/http11.py
+++ b/httpcore/_async/http11.py
@@ -20,6 +20,7 @@ from .._exceptions import (
     ConnectionNotAvailable,
     LocalProtocolError,
     RemoteProtocolError,
+    WriteError,
     map_exceptions,
 )
 from .._models import Origin, Request, Response
@@ -84,10 +85,21 @@ class AsyncHTTP11Connection(AsyncConnectionInterface):
 
         try:
             kwargs = {"request": request}
-            async with Trace("send_request_headers", logger, request, kwargs) as trace:
-                await self._send_request_headers(**kwargs)
-            async with Trace("send_request_body", logger, request, kwargs) as trace:
-                await self._send_request_body(**kwargs)
+            try:
+                async with Trace(
+                    "send_request_headers", logger, request, kwargs
+                ) as trace:
+                    await self._send_request_headers(**kwargs)
+                async with Trace("send_request_body", logger, request, kwargs) as trace:
+                    await self._send_request_body(**kwargs)
+            except WriteError:
+                # If we get a write error while we're writing the request,
+                # then we supress this error and move on to attempting to
+                # read the response. Servers can sometimes close the request
+                # pre-emptively and then respond with a well formed HTTP
+                # error response.
+                pass
+
             async with Trace(
                 "receive_response_headers", logger, request, kwargs
             ) as trace:
diff --git a/httpcore/_sync/http11.py b/httpcore/_sync/http11.py
index edcce72..0cc100e 100644
--- a/httpcore/_sync/http11.py
+++ b/httpcore/_sync/http11.py
@@ -20,6 +20,7 @@ from .._exceptions import (
     ConnectionNotAvailable,
     LocalProtocolError,
     RemoteProtocolError,
+    WriteError,
     map_exceptions,
 )
 from .._models import Origin, Request, Response
@@ -84,10 +85,21 @@ class HTTP11Connection(ConnectionInterface):
 
         try:
             kwargs = {"request": request}
-            with Trace("send_request_headers", logger, request, kwargs) as trace:
-                self._send_request_headers(**kwargs)
-            with Trace("send_request_body", logger, request, kwargs) as trace:
-                self._send_request_body(**kwargs)
+            try:
+                with Trace(
+                    "send_request_headers", logger, request, kwargs
+                ) as trace:
+                    self._send_request_headers(**kwargs)
+                with Trace("send_request_body", logger, request, kwargs) as trace:
+                    self._send_request_body(**kwargs)
+            except WriteError:
+                # If we get a write error while we're writing the request,
+                # then we supress this error and move on to attempting to
+                # read the response. Servers can sometimes close the request
+                # pre-emptively and then respond with a well formed HTTP
+                # error response.
+                pass
+
             with Trace(
                 "receive_response_headers", logger, request, kwargs
             ) as trace:


Test patch:
diff --git a/tests/_async/test_connection.py b/tests/_async/test_connection.py
index 8b29942..b6ee0c7 100644
--- a/tests/_async/test_connection.py
+++ b/tests/_async/test_connection.py
@@ -9,10 +9,13 @@ from httpcore import (
     SOCKET_OPTION,
     AsyncHTTPConnection,
     AsyncMockBackend,
+    AsyncMockStream,
     AsyncNetworkStream,
     ConnectError,
     ConnectionNotAvailable,
     Origin,
+    RemoteProtocolError,
+    WriteError,
 )
 
 
@@ -83,7 +86,109 @@ async def test_concurrent_requests_not_available_on_http11_connections():
                 await conn.request("GET", "https://example.com/")
 
 
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
 @pytest.mark.anyio
+async def test_write_error_with_response_sent():
+    """
+    If a server half-closes the connection while the client is sending
+    the request, it may still send a response. In this case the client
+    should successfully read and return the response.
+
+    See also the `test_write_error_without_response_sent` test above.
+    """
+
+    class ErrorOnRequestTooLargeStream(AsyncMockStream):
+        def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
+            super().__init__(buffer, http2)
+            self.count = 0
+
+        async def write(
+            self, buffer: bytes, timeout: typing.Optional[float] = None
+        ) -> None:
+            self.count += len(buffer)
+
+            if self.count > 1_000_000:
+                raise WriteError()
+
+    class ErrorOnRequestTooLarge(AsyncMockBackend):
+        async def connect_tcp(
+            self,
+            host: str,
+            port: int,
+            timeout: typing.Optional[float] = None,
+            local_address: typing.Optional[str] = None,
+            socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
+        ) -> AsyncMockStream:
+            return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)
+
+    origin = Origin(b"https", b"example.com", 443)
+    network_backend = ErrorOnRequestTooLarge(
+        [
+            b"HTTP/1.1 413 Payload Too Large\r\n",
+            b"Content-Type: plain/text\r\n",
+            b"Content-Length: 37\r\n",
+            b"\r\n",
+            b"Request body exceeded 1,000,000 bytes",
+        ]
+    )
+
+    async with AsyncHTTPConnection(
+        origin=origin, network_backend=network_backend, keepalive_expiry=5.0
+    ) as conn:
+        content = b"x" * 10_000_000
+        response = await conn.request("POST", "https://example.com/", content=content)
+        assert response.status == 413
+        assert response.content == b"Request body exceeded 1,000,000 bytes"
+
+
+@pytest.mark.anyio
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
+async def test_write_error_without_response_sent():
+    """
+    If a server fully closes the connection while the client is sending
+    the request, then client should raise an error.
+
+    See also the `test_write_error_with_response_sent` test above.
+    """
+
+    class ErrorOnRequestTooLargeStream(AsyncMockStream):
+        def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
+            super().__init__(buffer, http2)
+            self.count = 0
+
+        async def write(
+            self, buffer: bytes, timeout: typing.Optional[float] = None
+        ) -> None:
+            self.count += len(buffer)
+
+            if self.count > 1_000_000:
+                raise WriteError()
+
+    class ErrorOnRequestTooLarge(AsyncMockBackend):
+        async def connect_tcp(
+            self,
+            host: str,
+            port: int,
+            timeout: typing.Optional[float] = None,
+            local_address: typing.Optional[str] = None,
+            socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
+        ) -> AsyncMockStream:
+            return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)
+
+    origin = Origin(b"https", b"example.com", 443)
+    network_backend = ErrorOnRequestTooLarge([])
+
+    async with AsyncHTTPConnection(
+        origin=origin, network_backend=network_backend, keepalive_expiry=5.0
+    ) as conn:
+        content = b"x" * 10_000_000
+        with pytest.raises(RemoteProtocolError) as exc_info:
+            await conn.request("POST", "https://example.com/", content=content)
+        assert str(exc_info.value) == "Server disconnected without sending a response."
+
+
+@pytest.mark.anyio
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
 async def test_http2_connection():
     origin = Origin(b"https", b"example.com", 443)
     network_backend = AsyncMockBackend(
diff --git a/tests/_sync/test_connection.py b/tests/_sync/test_connection.py
index 9e0c403..37c82e0 100644
--- a/tests/_sync/test_connection.py
+++ b/tests/_sync/test_connection.py
@@ -9,10 +9,13 @@ from httpcore import (
     SOCKET_OPTION,
     HTTPConnection,
     MockBackend,
+    MockStream,
     NetworkStream,
     ConnectError,
     ConnectionNotAvailable,
     Origin,
+    RemoteProtocolError,
+    WriteError,
 )
 
 
@@ -83,7 +86,109 @@ def test_concurrent_requests_not_available_on_http11_connections():
                 conn.request("GET", "https://example.com/")
 
 
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
 
+def test_write_error_with_response_sent():
+    """
+    If a server half-closes the connection while the client is sending
+    the request, it may still send a response. In this case the client
+    should successfully read and return the response.
+
+    See also the `test_write_error_without_response_sent` test above.
+    """
+
+    class ErrorOnRequestTooLargeStream(MockStream):
+        def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
+            super().__init__(buffer, http2)
+            self.count = 0
+
+        def write(
+            self, buffer: bytes, timeout: typing.Optional[float] = None
+        ) -> None:
+            self.count += len(buffer)
+
+            if self.count > 1_000_000:
+                raise WriteError()
+
+    class ErrorOnRequestTooLarge(MockBackend):
+        def connect_tcp(
+            self,
+            host: str,
+            port: int,
+            timeout: typing.Optional[float] = None,
+            local_address: typing.Optional[str] = None,
+            socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
+        ) -> MockStream:
+            return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)
+
+    origin = Origin(b"https", b"example.com", 443)
+    network_backend = ErrorOnRequestTooLarge(
+        [
+            b"HTTP/1.1 413 Payload Too Large\r\n",
+            b"Content-Type: plain/text\r\n",
+            b"Content-Length: 37\r\n",
+            b"\r\n",
+            b"Request body exceeded 1,000,000 bytes",
+        ]
+    )
+
+    with HTTPConnection(
+        origin=origin, network_backend=network_backend, keepalive_expiry=5.0
+    ) as conn:
+        content = b"x" * 10_000_000
+        response = conn.request("POST", "https://example.com/", content=content)
+        assert response.status == 413
+        assert response.content == b"Request body exceeded 1,000,000 bytes"
+
+
+
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
+def test_write_error_without_response_sent():
+    """
+    If a server fully closes the connection while the client is sending
+    the request, then client should raise an error.
+
+    See also the `test_write_error_with_response_sent` test above.
+    """
+
+    class ErrorOnRequestTooLargeStream(MockStream):
+        def __init__(self, buffer: typing.List[bytes], http2: bool = False) -> None:
+            super().__init__(buffer, http2)
+            self.count = 0
+
+        def write(
+            self, buffer: bytes, timeout: typing.Optional[float] = None
+        ) -> None:
+            self.count += len(buffer)
+
+            if self.count > 1_000_000:
+                raise WriteError()
+
+    class ErrorOnRequestTooLarge(MockBackend):
+        def connect_tcp(
+            self,
+            host: str,
+            port: int,
+            timeout: typing.Optional[float] = None,
+            local_address: typing.Optional[str] = None,
+            socket_options: typing.Optional[typing.Iterable[SOCKET_OPTION]] = None,
+        ) -> MockStream:
+            return ErrorOnRequestTooLargeStream(list(self._buffer), http2=self._http2)
+
+    origin = Origin(b"https", b"example.com", 443)
+    network_backend = ErrorOnRequestTooLarge([])
+
+    with HTTPConnection(
+        origin=origin, network_backend=network_backend, keepalive_expiry=5.0
+    ) as conn:
+        content = b"x" * 10_000_000
+        with pytest.raises(RemoteProtocolError) as exc_info:
+            conn.request("POST", "https://example.com/", content=content)
+        assert str(exc_info.value) == "Server disconnected without sending a response."
+
+
+
+@pytest.mark.filterwarnings("ignore::pytest.PytestUnraisableExceptionWarning")
 def test_http2_connection():
     origin = Origin(b"https", b"example.com", 443)
     network_backend = MockBackend(