Hacker News Reader

Comment by foolfoolz | Hacker News Reader

there’s really no reason this has to be 3 lines

    if err != nil {
        return err 
    }

i’m hoping they find a way to simplify this

View on HN

Returning an unannotated error like this is an antipattern. Every error return should include an annotation via fmt.Errorf.

Why? Errors are annotated by definition. I wouldn't feel better about returning my own string versus the error which already includes a message.

How much additional context needs to be there and how often is this convention used by simply duplicating the message with basically repetitive text?

panic(err) become "Problem with date formatting: invalid date format" or similar

I also think "antipattern" gets thrown around too often. This sounds more like a preference or convention.

I presume as a method of manually constructing backtrace, as the same error may occur in multiple places it's helpful to understand the context.

I too would label it a preference.

To get out of the error handling tedium in our platform I largely opt to panic instead whenever viable, which gives a nice trace for free. (I am but human, and the error handling particularly grates once you have gotten used to just typing `?`.)

I also panic whenever possible from the caller, in which case I get a pretty clear stack trace. I understand you don't always want that but imo I don't have a lot of cases where I'm ultimately throwing my hands up on a non-exception error.

I think V2 errors are taking more of a lead from the pkg/errors error.Wrap approach anyway.

pkg/errors.Wrap is already accomplished in the stdlib via fmt.Errorf("annotation: %w", err).

Programs which use `panic` as ersatz error mechanisms are fundamentally broken. `panic` expresses an invariant violation that's much different than normal errors.

`panic` is not equivalent to `return err`. It expresses a much more fundamental problem than an error return, and subverts the ability of the reader to model execution control flow. `panic` should essentially never be used in application code, and when it is used it should almost always immediately terminate the program.

Programming is a vast field, "essentially never" is quite a strong statement.

For many errors, in many situations, terminating the process is quite reasonable.

In my particular situation, the greater system will restart failed processes, and retry failed tasks. I find this useful as in many cases my program can just die when something weird happens, simplifying it's own logic.

This represents a false economy, or maybe a local optimum. It's lovely that your code can be simple in the sense that it can assume all kinds of invariants that, if violated, will simply terminate the execution, which can safely be assumed to start up again anew. But it's decidedly not lovely that you can no longer predict what effect an input will have on your code, and can't effectively reason about, well, anything beyond a trivial lifetime/callstack. If your process dies whenever something weird happens, it effectively becomes nondeterministic -- your greater system model has to assume it can die at any instant for any reason.

> your greater system model has to assume it can die at any instant for any reason

Correct. This is something I have to design for in the system anyway, because in practice anything can (and does!) die at unpredictable times. It's typically an inevitable fact of life that a machine/kernel/program will occasionally die, and your system has to survive that.

Of course it can, but the question is what this sort of termination represents. Hopefully, it represents a serious showstopper bug that gets fixed immediately! If your program is built such that call stacks don't have reasonably deterministic behavior, it's essentially impossible to build a usable model of the program as a maintainer.

I mostly agree, but I’m also comfortable using “panic” in an HTTP server to throw 500’s.

Goodness, I hope not! This breaks all your downstream middleware and whatever business logic is invoked by your handlers. If you encounter a 500-worthy problem it's an `error` return like anything else.

`panic` is not for business logic.

> `panic` is not for business logic.

Neither are 500 responses. A perfect HTTP server never responds 500 and there aren't any situations I've ever encountered where there's any valuable error recovery or business logic left to be done once the server has run into a 500-worthy issue.

IIRC, the HTTP server in the Go standard library recovers panics and issues 500 responses, which is what I would expect it to do.

500 maps to a set of business logic errors, sure. Why not?

Why do you believe errors are annotated by definition? They aren't?

Panic isn't an ersatz error mechanism.

Pedantry on my part, but annotated does not mean including a stack trace, simply that it includes a description of the error. Given it's a non-nil check, you never gave a guarantee it's valuable, but the error is itself at minimum an annotation.

panic brings important context, but you're right in that it's not an annotation in itself. It's a program flow mechanism, but I'd argue it's very often utilized as an error flow one.

My larger point was that this still doesn't feel like an "antipattern" and I see that word thrown around enough as a conversation stopper that I've become pretty cynical about it.

"Annotated" means given `err error`, you return `fmt.Errorf("annotation: %w", err)` — nothing more or less.

Keep in mind that the usefulness of stack traces quickly breaks down in the presence of goroutines.

that doesn't add much value but make it more easy to identify the offending place where the error occurred. What would be great is a unified way to add context in a standard and automated manner, like a stack trace.

you only need to wrap errors that you did not raise yourself. if your codebase already annotates an error you hopefully are annotating well enough that it’s identifiable. library code you didn’t write is unknown and needs help. wrapping every error is even more verbose and repetitive

No idea what people have against newline characters, but `if err != nil { return err }` is valid Go code.

go fmt will change that, and code should be uniformly formatted.

Go proverbs: "Gofmt's style is no one's favorite, yet gofmt is everyone's favorite."

agreed, but then what’s the issue?

I struggled with this a lot. Drove me nuts we couldn't get decent code coverage because of error handling for errors I don't even know how to replicate in the first place.

Plus once your code throws one error, every bit of calling code also needs to handle that error. The problem cascades through a codebase quickly. Seemed like a huge violation of DRY principles.

Rob Pike (my understanding as one of the main guys who created the language) has actually addressed this, it's a good read:

https://go.dev/blog/errors-are-values

Tldr refactor your error handling to treat it like code. DRY and SOLID principles would apply and etc. Article makes example of handle your errors in one place rather than 20 by using no-ops on remaining operations after an error occurs.

I don't actually agree with the choice, as it takes one key library which throws errors at every call (like I'm dealing with now) for this to just become a huge pain to do. I had to completely change business logic to implement his suggestion, which isn't always viable (and I'm subsequently finding that out that that wasn't completely viable for us first hand now). Also a lot more boilerplatey type no value add type code needs to be written.

I much prefer unchecked exceptions for the most part, but at least I can understand WHY error handling is the way it is in Go.

Approximately 0% of code bases do this in golang practice. I would be floored if someone provided an example of a large project that does.

In Rust, that could be a Result::unwrap() or propagated using the ? operator.

Explicit is better than implicit

I think that was the comment about Java checked exception, it's one line and it's explicit.

Though personally I feel in this case defaulting to rethrowing uncaught errors is better. Since that's the 99% case. I'd rather it be zero line.

Unless something has changed, Java checked exceptions aren't explicit--it's not generally not possible to tell whether a given function call can raise an exception without inspecting the signature of that function.

> Though personally I feel in this case defaulting to rethrowing uncaught errors is better. Since that's the 99% case. I'd rather it be zero line.

This is ambiguous with the "doesn't error at all" case. If you're looking at source code `foo()` you can't tell whether that's equivalent to `if err := foo(); err != nil { return err }` or just `foo()`. You have to check the function signature to see what the return arguments are (or in Java's case, whether or not it throws).

The error being part of the method signature seems very explicit, no? You will get a compiler error if you don’t handle it in some way in the client code.

Unchecked exceptions are not explicit.

I suppose it becomes part of the signature of the caller, so maybe? I guess I was thinking "locally at the call site, I don't see anything corresponding to `if err != nil { return err }`", but implicit vs explicit error handling probably isn't well-defined, so I suppose it's up to interpretation. I usually think of it more in a local context ("is it evident right at the call site") but the caller's function signature is certainly local-ish. (shrug)

At the call site you will get a compile error. And the signature of the method you call explicitly tells you what errors you need to handle.

Most IDEs will conveniently put a red squiggle line beneath the exact call site as well, and show you the compile error when you hover over it.

And if you choose to rethrow it, you will need to add to your method an explicit annotation.

Verbose and explicit are two separate things.

This is explicit and verbose. Explicit does not need to be verbose.

Reply on news.ycombinator.com