Large response payload cannot be fully read due to context cancelled

[jkosecki]

Hi,

I've noticed that for some larger payloads in the response (in my case it was just an array of 40 objects or so) I was getting context cancelled error when reading the response body. After digging a bit more into the code and reading the docs of net/http I' found that it was caused by use of timed context within your sendReq method.

reqCtx, cancel := context.WithTimeout(ctx, c.timeout)
defer cancel()

Following the docs of NewRequestWithContext:

For an outgoing client request, the context controls the entire lifetime of a request and its response: obtaining a connection, sending the request, and reading the response headers and body.

So what happens here is that once a request is made, you return Response object that might be still buffering response payload. However, cancel() happens around the same time, which, in case of larger payloads that are not fully buffered before sendReq's defer is called, results in partial response body and error returned when calling response.Body()

One thing would be to pass ctxCancel function to the response object returned and call cancel as part of reading the response payload - however, docs of net/http.Client says:

// Timeout specifies a time limit for requests made by this
// Client. The timeout includes connection time, any
// redirects, and reading the response body. The timer remains
// running after Get, Head, Post, or Do return and will
// interrupt reading of the Response.Body.
suggest that timeout set when the client is created does most likely exactly the same what your reqCtx, cancel := context.WithTimeout(ctx, c.timeout) does. So keeping it in sendReq is unnecessary and one can rely on the internal implementation of the client.timeout

[bozd4g]

Hey @jkosecki ,

I'm using default timeout as 10 seconds. But still you can change the timeout with option functions if you need buffer big payloads from a server. An example is here;

import ghc "github.com/bozd4g/go-http-client"

opts := []ghc.ClientOption{
	ghc.WithTimeout(time.Second * 3), // your timeout
}

client := ghc.New("https://jsonplaceholder.typicode.com", opts...)
client.Get(...)

Also I saw some discussions on the stackoverflow about this issue and context canceling way is more preferable. Therefore I'll remove the timeout from http client asap.

Thanks for reaching out to me.

[jkosecki]

Hi,

the first thing I did when I encountered issue with reading response body was to increase the timeout to 1 hour - it didn't help though.

Basically, what happens here is a race-condition between buffering response payload and calling deferred cancel function. As far as I understand, a call c.httpClient.Do blocks only partially - it waits for response status etc. but it buffers the payload asynchronously. So it's possible that cancel is called before a full response body is loaded. And if that happens, calling response.Body returns context canceled error

[bozd4g]

Hey @jkosecki ,

I solved the problem on #5 PR and also created an integration test to cover your use-case.

Also I created a new release and you can use it 🎉

Also thanks for sharing and clarifying the issue.