Streaming requests should raise an error when network problems occur

[swinton]

It seems streaming requests (as per example given here) do not raise an error when the connection is dropped or when there are network problems that prevent the stream from being consumed. This makes it difficult to know if we should reconnect to the stream.

If an error was raised, then disconnections could be handled gracefully, e.g.

while True:
    try:
        for line in r.iter_lines():
            if line: # filter out keep-alive new lines
                print json.loads(line)
    except ConnectionError:
        # reconnect...

[slingamn]

See also #844.

[swinton]

I'd quite happily have a go at implementing a fix for this myself, if someone more familiar with the codebase could give me a few pointers of where to start?

[slingamn]

Check out iter_content in requests/models.py. This defines an iterator that does self.raw.read(chunk_size) repeatedly --- this read call goes to a requests.packages.urllib3.response.HTTPResponse, and from there to an httplib.HTTPResponse, and from there to a socket.

From my cursory inspection of the code, it's actually surprising to me that socket errors from these read calls are not being properly raised. I don't see anything that would be suppressing them :-/

Do you have a test case for the issue?

Thanks for the bug report and the offer of a patch, always appreciated :-) Tell us if we can offer any more guidance.

[ruli]

Good to know that this issue has some attention.

Just for the moment, what would be the best way to handle server side or client side HTTP errors when streaming?

The current approach I'm using now. I don't know if it's going to work though, as I'm not sure whether r.status_code is going to change on the fly. Is it better to actually check r.status_code, or check r.headers['connection']?

r = request.get(url, ...)

while r.status_code >= 200 and r.status_code <= 299:
    for line in r.iter_lines(chunk_size=chunk_size):
        if line:
            # do great things
else:
    # handle potential HTTP errors

And furthermore, how do I appropriately implement retry logic in this case?

[sigmavirus24]

@ruli look at requests/models.py around line 735, status_code will never change as a result of using iter_lines or iter_chunks. Response.headers does not seem to change either unfortunately.

I think there is retry logic recipe (in general) in the docs written as a hook. I'm not 100% sure though. Hope this gives you a hand.

[ruli]

Ah, alright. So the hook would give me a timely update on the status? I'm checking the docs and haven't seen it mentioned though. And my impression is that if I attempt to retrieve the headers, then the connection is automagically closed even if no error (server/client side) has occured. Right now I'll just use a naive way:

try:
    while 1:
        r = requests.get(url, ..., prefetch=False)

        if r.status_code == 200:
            for line in r.iter_lines():
                if line:
                    # do greatness
        else:
            # log
            # exponential or fixed time.sleep(x)
except ... # Major nastiness happened, we may not see 2013 coming

This use case is used primarily for (theoretically) endless HTTP streams.

[jkl1337]

For a stream with an expected rate of responses, it is probably best to set a sanity timeout setting for the request. This timeout should be the maximum time a socket call will block in http.client/httplib.

The status_code will never change for an HTTP response, and it would be surprising to change its behavior. It seems the issue is the general one that network problems can cause what may be effectively indefinite delays. If the OS raises an socket.error it should be rethrown as a ConnectionError but the delay before the OS assumes the network is dead or a TCP FIN is received can be many minutes.

TCP does have the concept of keep alive. This can be enabled with s.setsockopt(socket.SOL_SOCKET, socket.SO_KEEPALIVE, 1). It is disabled by default in Linux and probably Mac OS X. This can cause a more prompt failure in the right conditions. But this may be overcomplicated and less effective than a request timeout.

[sigmavirus24]

@jkl1337 we don't set the options on the sockets and I'm not sure if urllib3 does either (although if it does, it would probably be a good idea to submit the idea to @shazow). That aside, it would be ideal if we could use select but that would break Windows support.

We could definitely catch and re-throw the socket.error. That's easy. The problem it seems (to me) is that reading from a broken socket isn't raising any exception at all.

[jkl1337]

There is already a timeout parameter available for the requests constructor. I tested a bit and it and it seems to work. My reading of the original issue is that it is a misunderstanding of what happens when TCP connections go bad (or not). For HTTP streaming or long-polling if a timeout is not set, one can expect to block indefinitely for numerous network failure modes. This is one of the reasons WebSockets has PING/PONG.

The solution to the original post I believe is to just use an appropriate timeout argument to requests.

That said, there might be an API defect in how the exceptions are handled (though they still come through):
When a timeout is set a socket.timeout may be raised out of self.raw.read() in models.py.
However, it looks from adapters.py that it is both not expecting a socket.timeout to be raised and also the r.content property access is outside of the try block, so any socket exception doesn't get translated to a one of those urllib3 types.

Of course, the same will occur for the streaming case. A raw socket.error or socket.timeout will be propagated. So I am not yet sure what is the intent of the translation in HTTPAdapter.

The exact tests I used were an HTTP server that returns a Content-Length greater than the amount of data sent before a long pause (no FIN sent) and also an immediate close() after partial transfer. In the former a socket.timeout occurs (from self.raw.read() in models) and in the latter a socket.error(ECONNRESET) is propagated as expected.