Hacker News new | ask | show | jobs
by sagichmal 2528 days ago
One foundational principle of Go is that the sad path is at least as important, and maybe more important, than the happy path. The best Go programmers I know write the sad path of their programs first, and then backfill the happy-path logic. So:

> you only need to write [error checking] when you actually have something meaningful to do.

Although it's the subject of a lot of ridicule, `if err != nil { return err }` is actually bad Go code, and not often written by good Go programmers. Errors in Go are, at a minimum, annotated with contextual information before being returned. Frequently, they are programmed-with in other, more sophisticated ways, depending on the domain of the program.

Shifting your mindset to understand errors as something significantly more important than the off-gassing of your program's execution is worthwhile in general, and, in Go, fundamental.

3 comments

The fact that you need to add context to errors usually exacerbates the problem and makes it even harder to read the code. You often end up with

  err = doThing()
  if err! = nil {
    return errors.New("Error doing thing", err)
  }
This doesn't add any useful information for whoever is reading the code, it's just boilerplate that you learn to skip while reviewing, while hopefully not missing any important thing that does happen on the error path.

As for 'programming with the error', I would like to see an actual use case for constantly doing this, and why exceptions would prevent that pattern. The only one I can remember is something highlighted as a 'good practice' by Rob himself: write everything you want to a bufio.Writer, without checking the error messages, and then calling Flush and only then checking if maybe something failed. If this is good, safe, sad-path-first, errors-are-values style... then my taste in programming is obviously bad. Obviously, the same could be achieved with exceptions.

First, the signature for `errors.New` is `New(text string) error`. It won't take more parameters than that. So I guess you mean `fmt.Errorf`.

If above is true, then how about:

    err := renderTemplate()

    if err! = nil {
        return fmt.Errorf("Error rendering template: %s", err)
    }
The end error could then be something for example:

    Error rendering template: Compiler has failed: Cannot load template: File /tmp/test.tpl was not found
    ------------------------  -------------------  --------------------  --------------------------------
     |                         |                    |                     |
    Returned by that           |                    |                     |
    example                   Returned by the       |                     |
                              fictional compiler   Returned by the        |
                                                   fictional template    Returned by the fictional file 
                                                   Loader                reader
I didn't even twist your example, and yet you can already see more information. And with that information, even a user can understand what's going on clearly. So ... more useful?
Oops, I forgot if errors.New takes the 'cause' as well.

Regarding you example: the code itself still contains redundant information for someone reading it. True, the error ends up being nicer, though I would argue that the user would have been better served with a simple 'failed to load template file: /tmp/test.tpl', no need to show the pseudo call stack (so, only the fictional template loader should have been wrapping the error,for this particular case). And for a developer, the full call stack may be more useful. Exceptions would give you both for free - a nice message that can be shared to the user by whoever caused the most understandable error, and a call stack that can be logged at the upper layer so developers can see it if a bug is logged, and get a much fuller context.

The full call stack is available in Go but it is up to the developer if they want to include it or not which they can do by creating a custom error type and implementing that to be part of their type. And having the choice seems like a benefit to me.
I would argue that if you are using boilerplate annotations, you are doing it wrong. If you really do not need to add context, don't add context. But in my code I find that I want to add context about 90% of the time.

But then I am super zealous about making sure my error messages understandable without the need to track down other information. For example, I want to know that (something like) "the config file needs group read permissions" not "file cannot be opened." But maybe others value ease of programming more than I do and are less concerned about error UX than I am?

> `if err != nil { return err }` is actually bad Go code, and not often written by good Go programmers.

Like the people who wrote the Go stdlib? Because that's filled with those - just look at the net/* packages.

One distinction that is often made by core team members is that you only need to annotate errors at package boundaries, and that returning unannotated errors within a package is fine. But in most code, the packages are not so well-defined, or well-thought-out, or sacrosanct, that they represent a good proxy for annotation decisions.

I would much rather have an error with too much or duplicate annotation than one with not enough. And I would further argue that, yes, in the stdlib, errors are generally under-annotated.

Legitimate question: then what does good Go code that is written by good Go programmers do/look like? Wrap `err` in `errors.New("some function failed", err)`?
At a minimum, an error should be logged with appropriate context and execution allowed to continue; or annotated and returned. Error annotation is currently best achieved with pkg/errors as e.g. `errors.Wrap(err, "error doing thing")`. (The xerrors suggestion of `fmt.Errorf("error doing thing: %w", err)` is awkward and hacky.)

Many programs can benefit from a more structured approach to error management. But once you get past the minimum (above) there's no one-size solution for what "a more structured approach" looks like. I really enjoy how upspin.io does their errors package, though it is somewhat esoteric. I'm also reading and generally liking how Cockroach does things, though I don't like the coupling to Protobufs.

Recently the Go team introduced the xerrors package for improved error management: https://godoc.org/golang.org/x/xerrors

So using these functions is becoming part of what good Go programmers do.

That statement is still correct even in the context of the stdlib.
> Errors in Go are, at a minimum, annotated with contextual information before being returned.

What surprised me when I last wrote Go was that there was no out-of-the-box solution to adding a stack trace to the error.

Stack traces are, for me, too much noise, and not enough signal. I prefer reading annotations added (prefixed) by programmers deliberately. File and line information for the call stack leading to the error maybe provide value in the dev cycle (e.g. when fixing tests) but basically don't in logs in production.

This is all to say: I can understand why they aren't more naturally part of errors, and I think it also helps explain why they are part of panics.

And with all that said, I wouldn't object to making stack traces easier to add to errors, as long as it was opt-in.

Does anyone know if this was a conscious decision? I mean IIRC in Java you're generally discouraged from throwing errors for control flow because creating the stack trace is a relatively heavy process. In Go this is of less concern and returning an error is pretty normal for control flow (as in errors are expected, not exceptional), and you shouldn't have to worry that an error path would be 100x as expensive as a normal flow because a stack trace is being generated.
Java allows since ~a decade time to omit the generation of stacktraces, for exactly such cases
yeah; we're getting there though, see https://github.com/golang/go/wiki/ErrorValueFAQ
JMTCW, but since I generally program Go in GoLand with the debugger, I have full access to the stack all the time so I have not found this to be a major concern. But YMMV.