Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Do we need to close the response object if an error occurs while calling http.Get(url)?

In the following code is it also necessary to close the response body in the error case:

res, err := http.Get(url)

if err != nil {
    log.Printf("Error: %s\n", err)
}

defer res.Body.Close()
like image 278
Prashant Avatar asked Sep 28 '15 08:09

Prashant


1 Answers

General concept is that when a function (or method) has multi return values one being an error, error should be checked first and only proceed if the error is nil. Functions should return zero values for other (non-error) values if there is an error. If the function behaves differently, it should be documented. http.Get() does not document such deviation.

So it should be handled like this:

res, err := http.Get(url)
if err != nil {
    log.Printf("Error: %s\n", err)
    return
}

defer res.Body.Close()
// Read/work with body

Notes:

As JimB confirms too, if a non-nil error is returned, even if the response is non-nil, we don't have to close it. In case of a redirection error the non-nil response may hold context and further information about where following the redirect failed. See details below:

http.Get() honors the general concept "most of the time": it returns nil response if there is an error:

return nil, someError

However checking client.go, unexported method Client.doFollowingRedirects(), currently line #427:

if redirectFailed {
    // Special case for Go 1 compatibility: return both the response
    // and an error if the CheckRedirect function failed.
    // See https://golang.org/issue/3795
    return resp, urlErr
}

So due to a backward compatibility issue it may return a non-nil response and a non-nil error at the same time, if redirection fails.

On the other hand trying to call resp.Body.Close() if resp is nil will cause a run-time panic.

So if we want to close response body in this case, it could look like this (can only be closed if resp is not nil):

res, err := http.Get(url)
if err != nil {
    log.Printf("Error: %s\n", err)
}
if res != nil {
    defer res.Body.Close()
    // Read/work with body
}

Or:

res, err := http.Get(url)
if err != nil {
    log.Printf("Error: %s\n", err)
}
if res == nil {
    return
}

defer res.Body.Close()
// Read/work with body

The doc of http.Response guarantees that Response.Body will not be nil even if there is no response data:

// The http Client and Transport guarantee that Body is always
// non-nil, even on responses without a body or responses with
// a zero-length body.

But if the error is not nil, you don't have to close the non-nil response body.

like image 148
icza Avatar answered Oct 19 '22 17:10

icza