profile picture

Error handling in defer

October 07, 2023 - go programming

I see periodically people handling errors in deferred calls like this:

func Do() (r Result, err error) {
    // ...
    defer func() {
        err = multierr.Append(err, f.Close())
    }()
    // ...
}

or another variant:

func Do() (err error) {
    // ...
    defer func() {
        err1 = f.Close()
        if err == nil && err1 != nil {
            err = err1
        }
    }()
    // ...
}

In my opinion this is ugly and unnecessary. First, I don't like named returns, matter of taste perhaps, but I find it more readable when return statements list the values being returned. Second, the purpose of the deferred statement is to clean up resources, I feel stongly against using it to modify the return value in any way. That means that I never defer a function that might return an error unless I can safely ignore that error. If we take closing files as an example, then there are two possibilities: files openned for reading and for writing.

If I open a file for reading, then I defer Close() and that's it, there's no issue with ignoring the error that it returns. As an example you can have a look at the implementation of os.ReadFile function. It opens the file, defers f.Close(), then reads data from the file and appends it to []byte buffer. If it reads the whole file successfully, then by the time it calls f.Close() it has all the data it needed and it doesn't really matter if f.Close() fails, user may have pulled the USB stick out beteween the moment our code read the last byte of the file and the moment it tried to close the file, but we do not need to do anything about it, it doesn't affect the result. If on the other hand user pulled that USB stick out before we finished reading the file, then f.Read would fail and return an error, at this point we return that error from the function; deferred f.Close would likely fail as well, likely with exactly the same error, but do we need to return that error to the caller? I don't see it adding any useful information to the initial error from f.Read.

If on the other hand the file is openned for writing, then we can't be sure that we have successfully written all the data until we called Close() on the file and it returned success. In this case I would not defer this final call to Close(). Good example can again be found in stdlib: os.WriteFile. Note again, that there's no need to combine errors returned by f.Write() and f.Close(), if f.Write() returned an error then the error from f.Close() can be just dropped, it really doesn't add anything to the error we already have. If the function logic more complicated, I see no problem in deferring call to Close as long as I still call it explicitely on the success path, something like this:

func Do(path string) error {
    f, err := os.Create(path)
    if err != nil {
        return err
    }
    defer f.Close()
    for i := 0; i < 10; i++ {
        if _, err := f.Write([]byte{i}); err != nil {
            // deferred f.Close will be executed and the error it returns will
            // be ignored, but we can ignore that error because we already know
            // that the file hasn't been written correctly and is corrupted
            return err
        }
    }
    // all writes have succeeded, so we call f.Close and if it fails then we
    // return the error to the caller, if it succeeds, then we still call
    // deferred f.Close which returns an error which we ignore
    return f.Close()
}