On: Don't defer Close() on writable files (2017)

[nivyed]

The article suggests using a named return value `err` to allow the return value of `Close` to be be propagated - unless doing so would overwrite an earlier error:

    defer func() {
        cerr := f.Close()
        if err == nil {
            err = cerr
        }
    }()

Wouldn't it be better to use `errors.Join` in this scenario? Then if both `err` and `cerr` are non-nil, the function will return both errors (and if both are `nil`, it will return `nil`):

    defer func() {
        cerr := f.Close()
        err = errors.Join(err, cerr)
    }()

[joeshaw → nivyed]

OP here. Another commenter pointed out that `errors.Join` didn't exist when I wrote this, but I wouldn't have changed my guidance if it had.

The core issue here is that you want to deal with errors as soon as possible. The async nature of writes to files makes this more challenging. Part of the purpose of this post was to inform people that you don't necessarily always see write errors at write-time, sometimes you see them at close-time and should handle them there. (And sometimes you don't see them at all without fsync.)

Even if you do handle errors in your deferred close, you may be taking other actions that you'd rather not have until the file i/o is completed, and this could leave your program in an inconsistent state. Side effects in Go are common, so this is a practical concern and it is hard to spot and debug.

[jfindley → nivyed]

This is from 2017, errors.Join did not exist at the time. But yes, today you'd do it differently.

[admax88qqq → nivyed]

Go error handling is brutal.

I miss exceptions

[Cthulhu_ → admax88qqq]

Exceptions aren't exceptional though; they are too expensive for not-exceptional errors, like failing writes.

That said, a language feature where you can throw lightweight error values without generating a stack trace etc might be a middle ground. But it won't land in Go, given the discussion about alternative error handling some years ago.

Anyway, in practice it's not that bad. A write can go wrong, you as a developer should write code that handles that situation. Exceptions lead a developer to miss errors, or to not handle them in a finegrained manner - e.g. generic "catch all, log error, maybe" style error handling.

[admax88qqq → Cthulhu_]

Exceptions are slow in some languages based upon how they are implemented. I'm not convinced that is fundamental to exceptions and rather a choice of how they were implemented. In Java exceptions arn't actually that slow, most of the cost is just allocating the exception object (and allocations in Java are fast).

> Exceptions lead a developer to miss errors, or to not handle them in a finegrained manner - e.g. generic "catch all, log error, maybe" style error handling.

I don't see how Go error handling makes people handle things any more explicitly than exceptions. Most people just `if err != nil { return err }`, which to be honest is the _correct_ logic in many cases, and it's pretty easy to forget to check if err and keep on trucking. At least with exceptions if you don't catch it your thread terminates making unhandled exceptions

Exception bubbling means its easier to catch the error at the level that makes sense, and because they are real objects type checking is easier as opposed to the performance of `errors.Is()` which is surprisingly slow.

[randomdata → admax88qqq]

> which to be honest is the _correct_ logic in many cases

It is almost never the correct logic. The only time it might be appropriate is in a private helper function that has limited scope around another function.

It is most definitely not the correct logic if you are returning that from a public function! For many reasons, but especially because it now binds you to the implementation of the function you called forevermore. That is a horrible place to be.

For example, find out your os.File usage would be better served by SQLite? Too bad. You can't change it now because the users of your function have come to rely on errors from the os file operations when they handle the error you give them. Their code can't deal with the errors coming out of SQLite.

Instead, you need to return errors that are relevant to your function. It may be appropriate to wrap the source error in some circumstances, but your error structures should compel the user to rely on your errors, leaving the wrapped error only for things like logging where a change in the future won't break the callers.

[Matl → admax88qqq]

There was [1].

1 - https://go.googlesource.com/proposal/+/master/design/go2draf...

[throwaway173920 → nivyed]

IMO the formatting of the error string returned by errors.Join is atrociously opinionated and not very logging-friendly - it adds a newline between each error message. I know I'm not the only one that has this opinion

[kjksf → throwaway173920]

It's a trivial function: https://cs.opensource.google/go/go/+/refs/tags/go1.23.1:src/...

You could write your own errorsJoin() and change Error() method to suit your needs.

But really in this particular scenario you would be better served by something like:

    func errorsConcat(err1 error, err2 error) error {
      if (err1) {
        return err1;
      }
      return err2;
    }

And then do: err = errorsConcat(err, f.Close())

In the scenario described in this article, errors.Join() would most often reduce to that (in terms of what Error() string would produce).

[Matl → throwaway173920]

Wouldn't you use slog if you want logging friendly?