Twisted.web.server does not notify if a connection is cancelled by client

Question

The documentation implies that the Deferred at request.notifyFinish() should be notified if the connection drops for any reason. I loaded similar code below:

from twisted.web.resource import Resource
from twisted.web.server import NOT_DONE_YET
from twisted.internet import reactor
from twisted.logger import Logger

log = Logger()

class DelayedResource(Resource):
    isLeaf = 1

    def _delayedRender(self, request):
        log.info("Rendered!")
        request.write("<html><body>Sorry to keep you waiting.</body></html>")
        request.finish()

    def _responseFailed(self, err, call):
        call.cancel()
        log.info("No. Wait. Stop")

    def render_GET(self, request):
        log.info("Lets try this!")
        call = reactor.callLater(5, self._delayedRender, request)
        request.notifyFinish().addErrback(self._responseFailed, call)
        return NOT_DONE_YET

If I cancel the connection from the client, _responseFailed does not get called. The log statement "No. Wait. Stop" is never printed and the "Rendered!" statement is printed.

This is important, because in my real use case the I am allocating resources that need to be deallocated if the connection drops. After upgrading to twisted 17.1.0, these stopped getting cleaned up.

Am I doing this wrong? Or is this a bug in twisted?

It'd be nice if you completed your code snippet so it's a self-contained runnable example. And, yea, the feature looks broken to me. — Jean-Paul Calderone, Jun 29 '17 at 23:56
@Jean-PaulCalderone are you saying this is a broken feature in Twisted or the OP's code? I've noticed something similar in the past and figured the browsers or OS was doing something and not cutting the connection. — notorious.no, Jun 30 '17 at 14:39
Preponderance of the evidence is pointing to a bug in twisted, which is vexingly labeled "won't fix" https://twistedmatrix.com/trac/ticket/8692 — Matthew Scouten, Jun 30 '17 at 15:15
@notorious.no broken feature in Twisted. I didn't realize there was a closed-wontfix issue (as found by @matthew-scouten). That the issue is closed-wontfix makes me sad, indeed. — Jean-Paul Calderone, Jul 01 '17 at 00:29
I happen to know two things. (1) He is far more advanced technology than any mere "robot". (2) He is capable of prodigious feats of both sleep and non-sleep. — Jean-Paul Calderone, Jul 04 '17 at 17:56

score 1 · Accepted Answer · answered Jul 04 '17 at 18:02

1

This was due to a bug (or an accidentally introduced feature - you be the judge!) introduced by http://tm.tl/8320 but recently fixed by http://tm.tl/8692. Therefore, Twisted 16.3 through Twisted 17.5 will not fire notifyFinish() with a Failure (though it seems it still fires in the success case). Due to complexities in the HTTP implementation and the underlying features of TCP, even versions of Twisted outside that range may not always fire notifyFinish() with a Failure in cases where the HTTP client observes some kind of error. Bear this in mind and make sure that any processing that must occur at the end of a request does not rely on an errback on this Deferred (ie, attach a callback as well or ensure that processing runs as part of the response generation code).

answered Jul 04 '17 at 18:02

Jean-Paul Calderone

47,755
6
94
122

Versions post Twisted 17.5 are supposed to be fixed? Good. Except it looks like 17.5 is the current highest version. I guess the answer is "wait until the next twisted version". – Matthew Scouten Jul 05 '17 at 14:14
Indeed. Though, to be safe, one could also test out master@HEAD and verify that the implemented fix actually provides the desired behavior (and then, if it doesn't, complain before the next release :). – Jean-Paul Calderone Jul 05 '17 at 16:37
We eventually got around to updating to a more recent twisted and the problem seems to be solved. – Matthew Scouten Jan 31 '19 at 20:23

Twisted.web.server does not notify if a connection is cancelled by client

1 Answers1