Should I error-check Close() on a response body?

Viewed 11978

The docs for net/http have the following example:

resp, err := http.Get("http://example.com/")
if err != nil {
    panic(err)
}
defer resp.Body.Close()
body, err := ioutil.ReadAll(resp.Body)
fmt.Printf("%s", body)

Close returns an error, but it is not checked. Is there something I'm missing here? The importance of checking every error is frequently emphasized in go, but I see this defer resp.Body.Close() pattern a lot with no error checks.

4 Answers

There are two things to consider: What would you do with it if you checked it and there was an error? And, what would the side-effects be if there was an error?

In most cases, for closing a response body, the answer to both questions is... absolutely nothing. If there's nothing you'd do if there was an error and the error has no appreciable impact, there's no reason to check it.

Also note that Close() returns an error in order to fulfill the io.Closer interface; the function doesn't necessarily return an error. You'd need to check the source to know for sure if it has an error case.

This is a downside of using defer

It would be advised, as a responsible developer, that you check for all possible error prone points, and handle them as gracefully as you can.

Here are some of the options you can choose in handling this situation:

Option #1

Do not use defer, instead manually call close once you're done with the response's body and simply check for errors then.

Option #2

Create an anonymous function that wraps the closing and error checking code. Something like this:

defer func() {
    err := resp.Body.Close()
    if err != nil {
        log.Fatal(err)
    }
}()

Avoid using panics in your programs. Try to handle the errors gracefully by doing something or at least logging the error.


Additional information can be found here.

To add to @Mihailo option #2, call it option #2.1 Define function dclose() like so:

func dclose(c io.Closer) {
    if err := c.Close(); err != nil {
        log.Fatal(err)
    }
}

use like so:

defer dclose(resp.Body)

Also in your code the check for err!=nil can declare:

func errcheck(err error) {
    if err != nil {
        log.Fatal(err)
    }
}

then use:

errcheck(err)

then your code becomes:

resp, err := http.Get("http://example.com/")
errcheck(err)
defer dclose(resp.Body)

body, err := ioutil.ReadAll(resp.Body)
errcheck(err)

fmt.Printf("%s", string(body))

IMHO a little cleaner perhaps? But I'll wait for the go aficionado's to correct me and highlight drawbacks.

EDIT

Thanks! @RayfenWindspear

Do replace log.Fatal(err) with log.Println(err) to avoid unnecessary panic.

EDIT2

dclose() to avoid confusion with go close()

Have fun!

From what I could gather, net/http can't err. However, I would rather assume that .Close() can err for any implementation of io.Closer than to study the internals.

Below is an example using named returns where defer only sets the returned error if the error would otherwise be nil:

func printResponse(url string) (retErr error) {
    resp, err := http.Get(url)
    if err != nil {
        return err
    }
    defer func() {
        err := resp.Body.Close()
        if err != nil && retErr == nil {
            retErr = err
        }
    }()
    body, err := ioutil.ReadAll(resp.Body)
    if err != nil {
        return err
    }
    fmt.Printf("%s", body)
    return nil
}
Related