4 ms·

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

• nkozyra 5y ago
  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.
  • srer 5y ago
    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 `?`.)
    • nkozyra 5y ago
      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.
      • preseinger 5y ago
        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.
    • preseinger 5y ago
      `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.
      • srer 5y ago
        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.
        • preseinger 5y ago
          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.
          • srer 5y ago
            > 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.
            • preseinger 4y ago
  • preseinger 5y ago
    Why do you believe errors are annotated by definition? They aren't? Panic isn't an ersatz error mechanism.
    • nkozyra 5y ago
      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.
      • preseinger 5y ago
        "Annotated" means given `err error`, you return `fmt.Errorf("annotation: %w", err)` — nothing more or less.
  • cube2222 5y ago
    Keep in mind that the usefulness of stack traces quickly breaks down in the presence of goroutines.
• jhoechtl 5y ago
  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.
• foolfoolz 5y ago
  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