Uh oh!
There was an error while loading. Please reload this page.
Properly wrap OS errors when using trio - #225
Conversation
cdeler
left a comment
There was a problem hiding this comment.
Looks good for me (with some naming remarks)
| ), | ||
| ], | ||
| ) | ||
| async def test_cannot_connect_tcp(backend: str, url) -> None: |
There was a problem hiding this comment.
May be we can make it catchy, a-la test_tcp_connection_errors_are_properly_wrapped, like in BDD (then we can omit the comment)
| @pytest.mark.anyio | ||
| async def test_cannot_connect_uds(backend: str) -> None: |
| ) -> AsyncSocketStream: | ||
| connect_timeout = timeout.get("connect") | ||
| unicode_host = hostname.decode("utf-8") | ||
| exc_map = { |
There was a problem hiding this comment.
Small nit: we could move the mapping to the class level. That way it's not created on each method call, and also could be shared between open_tcp_stream() and open_uds_stream() since the content is the same.
I see that the other backends have the same issue, so happy to defer this to a followup PR if we think it's worth doing.
There was a problem hiding this comment.
I like that this style makes it simple-as-cake to visually keep track of which exactly exceptions we map in each case, and I think moving it to a class variable (further away than from where it's actually used) wouldn't help with readability either. So overall, I'm thinking this may not really be worth a follow-up?
Closes#224
The problem in #224 turns out to hit
trio/anyio+triofor both TCP and UDS.We were not properly mapping
OSErrorin those places, and this PR fixes that, adding regression tests to our test suite to make sure the proper behavior applies everywhere.