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

asyncio.EventLoop.start_tls() returns None #105993

Closed
Dreamsorcerer opened this issue Jun 22, 2023 · 7 comments
Closed

asyncio.EventLoop.start_tls() returns None #105993

Dreamsorcerer opened this issue Jun 22, 2023 · 7 comments
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@Dreamsorcerer
Copy link
Contributor

Dreamsorcerer commented Jun 22, 2023

Bug report

The documentation says the function should return a transport:
https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.start_tls

But, in some cases it actually returns None. It looks like this may happen when it receives a closing transport:
aio-libs/aiohttp#3355 (comment)

I suspect this is because connection_lost() gets called during the start_tls() call, which ends up setting it to None:

self._app_transport = None

Should this be allowed to return None, or should there be an exception occurring here?
A simple change could be to check for None and raise an exception, but I'm not too familiar with how this part of the code is expected to work.

Linked PRs

@gvanrossum
Copy link
Member

It seems reasonable to return None in this case, maybe the docs need to be updated?

@Dreamsorcerer
Copy link
Contributor Author

OK, mainly wanted to confirm this wasn't deemed a bug before updating typeshed, but a doc update would be good too.

@Dreamsorcerer
Copy link
Contributor Author

I would note that aiohttp seems to already expect a bunch of exceptions from this function, so I think it'd be simpler for us if there was an exception here. But, I'm not really familiar with the code, so I don't have a strong opinion. If you want to stick with the current behaviour, I've made a PR for the documentation: #105995

@gvanrossum
Copy link
Member

I really don't know -- I have never used the TLS functionality in asyncio myself. Mayve @kumaraditya303 has an opinion?

@kumaraditya303
Copy link
Contributor

Have you investigated why aiohttp ends up passing a closing transport?

@Dreamsorcerer
Copy link
Contributor Author

I've not figured that out, but one cause might be something to do with the TLS-in-TLS proxy code. But, I guess client disconnection can happen at any time, so maybe it's just something that we'd expect to happen on the rare occasion, even when everything is working correctly.
There's various bits of information from different users in:
aio-libs/aiohttp#6239
aio-libs/aiohttp#3355

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 28, 2023
…p.start_tls` docs (pythonGH-105995)

(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 28, 2023
…p.start_tls` docs (pythonGH-105995)

(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jun 28, 2023
…p.start_tls` docs (pythonGH-105995)

(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
@kumaraditya303
Copy link
Contributor

Okay, the PR is merged now.

kumaraditya303 pushed a commit that referenced this issue Jun 28, 2023
…op.start_tls` docs (GH-105995) (#106189)

gh-105993: Add possible `None` return type to `asyncio.EventLoop.start_tls` docs (GH-105995)
(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
kumaraditya303 pushed a commit that referenced this issue Jun 28, 2023
…op.start_tls` docs (GH-105995) (#106188)

gh-105993: Add possible `None` return type to `asyncio.EventLoop.start_tls` docs (GH-105995)
(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
ambv pushed a commit that referenced this issue Jul 5, 2023
…op.start_tls` docs (GH-105995) (#106190)

(cherry picked from commit 6b52a58)

Co-authored-by: Sam Bull <git@sambull.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

4 participants