Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

anyio.EndOfStream can leak up #808

Closed
mcclurem opened this issue Sep 18, 2023 · 8 comments · Fixed by #899
Closed

anyio.EndOfStream can leak up #808

mcclurem opened this issue Sep 18, 2023 · 8 comments · Fixed by #899
Labels
good first issue Good for newcomers

Comments

@mcclurem
Copy link

Originally raised on httpx, referred here

Discussed in encode/httpx#2848

When using httpx as follows:

async with httpx.AsyncClient(verify=client.verify_ssl) as _client:
        response = await _client.request(**kwargs)

with a misbehaving server I will occasionally get an anyio.EndOfStream exception thrown from deep in the call stack.
I would expect that this should have been caught by httpx and converted into one of the applicable httpx exceptions (probably one of the StreamErrors)

call stack that I see this in:

File "/lib/python3.10/site-packages/httpx/_client.py", line 1530, in request
    return await self.send(request, auth-auth, follow_redirects-follow_redirects)
File "/lib/python3.10/site-packages/httpx/_client.py", line 1617, in send
    response = await self._send_handling_auth
File "/lib/python3.10/site-packages/httpx/_client.py", line 1645, in _send_handling_auth
    response = await self._send_handling_redirects
File "/Users/rack-user/minion_venv/1ib/python3.10/site-packages/httpx/_client.py", line 1682, in _send_handling_redirects
    response = await self._send_single_request (request)
File "/lib/python3.10/site-packages/httpx/_client.py", line 1719, in _send_single_request
    response = await transport. handle_async_request(request)
File "/Users/rack-user/minion_venv/lib/python3.10/site-packages/httpx/_transports/default.py", line 353, in handle_async_request
    resp = await self._pool.handle_async_request(reg)
File "/lib/python3.10/site-packages/httpcore/_async/connection_pool.py", line 262, in handle_async_request
    raise exc
File "/lib/python3.10/site-packages/httpcore/_async/connection_pool.py", line 245, in handle_async_request
    response = await connection.handle_async_request(request)
File "/lib/python3.10/site-packages/httpcore/_async/connection.py", line 92, in handle_async_request
    raise exc
File "/lib/python3.10/site-packages/httpcore/_async/connection.py", line 69, in handle_async_request
    stream = await self._connect(request)
File "/lib/python3.10/site-packages/httpcore/_async/connection.py", line 149, in _connect
    stream = await stream.start_tls(**kwargs)
File "/Users/rack_user/minion-venv/lib/python3.10/site-packages/httpcore/_backends/anyio.py", line 78, in start_tls
    raise exc
File "/lib/python3.10/site-packages/httpcore/_backends/anyio.py", line 69, in start_tls
ssl_stream = await anyio.streams.tls.TLSStream.wrap(
File "/lib/python3.10/site-packages/anyio/streams/tls.py", line 125, in wrap
    await wrapper._call_sslobject_method(ss1_object.do_handshake)
File "/lib/python3.10/site-packages/anyio/streams/tls.py", line 165, in _call_sslobject_method
    raise EndOfStream from None
@karpetrosyan
Copy link
Member

It seems like we should just add the new exception mapping to the anyio backend module.

EndOfStream class definition:

class EndOfStream(Exception):
    """Raised when trying to read from a stream that has been closed from the other end."""

I believe it would be sufficient to add EndOfStream mapping with ReadError here.

exc_map = {
TimeoutError: ConnectTimeout,
anyio.BrokenResourceError: ConnectError,
}

and here

exc_map = {
TimeoutError: ReadTimeout,
anyio.BrokenResourceError: ReadError,
anyio.ClosedResourceError: ReadError,
}

@karpetrosyan karpetrosyan added the good first issue Good for newcomers label Sep 19, 2023
@tomchristie
Copy link
Member

For bonus points...

  • Which exceptions does anyio document may be raised in each circumstance? If a pull request here can reference the anyio docs then we'll have good interlinking, and be confident that we've correctly covered the various possible exception cases. I dug into this from a quick look, but haven't comprehensively listed out which exceptions could be raised from each of our stream operations.
  • Can you demo the issue in the most isolated case possible, by working with just the network backend API? Being able to show a case where the exception is raised just using .connect_tcp() and .start_tls() would show that we've properly isolated the issue. (I know we have, but it'd be a neat wrapping up.) Do our docs properly indicate which exceptions may be raised by the network backend operations?

@PrathamSoni
Copy link

Hi! Were there any updates to this?

@tomchristie
Copy link
Member

This is still up for grabs, pull requests welcome. 🤗

@karpetrosyan's comment above points to where we need a couple of entries adding in our exception mapping.

@jakekaplan
Copy link

jakekaplan commented Mar 19, 2024

Really glad to see this issue is closed 🙌!

Any idea when the next release will be that includes this?

@seano314
Copy link

Really glad to see this issue is closed 🙌!

Any idea when the next release will be that includes this?

Agree completely and would love to know the answer to this question too :)

@tomchristie
Copy link
Member

Welcome to issue a release PR.

For example, see... #892

@okba-boularaoui
Copy link
Contributor

Thank you @tomchristie, Version 1.0.5 (#904) PR is ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants