Incorrect behavior when using timeout parameter

[psliwka]

Hello,
I've recently ran into an issue when trying to handle request timeouts in my code.

As I understand from the documentation, when the timeout fires, the request should fail raising a CancelledError. However, I could not reliably catch it.

Below there's a minimal example script to illustrate the problem:

import sys

import treq
from twisted.internet.defer import CancelledError, inlineCallbacks
from twisted.internet.task import react

# request to the URL below takes about 5 seconds to complete:
TEST_URL = 'http://httpbin.org/drip?duration=5'
TIMEOUT = float(sys.argv[-1])


@inlineCallbacks
def main(_):
    try:
        response = yield treq.get(TEST_URL, timeout=TIMEOUT)
        print response.code
    except CancelledError:
        print 'timeout'

react(main)

As stated in the comment, the request to the test URL requires about 5 seconds to complete. Provided that the timeout is long enough (say, 10 seconds), the request indeed completes correctly:

$ python example.py 10
200

In case the timeout is too short, I'd expect the script to print 'timeout'. Instead it raises an unhandled failure:

$ python example.py 4
main function encountered error
Traceback (most recent call last):
Failure: twisted.web._newclient.ResponseNeverReceived: [<twisted.python.failure.Failure twisted.internet.defer.CancelledError: >]

Make the timeout even shorter, so that the request is canceled even before establishing the connection, and it will raise different error instead:

$ python example.py 0.01
main function encountered error
Traceback (most recent call last):
Failure: twisted.internet.error.ConnectingCancelledError: IPv4Address(TCP, 'httpbin.org', 80)

Again, I believe that in both these cases the excepted behavior is that treq.get raises a CancelledError in deferred, which should be handled by the script above. Am I correct?

Tested with treq 15.0.0 and Twisted 15.4.0.

[mlowicki]

@glyph ping.

[glyph]

@mlowicki this is a great bug report, thanks. Sorry we haven't gotten to it yet. I am on vacation at the moment, so perhaps post another ping next week :).

[mlowicki]

@glyph pinging then ;)

[glyph]

Thanks, looking into it now :).

[mlowicki]

@glyph any luck?

[glyph]

Upon closer inspection, this is, unfortunately, sort of the expected behavior. IAgent.request is super vague about what type the Failure fires with, and the actual agents wrap their failures in private exceptions. Effectively, this is a duplicate of this issue from Twisted.

The good news is that since IRequest is so super vague about exception types, we could potentially fix this without really violating the interface :). However, that might break code that's handling errors properly right now, which is hard to do. So I think the main thing to do here is to make these exception types public and document them, and explain what IAgent is actually expected to fail with, when it fails. But most of this is stuff that should happen in Twisted, not in the wrappers within Treq.