Skip to content

response.aclose() raises httpcore errors, not HTTPError #1463

Closed

Description

@adamhooper

Checklist

  • The bug is reproducible against the latest release and/or master.
  • There are no similar issues or pull requests to fix it yet.

Describe the bug

If a server resets the connection, then response.aiter_bytes() correctly raises an httpx.HTTPError.

-BUT- the error handling doesn't end there. If response.aiter_raw() doesn't succeed, it doesn't call response.aclose(). (I imagine it's the caller's responsibility?) So the caller will call response.aclose() ... and that will raise an httpcore exception. It can be caught ... but that's a big hassle and it's a hole in the abstraction layer.

response = await httpx_client.send(request, stream=True, timeout=timeout)  # raise HTTPErrors
try:
    async for blob in response.aiter_raw():
        pass
finally:
    await response.aclose()  # raise httpcore errors
async with httpx_client.stream("GET", url) as response:
    async for blob in response.aiter_raw():
        pass
        # but if aiter_raw() raises an exception, the aclose() will raise an httpcore error

To reproduce

I'm having trouble with the server init, but this works enough of the time....:

import _socket
import asyncio
import io
import struct

import httpx
import pytest


@pytest.mark.asyncio
async def test_download_stream_connection_reset():
    async def respond_then_die(reader, writer):
        # Read request -- as we should
        await reader.readuntil(b"\r\n\r\n")
        # Send HTTP header and start sending data -- so the client is streaming
        writer.write(
            b"HTTP/1.1 200 OK\r\nContent-Length:1000\r\nContent-Type:text/plain\r\n\r\nstart"
        )
        await writer.drain()
        # Now force a TCP RST packet, which will cause response.aclose() to fail
        # http://deepix.github.io/2016/10/21/tcprst.html
        fd = writer._transport._sock.fileno()
        linger = struct.pack("=II", 1, 0)
        _socket.socket(fileno=fd).setsockopt(
            _socket.SOL_SOCKET, _socket.SO_LINGER, bytearray(linger)
        )
        writer.close()

    server = await asyncio.start_server(respond_then_die, "127.0.0.1")
    async with server:
        host, port = server.sockets[0].getsockname()
        async with httpx.AsyncClient() as client:
            async with client.stream("GET", f"http://{host}:{port}") as response:
                async for blob in response.aiter_raw():
                    pass

Expected behavior

httpx.ReadError: [Errno 104] Connection reset by peer

Actual behavior

httpcore.CloseError: [Errno 104] Connection reset by peer

Debugging material

.tox/py38/lib/python3.8/site-packages/httpx/_client.py:1846: in __aexit__
    await self.response.aclose()
.tox/py38/lib/python3.8/site-packages/httpx/_models.py:1299: in aclose
    await self._on_close(self)
.tox/py38/lib/python3.8/site-packages/httpx/_client.py:1513: in on_close
    await stream.aclose()
.tox/py38/lib/python3.8/site-packages/httpcore/_async/connection_pool.py:69: in aclose
    await self.stream.aclose()
.tox/py38/lib/python3.8/site-packages/httpcore/_bytestreams.py:78: in aclose
    await self._aclose_func()
.tox/py38/lib/python3.8/site-packages/httpcore/_async/http11.py:192: in _response_closed
    await self.aclose()
.tox/py38/lib/python3.8/site-packages/httpcore/_async/http11.py:202: in aclose
    await self.socket.aclose()
.tox/py38/lib/python3.8/site-packages/httpcore/_backends/asyncio.py:198: in aclose
    await self.stream_writer.wait_closed()  # type: ignore
/usr/lib64/python3.8/contextlib.py:131: in __exit__
    self.gen.throw(type, value, traceback)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

map = {<class 'OSError'>: <class 'httpcore.CloseError'>}

    @contextlib.contextmanager
    def map_exceptions(map: Dict[Type[Exception], Type[Exception]]) -> Iterator[None]:
        try:
            yield
        except Exception as exc:  # noqa: PIE786
            for from_exc, to_exc in map.items():
                if isinstance(exc, from_exc):
>                   raise to_exc(exc) from None
E                   httpcore.CloseError: [Errno 104] Connection reset by peer

.tox/py38/lib/python3.8/site-packages/httpcore/_exceptions.py:12: CloseError
------------------------------------------------- Captured log call --------------------------------------------------
ERROR    asyncio:base_events.py:1707 Exception in callback _SelectorSocketTransport._call_connection_lost(None)
handle: <Handle _SelectorSocketTransport._call_connection_lost(None)>
Traceback (most recent call last):
  File "/usr/lib64/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/lib64/python3.8/asyncio/selector_events.py", line 970, in _call_connection_lost
    super()._call_connection_lost(exc)
  File "/usr/lib64/python3.8/asyncio/selector_events.py", line 730, in _call_connection_lost
    self._sock.close()
  File "/usr/lib64/python3.8/socket.py", line 500, in close
    self._real_close()
  File "/usr/lib64/python3.8/socket.py", line 494, in _real_close
    _ss.close(self)
OSError: [Errno 9] Bad file descriptor

(That map_exceptions() is the one within httpcore, not the one that I would expect to adorn aclose() in httpx.)

Environment

  • OS: Linux
  • Python version: Python 3.8.7
  • HTTPX version: 0.16.1
  • Async environment: asyncio
  • HTTP proxy: no
  • Custom certificates: no

Additional context

I'm trying to perform HTTP fetches for non-tech-savvy users. I need to handle every exception to present a friendly message.

You might want to split this into two errors, actually?:

  1. response.aclose() exceptions aren't wrapped
  2. StreamContextManager.__aclose__() raises whatever exception response.aclose() raised (should it instead raise the original exception -- the one from aiter_raw()?)

Activity

  1. added a commit that references this issue on Feb 10, 2021
    25a5a68
  2. florimondmanca commented on Feb 10, 2021

    @florimondmanca
    Contributor

    Just popping by to say — if async with stream(...) doesn't close the response if there's a´ exception raised while streaming, then I think that could be considered a bug too. async with usage should take care of clean up in all cases.

  3. added
    bugSomething isn't working
    httpcoreIssues related to HTTPCore (core HTTP networking layer) - https://github.com/encode/httpcore
    user-experienceEnsuring that users have a good experience using the library
    on Feb 10, 2021
  4. adamhooper commented on Feb 10, 2021

    @adamhooper
    ContributorAuthor

    @florimondmanca async with stream() does close the response correctly. The issue I'm highlighting here is only that the wrong exception is raised.

  5. removed
    httpcoreIssues related to HTTPCore (core HTTP networking layer) - https://github.com/encode/httpcore
    on Feb 12, 2021
  6. added this to the v0.17 milestone on Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinguser-experienceEnsuring that users have a good experience using the library

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions