twisted icon indicating copy to clipboard operation
twisted copied to clipboard

readBody from twisted.web.client cannot be correctly canceled

Open taroved opened this issue 3 months ago • 3 comments

twisted.internet.defer.AlreadyCalledError exception if you cancel readBody deferred readBody from twisted.web.client cannot be correctly canceled if connection is hang

How to cause this behavior

To reproduce the problem you need slow server or broken http server which doesn't response full content:

from twisted.web import server, resource
from twisted.web.server import NOT_DONE_YET
from twisted.internet import reactor, defer, endpoints

def printme(request):
    request.write('Message'.encode('utf-8'))
    # there is no request.finish() so the connection hangs


class MySite(resource.Resource):
    isLeaf = True
    
    def render_GET(self, request):
        reactor.callLater(1, printme, request)
        return NOT_DONE_YET


def run():
    endpoints.serverFromString(reactor, "tcp:%s" % 9999).listen(server.Site(MySite()))
    reactor.run()

run()

Client code:

from twisted.web.client import Agent, readBody
from twisted.internet import reactor

def startDownload(response):
    print("response", response.code)

    d = readBody(response)
    reactor.callLater(3, d.cancel)  # cancel should stop hanged connection. From time to time it doesn't close all sockets but it's not the topic of the issue

    d.addBoth(lambda canceled_error: print('Canceled'))


def main():
    a = Agent(reactor)
    d = a.request(b"GET", b"http://127.0.0.1:9999/")
    d.addCallback(startDownload)
    d.addErrback(lambda error: print('Download start error: ', error))

    reactor.callLater(5, lambda: reactor.stop())

if __name__ == "__main__":
    main()
    reactor.run()

Run server and client in separate consoles and you will see exception in client console:

Traceback of exception:

while interacting with body decoder:
Traceback (most recent call last):
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/web/_newclient.py", line 546, in connectionLost
    self.response._bodyDataFinished(
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/web/_newclient.py", line 1057, in dispatcher
    return func(*args, **kwargs)
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/web/_newclient.py", line 1301, in _bodyDataFinished_CONNECTED
    self._bodyProtocol.connectionLost(reason)
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/web/client.py", line 1749, in connectionLost
    self.deferred.errback(reason)
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/internet/defer.py", line 926, in errback
    self._startRunCallbacks(fail)
  File "/Users/taroved/venv_downloader/lib/python3.11/site-packages/twisted/internet/defer.py", line 982, in _startRunCallbacks
    raise AlreadyCalledError
twisted.internet.defer.AlreadyCalledError: 

Correct behavour I suppose that cancel of hanged client requests should work silently

I wrote dirty fix

What do you think about it?

Testing environment

  • on Linux, Linux 6.8.0-71-generic #71-Ubuntu SMP PREEMPT_DYNAMIC Tue Jul 22 16:52:38 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux DISTRIB_ID=Ubuntu DISTRIB_RELEASE=24.04 DISTRIB_CODENAME=noble DISTRIB_DESCRIPTION="Ubuntu 24.04.2 LTS"
  • on macOS, ProductName: macOS ProductVersion: 15.4.1 BuildVersion: 24E263
  • Twisted version
    • 24.11.0

taroved avatar Aug 30 '25 04:08 taroved

Many thanks for the detailed report.

I think that the fix is ok. We will need at least one automated test to merge the fix.

My understanding is that

  1. You cancel the request and this triggers the error
  2. When connection is lost it tries to trigger the error again

adiroiban avatar Aug 30 '25 09:08 adiroiban

It seems your thoughts are right.

I created PR #12502 and added test in it.

taroved avatar Sep 11 '25 19:09 taroved

@taroved thanks so much for both reporting this and working on it.

glyph avatar Sep 12 '25 04:09 glyph