Deferred object created from future returns wrong CancelledError type

[Adam Wierzbicki]

Caller of Deferred.cancel() expects the cancelled deferred object to return twisted.internet.defer.CancelledError. On the other hand, calling Future.cancel() results in concurrent.futures.CancelledError. This causes problems with deferred objects created with Deferred.fromFuture(). E.g. setting timeout for such objects won't work because _cancelledToTimedOutError only traps twisted.internet.defer.CancelledError and not concurrent.futures.CancelledError.

[Kyle Altendorf]

For the record: ​https://github.com/twisted/twisted/pull/1169

[Glyph]

HI Adam,

Thanks for this contribution. Sorry it's taken so long to get to. (If you become a contributor and review code too, that queue will move faster :-))

This change raises some problematic compatibility concerns. The fact that the existing tests raise asyncio exceptions means it was a considered property of the implementation, not an accident; code using this integration in anger and properly handling these errors will be broken by this change, with very little warning. We have a policy that governs this sort of change: ​https://twistedmatrix.com/documents/current/core/development/policy/compatibility-policy.html

I'd say that probably the compatible way forward here is to make a new hybrid cancellation error, one that inherits from *both* asyncio and Twisted exception types, so that both kinds of thing can work; existing code that handles asyncio cancellation will keep working, but code which depends on twisted's exception types will also work.

There are other things we could do though, like fixing the timeout code. It's not generally valid to assume .cancel() will always give you a cancellation exception, since it might stop a process midway through, resulting in some other exception type, or perhaps even a partial success result.

When you have time to address this, please submit for re-review. Thanks!

This is sufficiently subtle and foundational that it might be a good idea to raise it on the mailing list as well.