Hacker News new | ask | show | jobs
by natefinch 2535 days ago
Try makes it worse because it is so easy to miss when reading the code. Because it encourages nesting function calls, which is harder for a human to parse than separate statements across lines. Because it means you can exit the current function from the middle of a line of code, and what runs before or doesn't run before is based on order of operations rather than requiring the exit to be a statement on its own line whose order cannot be misunderstood. Because it discourages giving more information with an error, so instead of "failed to open config file: EOF", you just get the EOF.

Go's error handling isn't any more error prone that writing if statements for all the rest of your code. if err != nil is no different than if age < 18. Either one is a branch in the code. I can literally count on one hand the number of times in 6 years of full time go development that I've seen people miss writing the if err != nil.

Being explicit is good. Spreading out the logic is good. Cramming a lot of logic into one line is bad.... and that's the sole purpose of try.

Maybe there's another way to make error handling better in Go. I'm not averse to looking into that. But try wasn't it.

You're talking about writing if err != nil being tedious, but what about matching Results from Rust, isn't that tedious? What about writing proper catch blocks in java or C++ or python, isn't that tedious? It's all just logic.

3 comments

Writing Go professionally for 4 years already and being a Go fanboy since 2009: while endorsing many benefits of "if err" blocks, I do very much have the following issues with them (in no specific order):

- It's hard to spot outliers. This leads to occasional bugs that tend to get easily overlooked in code review. Also, it makes code reading harder when an "if err" block is subtly different. The most common case here being "if err == nil" (sometimes bug, sometimes on purpose) - super hard to notice.

- You say you have seen missed "if err" blocks only a few times. I say that's 100% too many; every one of them in my experience was a subtle bug (possibly comparable to off-by-one errors in C).

- When I need to focus on understanding/analyzing the optimistic path in a fragment of code (always the first thing I do when reading), the everpresent "if err" blocks introduce tiresome visual noise and make the reading/grokking process slower and harder (having to constantly try and mentally filter out some 80% of what my eyes see).

Just curious. Do you have a vision for how Go could change to improve your issues? (One of the key problems is nobody could agree on a better approach...)

Also, what editor/IDE do you use? The reason I ask is because of this: https://youtrack.jetbrains.com/issue/GO-7747

The difference with catch blocks in Java, C++ or Python is that you only need to write them when you actually have something g meaningful to do.

If you only need to propagate the error or cleanup resources then propagate the error, then all you would write is... Nothing. And cleanup+propagation is by far the most common error handling strategy. In Java and Python exceptions even add context for you automatically to help track down what happened.

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.

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.
But the issue with this is that it can be hard to know what all the possible error conditions are, and thus whether you have anything meaningful to do.

Using Rust, which makes errors explicit like Go, has been eye-opening to me. My programs never crash because I've handled every error condition. No effort on my part. No tests needed.

Java also makes you handle every possible error condition, unless of course you chose to use an escape hatch. Rust allows the same.

By the way, Go is much happier to crash than Java - for example, a simple array index out of range will cause a program crash in a typical Go program, where it would only cause a request failure in a typical Java program. Not sure how Rust handles this.

Finally, choose that isn't tested (manually or automatically) is very unlikely to work. Maybe you can guarantee it doesn't crash, which is a much weaker guarantee, but I doubt even fully proven code (like seL4) is all bug-free before ever being run.

Rust's use of Result is very different from try/catch and exceptions in Java, even if you opt-in to checked exceptions. The big difference is ergonomics and what patterns are used in underlying libraries - opting out of the idiomatic way in Rust feels wrong if you try doing it.

Rust handles your out of range scenario the same way Go does.

If any of this matters to you, the good news is that Kotlin's sealed classes (and soon, Java's sealed classes) allow you to easily implement your own Result-like sum type.

So how did Rust handle the case of a function that may return an error or nothing? Can you forget to check the error, or does it force you to explicitly handle it in some way?

I understand how Result forces you to handle the possibility of an error when you want to access the actual return value, but I don't know what happens if you aren't planning on accessing the return.

I'm also curious how Result-based error handling composes. For example, if I want to sort a list of structs where my comparison function may fail, can I use the built-in sort function, and still get any error that may have occurred back?

With (unchecked) exceptions, this is trivially easy - the sort() function doesn't need to be aware of exceptions in order for them to be propagated through it. With checked exceptions in Java, you need to go through a little dance of wrapping and unwrapping, but it can still be done. If I understand correctly, in Haskell this can be done with the Either monad and liftM, though I can't claim to understand the specifics.

Is there a Rust solution?

You get a warning if you don’t check the error.

Yes, the default semantics is that it will stop when the first comparison fails and give you that error. You can write slightly different codebase if you want a list of all errors instead.

Rust will also crash if you use the indexing operator and go out of bounds. For arrays, you can also use the .get(index) method which returns an Option<&T> instead of &T, so it doesn't have to crash. For most things, iterators get used instead of indexing anyway.
> Because it encourages nesting function calls, which is harder for a human to parse than separate statements across lines.

I absolutely agree. Beyond the human parsing aspect it also makes commit changes easier to reason about and review. I want functionality to be limited per-line and view the ability to combine a lot of functionality into one line as a liability more than a benefit.

Go's error handling isn't carefree or hands-off, but that's because error handling is serious. Especially in network code and cryptography.

I thought it could lead to doing method chaining for a fluent like API which I find cleaner than how things work now.
I really dislike method chaining. I'd much rather have 5 lines than 5 chained methods. If that's too much to read, you can always encapsulate it in a well-named function.
But five functions that return a value and an error would each have to run the if err != nil dance whereas with method chaining it's cleaner
not if each one can fail. What if call #1 and call #3 can return the same error.. how does the caller know which one failed? This is the same as wrapping a bunch of calls with a catch (Exception) ... you lose context of what failed and can't behave differently for different failures. All you can do is perform a generic "something went wrong" behavior.
Interestingly you _can_ implement method chaining to require a terminal method call i.e. err := GetFoo().SetBar(1).SetBaz(2).Run() and then each chained method would set an error property in the object and if err!=nil then do nothing but return the object, and then the last method could return the error value.

That said, I am not a huge fan of fluent interfaces. I much prefer passing in a struct as an "args" parameter, in most case (but not all.)

That can easily be done right now with the current way of error handling.
Do you have any examples?