Hacker News Reader

Comment by preseinger | Hacker News Reader

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

View on HN

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.

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

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.

Reply on news.ycombinator.com