go1.8 and json.NewDecoder(resp.Body)

[John Shahid]

Hi all,

Some background, we (CloudFoundry) noticed some weird behavior after upgrading from go1.7.4 to go1.8.1. An http client that was polling 260 servers started showing more timeouts after the upgrade. After investigating the issue we realized that the following pattern combined with tls connection is the culprit:

    resp, err := client.Get(url)
    if err != nil {
        panic(err)
    }
    defer resp.Body.Close()
    m := map[string]string{}
    err = json.NewDecoder(resp.Body).Decode(&m)
    if err != nil {
        panic(err)
    }

The reason for this change in behavior between go1.7.4 and go1.8.1 is the fix for issue#17355. The new behavior of the chunkedReader is causing the above code to not read the entire body, i.e. it falls short of reading the last zero length chunk. This causes the connection not to be reused on subsequent requests.

I created a small demo app. note the demo app cannot be run on the playground since it listens and makes network connections.

I honestly don’t know what’s the right way to fix this issue. One one hand it seems like the reader and json decoder are following the api contract, i.e. it’s not really a bug. On the other hand, I think that many go users use the above pattern which causes very unexpected behavior that is hard to debug.

The only way I can think of to fix this is either using ioutil.ReadAll(resp.Body) combined with json.Unmarshal, or use json.NewDecoder() followed by ioutil.ReadAll and discard the result.

What do you all think ?

​

[Ian Davis]

On Tue, 23 May 2017, at 01:33 PM, John Shahid wrote:

The only way I can think of to fix this is either using ioutil.ReadAll(resp.Body) combined with json.Unmarshal, or use json.NewDecoder() followed by ioutil.ReadAll and discard the result.

What do you all think ?

I noticed just yesterday that your first suggestion is used in the Google API client, e.g. when reading an error response:

https://github.com/google/google-api-go-client/blob/master/googleapi/googleapi.go#L114

Ian

[Jim Robinson]

I'd suggest the latter, drain the reader as the last step.  Perhaps define a DrainReadCloser that can fully drain  the input ReadCloser as part of its closing operation?

[Dmitri Shuralyov]

Relevant: https://github.com/google/go-github/pull/317

See the discussion and the CLs to Go std lib that Brad sent out as a result.