a Go HTTP client with timeouts

I might be doing something wrong but I'm on Go 1.1 and i'm using the new code from Master and when I do more than 1 request I get the following error:

read tcp xxx.xxx.xxx.xxx:80: use of closed network connection

Summary of my code:

var TimeoutTransport = &httpclient.Transport{
    ConnectTimeout:        1*time.Second,
    RequestTimeout:        10*time.Second,
    ResponseHeaderTimeout: 5*time.Second,
}

func example() {
client := &http.Client{Transport: TimeoutTransport}
  for _, url := range *segmentUrls {
    resp, err := client.Get(url)
    defer resp.Body.Close()
   //[...]
  }

}

I might be doing something wrong but I get this error randomly :(

Might be related to: https://code.google.com/p/go/issues/detail?id=4704

Proposal for an IdleTimeout option on the httpclient.Transport.

• This allows finer control of the timeouts on the client, and gives the caller an actual timeout error when hit, vs the closed connection error returned after canceling the request.
• This obviously interferes with Keepalive connections. Not sure if it should disable them by default, or just note it in the documentation.
• I punted a bit on the tests -- really need to put in a slow-writer endpoint to test for the IdleTimeout specifically.

httpclient has been cleaned up so that the user never sees any references to 'hc_http(s)?' when they get their request back OR inside their redirect policy (in response to issue #4).

Fixing the issue for the response's resp.Request.URL object was straightforward (we simply remove it in httpclient.Do), however removing it from the redirect chain required some h4x0ring.

In order to do this, we first clean up the new redirect to be considered and the newest entry in the array of previous requests. After consulting the user defined RedirectPolicy, we reinstate the 'hc_' prefix to the current request (or else our custom RoundTripper doesn't get triggered). Is there a better way?!

The "hc_" hack was generalized to also work for https connections. In addition, there now exists a TLSClientConfig object within the HttpClient object that can be used to control the TLS settings for HTTPS connections. One caviat is that the object cannot be moved anywhere in memory, so it can only be changed inplace.

• Switch from an interface to a struct pointer in the rwTimeoutConn wrapper. In extreme cases, the interface overhead may be up to 5% (though it's probably very close to 0% at normal network speeds). Since http is only a TCP protocol, we don't need the net.Conn abstraction anyway.

I'm not entirely sure if there is a way to handle this, but filing an issue nonetheless. Essentially once a connection is cached, it stays there and will get reused the next time the same connection is required. If during this time the remote server closes the connection, the socket has already entered the CLOSE_WAIT state on the client side, but the library will still attempt to reuse this connection but the request will fail. It would be nice if this state could be detected in checkConnCache and the cached connection were dropped and not reused. I know there is no 100% solution to detecting closed connections (especially wrt NAT), it would be great if it worked for some of the more common possibly detectable cases.

I got here trying to add improved certificate handling to Kubernetes- your package is a dependency of Google's certificate-transparency which is in turn a dependency of cfssl. There's a Kubernetes build script that is literally grepping for the "Copyright" line to assemble a combined license file and my pre-commit tests fail without it. Would you mind adding one?

add feature of set tcp send buffer or receive buffer.

This is an advanced option for advanced users who want to tune low level TCP parameters to try and squeeze out more performance. Especially in the WAN environment.

When I tried using this example code:

transport := &httpclient.Transport{
    ConnectTimeout: 1*time.Second,
    ResponseHeaderTimeout: 5*time.Second,
    RequestTimeout: 10*time.Second,
}
defer transport.Close()

client := &http.Client{Transport: transport}
req, _ := http.NewRequest("GET", "http://127.0.0.1/test", nil)
resp, err := client.Do(req)
if err != nil {
    return err
}
defer resp.Body.Close()

in several places in my code, I ended up getting connection leaks. After running lsof -p <pid> I would see hundreds of connections in the ESTABLISHED state, even though these connections should have been closed.

After changing to using a single global client:

var globalClient *http.Client

func init() {
    t := &httpclient.Transport{
        ConnectTimeout:        60 * time.Second,
        RequestTimeout:        60 * time.Second,
        ResponseHeaderTimeout: 60 * time.Second,
    }
    globalClient = &http.Client{Transport: t}
}


func doSomeRequest() {
    req, _ := http.NewRequest("GET", destUrl, nil)
    resp, err := globalClient.Do(req)
}

the problem seemed to go away.

I'm posting this ticket for two reason:

1. You may want to put a note in the example code to mention that it might be wise to re-use the client object
2. Can you think of any issues to beware of with the approach mentioned above?

• ResponseHeaderTimeout were not passed to http.Transport, fixed it
• time.Timer already does all we need, and do it relatively well till 100_000 rps. If higher rate needed, than possibly custom queue should be kept.

When introspecting into a response's Request.URL object, the scheme still has the hc_ prefix from go-httpclient's internal workings. In my opinion, we should do something to clean this up so that the end user doesn't have to worry about any of the internal machinery.

Example:

httpclient := httpclient.New()
req, err := http.NewRequest("GET", "http://bitly.com", nil)
resp, err := httpclient.Do(req)

log.Println("URL: ", resp.Request.URL.String())

This will output:

URL: hc_http://bitly.com

Every exported function in a program should have a doc comment. The first sentence should be a summary that starts with the name being declared. From effective go.

I generated this with CodeLingo and I'm keen to get some feedback, but this is automated so feel free to close it and just say "opt out" to opt out of future CodeLingo outreach PRs.

Hiya!

We use a copy of go-httpclient in one of our internal applications, but occasionally see a weird test failure

=== RUN TestHttpsConnection
--- FAIL: TestHttpsConnection (3.00 seconds)
    httpclient_test.go:93: 1st request failed - Get https://httpbin.org/ip: read tcp 23.21.162.118:443: use of closed network connection
=== RUN TestHttpClient
--- PASS: TestHttpClient (0.45 seconds)
=== RUN TestSlowServer
--- PASS: TestSlowServer (0.50 seconds)
=== RUN TestMultipleRequests
--- PASS: TestMultipleRequests (2.00 seconds)
FAIL
FAIL    github.com/mreiferson/go-httpclient 5.965s

Have you seen this before, is it a known issue? :metal: