Could ProxyTimeoutError inherit TimeoutError?

[aaugustin]

I noticed that ProxyConnectionError inherits OSError.

It would help me if ProxyTimeoutError followed the same pattern and inherited TimeoutError here.

Why it's a good idea in general

You're wrapping socket.timeout (which is TimeoutError on Python ≥ 3.10) and asyncio.TimeoutError (which is TimeoutError on Python ≥ 3.11) in a ProxyTimeoutError which adds details about the error.

I believe that wrapping in a subclass of the original exception is a good practice because catching the class of the original exception still works. Here's what I mean:

class Proxy:
    def connect():
        try:
            ...
        except TimeoutError as exc:
            raise ProxyTimeoutError("...") from exc

try:
    Proxy.connect()
except TimeoutError:  # this works only if ProxyTimeoutError inherits TimeoutError
    ...

Is it backwards-compatible?

Even though python-socks always wraps TimeoutError in ProxyTimeoutError, it's still possible to construct a scenario where this change would be backwards-incompatible:

try:
    do_something_that_can_raise_timeout_error()
    Proxy.connect()
except TimeoutError:
    # this branch would be taken after the change
    ...
except ProxyTimeoutError:
    # this branch was taken after the change
    ...

In my opinion, that pattern is fairly unlikely and it should be written as follows, with narrow exception catching:

try:
    do_something_that_can_raise_timeout_error()
except TimeoutError:
    ...
try:
    Proxy.connect()
except ProxyTimeoutError:
    ...

For this reason, I believe that the backwards compatibility concerns are bearable.

Why would it help me specifically

I just added support for SOCKS proxies in websockets with python-socks (python-websockets/websockets#1582). The library works well, thank you!

I'm now polishing error handling. websockets already contains code to retry automatically on connection errors and timeouts. Since ProxyConnectionError inherits OSError, automatic retries on ProxyConnectionError are already working without any change.

If ProxyTimeoutError inherited TimeoutError, it would work out of the box too. (Well, almost — I'd have to deal with asyncio.TimeoutError and TimeoutError being different classes in Python < 3.11.)

python-socks is an optional dependency of websockets because it may also be used for writing servers and there's no reason to install python-socks in that case. As a consequence, I cannot simply import ProxyTimeoutError in the code that handles retries; I need conditional imports; that creates complications.

If you don't make that change, I will rewrap ProxyTimeoutError into a standard TimeoutError... but wrapping back in the original exception class feels wrong... hence I'm suggesting this change.

[romis2012]

It seems reasonable. Fixed in v2.7.0

[aaugustin]

That was fast! Thank you so much!