mreiferson / go-httpclient Goto Github PK

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

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

After further investigation, I realized that there may be an easier path...

Instead of hacking around with custom protocols to allow us to hook into the built in HTTP client we could instead provide a custom dial function to http.Transport that returned our own custom type that satisfies the net.Conn interface and extended deadlines on every call to read/write. It could also use DialTimeout to provide connection timeout.

There are some benefits and tradeoffs, though:

• You would no longer have access to the connection on the client side.
• The amount of work that the built in HTTP client does may be overkill for some high-performance, high-volume, HTTP only internal applications. This is largely resolved by this package (no goroutine per connection and no additional allocations per request).
• However, on the plus side, this would resolve any need to reproduce functionality such as HTTPS and all the other nice things the built in client provides.

It would look something like this:

type deadlinedConn struct {
    net.Conn
}

func (c *deadlinedConn) Read(b []byte) (n int, err error) {
    c.Conn.SetReadDeadline(time.Second)
    return c.Conn.Read(b)
}

func (c *deadlinedConn) Write(b []byte) (n int, err error) {
    c.Conn.SetWriteDeadline(time.Second)
    return c.Conn.Write(b)
}

transport := &http.Transport{
    Dial: func(netw, addr string) (net.Conn, error) {
        c, err := net.DialTimeout(netw, addr, time.Second)
        if err != nil {
            return nil, err
        }
        return &deadlinedConn{c}, nil
    },
}
httpclient := &http.Client{Transport: transport}

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?

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.

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? 🤘