Should response Body be fully read for HTTP connection re-use?

[Chandru]

Should I completely read the body of response (my be copy into ioutil.Discard), even if I'm not interested in it, for http.Client to re-use connections?

--
Chandra Sekar.S

[Dave Cheney]

Yes, and if you don't resp.Body.Close() will do it for you.

[James Bardin]

On Wednesday, December 10, 2014 8:49:59 AM UTC-5, Dave Cheney wrote:

Yes, and if you don't resp.Body.Close() will do it for you.

I'm fairly certain that's not true, otherwise closing an unneeded response could read from the connection indefinitely.

I think it does get read implicitly if it fits entirely in the read buffer, but there's no guarantee. Reading it all will ensure that the transport attempts to reuse the connection.

[Chandru]

After some digging into stdlib code, http://golang.org/src/pkg/net/http/transport.go?h=bodyEOFSignal#L1112 and http://golang.org/src/pkg/net/http/transport.go?h=bodyEOFSignal#L841 suggest the connection will be closed if body is not read till end.

Am I reading it wrong?

--
Chandra Sekar.S

On Wed, Dec 10, 2014 at 7:19 PM, Dave Cheney <da...@cheney.net> wrote:

Yes, and if you don't resp.Body.Close() will do it for you.

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[James Bardin]

On Wed, Dec 10, 2014 at 9:34 AM, Chandru <chand...@gmail.com> wrote:

Am I reading it wrong?

No, you are correct. Some of the body may be read into a buffer, but there's no guarantee how much.

[Dave Cheney]

I am sorry. This logic has changed in the past a few times and I am out of date.

I think the current logic is

io.Copy(ioutil.Discard, resp.Body); resp.Body.Close() // connection
will be reused
resp.Body.Close() // remaining body will be discarded, and the
connection will _not_ be reused.

[cbro...@gmail.com]

Is it considered a best practice to always add the following after http.DefaultClient.Do returns a non-error response?

defer res.Body.Close()

defer io.Copy(ioutil.Discard, res.Body)

That way subsequent code in the function can read the body however it sees fit, but if it doesn't read it to completion for some reason, we cleanup and make sure the connection can be re-used. And it's a no-op if the body was already read fully.

[James Bardin]

On Mon, Aug 24, 2015 at 12:48 PM, <cbro...@gmail.com> wrote:

Is it considered a best practice to always add the following after http.DefaultClient.Do returns a non-error response?

defer res.Body.Close()

defer io.Copy(ioutil.Discard, res.Body)

That way subsequent code in the function can read the body however it sees fit, but if it doesn't read it to completion for some reason, we cleanup and make sure the connection can be re-used. And it's a no-op if the body was already read fully.

defer'ing the Body.Close is definitely a best practice. 

You can add the io.Copy call if it suits your code, though I can't think of many cases where I didn't know if I would read the Body or not. While not exactly a noop, I don't think it ever really hurts to add it when you might need it, since ioutil.Discard gets its copy buffer from a sync.Pool.

[cbro...@gmail.com]

Thanks. There are 3 common cases I've encountered where the io.Copy is needed:

- Reading the body using json.Decoder - it can stop without reading the entire body contents

- Early return without reading the body b/c of a non-200 response

- Client code doing a POST where it checks the http response code without reading the body

Deferring the io.Copy at the same spot as the Close lets me ensure the body is always read without having to manually review each error case

[James Bardin]

On Mon, Aug 24, 2015 at 1:21 PM, <cbro...@gmail.com> wrote:

Thanks. There are 3 common cases I've encountered where the io.Copy is needed:

- Reading the body using json.Decoder - it can stop without reading the entire body contents

- Early return without reading the body b/c of a non-200 response

- Client code doing a POST where it checks the http response code without reading the body

Deferring the io.Copy at the same spot as the Close lets me ensure the body is always read without having to manually review each error case

All good points. 

A couple counterpoints to using this universally:

- You may not want to wait for the full response, and discarding the connection is better or faster for various reasons. You could dispatch it in another goroutine, but then you need to move the Close to that goroutine, it adds a little more overhead, and wastes bandwidth for large responses.

- You need to watch out for unexpectedly large responses, though you can mitigate this somewhat by wrapping the resp.Body in an io.LimitedReader.

[cbro...@gmail.com]

Good point about large responses 

[pierre....@schibsted.com]

Some news about: https://groups.google.com/forum/#!topic/golang-nuts/4Rr8BYVKrAI

And the corresponding code: https://github.com/golang/go/blob/6df4c3a44b20b9738ae5699fbc156052522f9f54/src/net/http/transfer.go#L931