ironport / shrapnel

Shrapnel is a scalable, high-performance cooperative threading library for Python.
Other
219 stars 34 forks source link

Make coro.TimeoutError subclass Exception #43

Closed ghost closed 11 years ago

ghost commented 11 years ago

The _coro.coro.TimeoutError class inherits from _coro.coro.Interrupted, which, in turn, inherits from BaseException. Simple Exception is not subclassed. Therefore, timeouts cannot be trapped so:

    try:
        coro.with_timeout(timeout, method)
    except Exception, unused:
        handle_exception()

Instead, one must use:

    try:
        coro.with_timeout(timeout, method)
    except (Exception, TimeoutError), unused:
        handle_exception()

According to documentation, BaseException is not to be directly inherited by user-defined classes. Likewise, BaseException is used in terminal situations which are not meant to be (normally) intercepted by the user. For example, GeneratorExit is a subclass of BaseException, while StopIteration subclasses Exception. Users are expected to handle StopIteration, while GeneratorExit is not often observed in userland.

Generally, I think timeout situation should not be considered as a terminal condition. If there is a possibility to impose an explicit timeout on a some operation, I think it should be possible to trap and handle the related exception on the same level, as anticipated, and not terminal, situation.

ehuss commented 11 years ago

That was done on purpose. Interrupted is considered a precious exception. "method" may be hundreds of thousands of lines of code, possibly in third party libraries, and you cannot guarantee that something might not catch all exceptions and eat them (as your example does). That would prevent timeouts from being handled. Wildcard exception catching should be extremely rare, but alas some people still do it. You can think of a timeout to be equivalent to SystemExit or the same reasoning why GeneratorExit inherits BaseException. The workaround is trivial if you really do want to handle TimeoutError the same as "some random exception", which I suspect is the wrong thing to do in most cases.

ghost commented 11 years ago

Thank you for the explanation, I accept your reasoning. In my case, TimeoutError is, usually, more like "some random exception". Server cluster coordination, or some kind of reliable communication with failover would imply that timeout is not a reason to quit, only to move on now or retry later. Of course, this is not a general case.

I certainly appreciate the distinctive nature of Interrupted in case of Shutdown exception. I have a coroutine which calls a callback in case it experiences an error condition, but not explicit shutdown.