couldn't read response headers (HTTP::ConnectionError)

[tycooon]

I get this error when using persistent connection with high keep_alive_timeout value.

Test script:

require "http"

url = "https://www.google.com/"
client = HTTP.persistent(url, timeout: 1000)

puts Time.now
puts client.get(url).to_s.size
sleep 5
puts client.get(url).to_s.size
sleep 240
puts client.get(url).to_s.size

This consistently gives me the following output:

15551
15594
Traceback (most recent call last):
	6: from tmp/test-http.rb:28:in `<main>'
	5: from /Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/chainable.rb:20:in `get'
	4: from /Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/client.rb:43:in `request'
	3: from /Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/client.rb:66:in `perform'
	2: from /Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/connection.rb:99:in `read_headers!'
	1: from /Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/connection.rb:99:in `loop'
/Users/tycooon/.rbenv/versions/2.5.0/lib/ruby/gems/2.5.0/gems/http-3.0.0/lib/http/connection.rb:101:in `block in read_headers!': couldn't read response headers (HTTP::ConnectionError)

[britishtea]

I think #420 is related.

[tycooon]

I checked how httpclient gem works and it looks like it just reconnects and retries the request:

require "httpclient"
require "httplog"

HttpLog.configure do |config|
  config.logger = Logger.new(STDOUT)
  config.log_response = false
end

url = "https://www.google.com/"
client = HTTPClient.new
client.keep_alive_timeout = 1000

puts Time.now
puts client.get(url).body.to_s.size
sleep 5
puts client.get(url).body.to_s.size
sleep 240
puts client.get(url).body.to_s.size

D, [2018-02-15T19:13:55.099962 #88760] DEBUG -- : [httplog] Sending: GET https://www.google.com/
D, [2018-02-15T19:13:55.100057 #88760] DEBUG -- : [httplog] Data:
D, [2018-02-15T19:13:55.100245 #88760] DEBUG -- : [httplog] Connecting: www.google.com:443
D, [2018-02-15T19:13:55.454928 #88760] DEBUG -- : [httplog] Status: 200
D, [2018-02-15T19:13:55.455023 #88760] DEBUG -- : [httplog] Benchmark: 0.3291 seconds
15584
Cookie#domain returns dot-less domain name now. Use Cookie#dot_domain if you need "." at the beginning.
D, [2018-02-15T19:14:00.459097 #88760] DEBUG -- : [httplog] Sending: GET https://www.google.com/
D, [2018-02-15T19:14:00.459185 #88760] DEBUG -- : [httplog] Data:
D, [2018-02-15T19:14:00.570022 #88760] DEBUG -- : [httplog] Status: 200
D, [2018-02-15T19:14:00.570082 #88760] DEBUG -- : [httplog] Benchmark: 0.110737 seconds
15580
D, [2018-02-15T19:18:00.575761 #88760] DEBUG -- : [httplog] Sending: GET https://www.google.com/
D, [2018-02-15T19:18:00.575906 #88760] DEBUG -- : [httplog] Data:
D, [2018-02-15T19:18:00.576904 #88760] DEBUG -- : [httplog] Sending: GET https://www.google.com/
D, [2018-02-15T19:18:00.576980 #88760] DEBUG -- : [httplog] Data:
D, [2018-02-15T19:18:00.577129 #88760] DEBUG -- : [httplog] Connecting: www.google.com:443
D, [2018-02-15T19:18:00.884618 #88760] DEBUG -- : [httplog] Status: 200
D, [2018-02-15T19:18:00.884695 #88760] DEBUG -- : [httplog] Benchmark: 0.307552 seconds
15584

[tycooon]

So I checked how httpclient is handling this case and it raises the KeepAliveDisconnected error in case when socket.gets returns nil: https://github.com/nahi/httpclient/blob/27fbb07685930d1c8dbdc8dd1e72027f79879bbe/lib/httpclient/session.rb#L808. They always retry on this error once: https://github.com/nahi/httpclient/blob/27fbb07685930d1c8dbdc8dd1e72027f79879bbe/lib/httpclient.rb#L1131

I also checked this against my server and in this case the request does not reach the server (doesn't appear in nginx access log) so it's safe to retry it.

And by the way, I also didn't find any retrying for errors like Errno::ECONNABORTED or Errno::ECONNRESET. Does this mean that it's up to user to retry on all these errors?

[tarcieri]

Per what @britishtea noted this appears to be a dupe of #420.

This library has no feature for automatic retries. That would be useful, but should probably be opened as a separate issue.

The root cause appears to be the connection timing out, although I am confused (as in #420) why this occurs as an error reading the response as opposed to sending the request.

Regarding this:

And by the way, I also didn't find any retrying for errors like Errno::ECONNABORTED or Errno::ECONNRESET. Does this mean that it's up to user to retry on all these errors?

Again, there's no automatic retry support, however these sorts of errors are collectively rescued as SystemCallError and re-raised as HTTP::ConnectionError, the latter being what your code needs to handle. See:

https://github.com/httprb/http/blob/master/lib/http/request/writer.rb#L103
https://github.com/httprb/http/blob/master/lib/http/connection.rb#L219

[tycooon]

these sorts of errors are collectively rescued as SystemCallError and re-raised as HTTP::ConnectionError, the latter being what your code needs to handle

The problem is, HTTP::ConnectionError can be raised because of all sorts of errors and it's hard to tell which of them are always safe to retry and which are not – there is no link to the original exception at least.

In my opinion, if the library provides persistent connections support, it should either handle the retrying itself or provide some relevant errors so that client can figure out what to do.

[zanker]

You should be able to look at ‘cause’ on the exception to get the superclass if you want to contextually retry.

…

Sent from my iPhone

On Feb 21, 2018, at 23:53, Yuri Smirnov ***@***.***> wrote: these sorts of errors are collectively rescued as SystemCallError and re-raised as HTTP::ConnectionError, the latter being what your code needs to handle The problem is, HTTP::ConnectionError can be raised because of all sorts of errors and it's hard to tell which of them are always safe to retry and which are not – there is no link to the original exception at least. In my opinion, if the library provides persistent connections support, it should either handle the retrying itself or provide some relevant errors so that client can figure out what to do. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[tarcieri]

HTTP::ConnectionError can be raised because of all sorts of errors and it's hard to tell which of them are always safe to retry and which are not – there is no link to the original exception at least.

All of these errors indicate an error within the request lifecycle. Whether they should be retried is more a question of the request being performed, not the specific error that occurred.

If you have a counterexample where depending on the specific error that occurred, you'd retry it in one case but not another, I'd love to hear it.

In my own observations of e.g. browser retry logic, GET requests are repeatedly retried even if the error which is occurring is changing, and these retries continue until the page loads successfully or some max retry threshold is reached.

In my opinion, if the library provides persistent connections support, it should either handle the retrying itself or provide some relevant errors so that client can figure out what to do.

Then please open an issue describing the specific feature you'd like added.

[jrochkind]

In my own observations of e.g. browser retry logic, GET requests are repeatedly retried even if the error which is occurring is changing, and these retries continue until the page loads successfully or some max retry threshold is reached.

Yep, this is because "GET" is defined as both idempotent and "safe". https://codeahoy.com/2016/06/30/idempotent-and-safe-http-methods-why-do-they-matter/

Whether the request should be retried in general probably depends on the HTTP method. In general, i think it would be not a bad feature for http.rb to be optionally able to automatically retry appropriate methods.

But I'm not convinced that persistent connections isn't a special case. If you are using persistent connections, the connection the client thought was still open in fact being dropped by the server is probably something you expect to run into, regularly. If it's possible to catch this case (and I'm not sure it is), I think it would be appropriate for http.rb to automatically re-establish connection in all cases, without it even being an option, without regard to the HTTP method in question. if it's possible to tell that the connection had been dropped and the request definitely never made it to the server -- not sure if it is, but this is a special case relevant to persistent connections.

I thin browsers must do something special here, because browsers will not retry POST in the general case; browsers DO use persistent connections; and browsers are not always complaining "Oh, we thought it was a persistent connection but it got dropped but it was a non-idempotent POST so we can't safely retry it, sorry you're out of luck".

[cpb]

I'm noticing this when the server signals to close the connection, and I found this resolved things for me:

    class RespectTheClose < Http::Feature
      def wrap_response(response)
        case response[:connection] when /close/i then
          response.flush.connection.close
        end

        response
      end

      Http::Options.register_feature(:respect_the_close,self)
    end

Do the response headers include a Connection header?

[ixti]

Yes. Server might respond with Connection: close header to notify client that it wants to close connection. And I believe this should be handled by core.

[froodian]

For the scenario where the server sent the Connection: close header in a previous response, an updated version of the workaround above:

module HTTP
  module Features
    class RespectConnectionCloseResponseHeader < HTTP::Feature
      def wrap_response(response)
        if response[:connection]&.casecmp("close") == 0
          response.flush.connection.close
        end
        return response
      end
      HTTP::Options.register_feature(:respect_connection_close_response_header, self)
    end
  end
end

and then at the usage site, instead of HTTP.persistent(url), use

HTTP.use(:respect_connection_close_response_header).persistent(url)

In terms of why that is happening, and maybe getting the issue fixed in the library, this appears to be necessary because this line of code is trying to call close on the connection unless keep_alive?, but that keep_alive? check is only checking whether the request headers specified keep-alive or close, and nothing checks whether the response headers specified keep-alive or close.