[Twisted-Python] Raising exception from a Deferred canceller.

Terry Jones terry at jon.es
Thu Aug 29 04:18:15 MDT 2013


Hi Kai

I think it's helpful to keep clear on two different things that cancelation
is intended to do: 1) to fire the original deferred so that things relying
on it can proceed, and 2) to try to terminate an ongoing action that the
deferred might be waiting on.

For 1, I think calling cancel() should *always* result in the deferred
being fired (with, as it currently stands, CancelledError being used if a
provided cancel function does not fire the deferred itself). Always firing
the deferred is very important because the caller of cancel may have set up
many deferreds that rely on each other and their entire program may not be
able to proceed at all until the offending deferred is actually fired. It's
also contractually simple, and easy to document & understand.

For 2, the question is: do we want to also return information to the caller
if 2a) the underlying cancel function detects that it cannot, or can no
longer, stop the operation, or 2b) there is some kind of exception when
cancel calls the cancellation function.  I don't think 2a) is really an
exception situation, so it makes sense, as you say, just to return False
from cancel in this case. It's basically the cancel function saying it was
too late to do anything about the underlying operation but not providing
more information than that. Internally raising and catching
CancellationFailedError (as in your code) in that case seems good to me.
 In the case of 2b) I would just let the exception bubble up to the calling
code. Agreed, it could break some existing code, but isn't that existing
code already subject to that exact failure? It's just currently
undefined/undocumented.

Terry



On Thu, Aug 29, 2013 at 10:27 AM, zhang kai <kylerzhang11 at gmail.com> wrote:

> Hi,
>
> As itamar mentioned in ticket #6676 <http://tm.tl/#6676>, If a
> cancellation function for a Deferred throws an exception(the cancel() method
> of Deferred won’t throw exceptions, but the canceller may), behavior is
> undefined. If the cancellation function throws an exception it is currently
> not caught, and cancellation does not occur.
>
> We can catch the exception and log it, and fallback to just firing
> Deferred withCancelledError. This won’t break any old code. But an
> exception raising from the cancellation function often means the
> cancellation is failed.
>
> Another option we have is taking this opportunity to make the cancellation
> being able to fail. There is the motivation:
>
> There are cases where a Deferred is uncancellable. For example, we can
> call twisted.mail.imap4.IMAP4Client.delete to delete a mailbox. When the
> operation is waiting in the queue, we can cancel it by removing it from the
> queue. However, when the operation is already sent and is waiting for the
> response, it becomes uncancellable.
>
> If we allow the canceller(NOT the cancel() method of the Deferred) to
> raise an exception, we can tell the user the cancellation is failed and the
> Deferredwon’t be fired with a CancelledError.
>
> Raising an exception from cancel() may break the old code. So we can
> catch the exception raised by the canceller, then return a False without
> firing theDeferred to tell the user that the cancellation is failed.
>
> In order to avoid missing unexpected exceptions, we can create a
> CancellationFailedError. When the canceller raises CancellationFailedError,
> we catch it and return False. When the canceller raises others
> exceptions, we catch it, log it then return False.
>
> Something like this:
>
> def cancel(self):
>     if not self.called:
>         canceller = self._canceller
>         if canceller:
>             try:
>                 canceller(self)
>             except CancellationFailedError:
>                 return False
>             except Exception:
>                 log.err(None, "Unexpected exception from canceller.")
>                 return False
>         else:
>             # Arrange to eat the callback that will eventually be fired
>             # since there was no real canceller.
>             self._suppressAlreadyCalled = True
>         if not self.called:
>             # There was no canceller, or the canceller didn't call
>             # callback or errback.
>             self.errback(failure.Failure(CancelledError()))
>         return True
>     elif isinstance(self.result, Deferred):
>         # Waiting for another deferred -- cancel it instead.
>         return self.result.cancel()
>     else:
>         return False
>
> This won’t break any code by raising an exception from cancel(), although
> some code may rely on cancel() not returning any value.
>
> So, what’s your opinion on raising an exception from the canceller?
>
>
> Regards,
>
> -Kai
>
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://twistedmatrix.com/pipermail/twisted-python/attachments/20130829/657cd96c/attachment-0001.html>


More information about the Twisted-Python mailing list