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

Retry ConnectError/ConnectTimeout happened in stream.start_tls #669

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

Pliner
Copy link
Contributor

@Pliner Pliner commented Apr 14, 2023

Hi,

I am experiencing a small rare issue which has the following trace:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/anyio/streams/tls.py", line 130, in _call_sslobject_method
    result = func(*args)
             ^^^^^^^^^^^
  File "/usr/local/lib/python3.11/ssl.py", line 979, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLSyscallError: Some I/O error occurred (_ssl.c:1002)

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/httpcore/_exceptions.py", line 10, in map_exceptions
    yield
  File "/usr/local/lib/python3.11/site-packages/httpcore/backends/asyncio.py", line 78, in start_tls
    raise exc
  File "/usr/local/lib/python3.11/site-packages/httpcore/backends/asyncio.py", line 69, in start_tls
    ssl_stream = await anyio.streams.tls.TLSStream.wrap(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/anyio/streams/tls.py", line 122, in wrap
    await wrapper._call_sslobject_method(ssl_object.do_handshake)
  File "/usr/local/lib/python3.11/site-packages/anyio/streams/tls.py", line 151, in _call_sslobject_method
    raise BrokenResourceError from exc
anyio.BrokenResourceError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/httpx/_transports/default.py", line 60, in map_httpcore_exceptions
    yield
  File "/usr/local/lib/python3.11/site-packages/httpx/_transports/default.py", line 353, in handle_async_request
    resp = await self._pool.handle_async_request(req)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/httpcore/_async/connection_pool.py", line 253, in handle_async_request
    raise exc
  File "/usr/local/lib/python3.11/site-packages/httpcore/_async/connection_pool.py", line 237, in handle_async_request
    response = await connection.handle_async_request(request)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/httpcore/_async/connection.py", line 86, in handle_async_request
    raise exc
  File "/usr/local/lib/python3.11/site-packages/httpcore/_async/connection.py", line 63, in handle_async_request
    stream = await self._connect(request)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/httpcore/_async/connection.py", line 150, in _connect
    stream = await stream.start_tls(**kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/httpcore/backends/asyncio.py", line 66, in start_tls
    with map_exceptions(exc_map):
  File "/usr/local/lib/python3.11/contextlib.py", line 155, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/usr/local/lib/python3.11/site-packages/httpcore/_exceptions.py", line 14, in map_exceptions
    raise to_exc(exc)
httpcore.ConnectError

I have retries enabled on AsyncTransport, but ConnectError didn't attempt to retry at all if it was raised in stream.start_tls method.

It looks like that it is safe to retry such an error as well.

@Pliner Pliner changed the title Retry start_tls errors Retry ConnectError/ConnectTimeout happened in stream.start_tls Apr 14, 2023
@Pliner
Copy link
Contributor Author

Pliner commented Apr 17, 2023

Hi @tomchristie,

Sorry for bothering you, could you please review this PR?

Thanks!

Copy link
Member

@tomchristie tomchristie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moving the start_tls inside the exception handling block looks correct to me, yup. Thanks.

@Pliner
Copy link
Contributor Author

Pliner commented Apr 17, 2023

@tomchristie Thanks!

@tomchristie tomchristie merged commit 82466e1 into encode:master Apr 18, 2023
@Pliner Pliner deleted the retry-start-tls-errors branch April 18, 2023 09:34
@Pliner
Copy link
Contributor Author

Pliner commented Apr 19, 2023

@tomchristie Thanks for merging it. Could you please release it as 0.17.x?

@Pliner
Copy link
Contributor Author

Pliner commented May 16, 2023

@tomchristie Thanks for merging it. Could you please release it as 0.17.x?

@tomchristie Sorry for bothering you again. Could you please release these changes?

@tomchristie
Copy link
Member

@Pliner - Sure let's make that happen.
Would you like to attempt a release pull request with a minor version bump?
I can help guide you through the process.

@Pliner
Copy link
Contributor Author

Pliner commented May 16, 2023

@tomchristie Sure, thanks. Let me look on a previous one(#661), then I will create a new one.

@Pliner Pliner mentioned this pull request May 16, 2023
@Pliner
Copy link
Contributor Author

Pliner commented May 16, 2023

#685

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

Successfully merging this pull request may close these issues.

2 participants