Skip to content

Instantly share code, notes, and snippets.

@deanveloper
Created September 3, 2018 07:34
Show Gist options
  • Save deanveloper/61544f16a7d4c3d517bda10c08080270 to your computer and use it in GitHub Desktop.
Save deanveloper/61544f16a7d4c3d517bda10c08080270 to your computer and use it in GitHub Desktop.

Go 2 Error Values

My main critique with Error Values is the Formatter interface.

I fear that nearly every format function will be written in the exact same fashion:

func (e *MyErr) Format(p errors.Printer) (next error) {
    p.Print(/* short error string */)
    if p.Detail() {
        p.Print(/* details */)
        p.Print(/* details */)
        p.Print(/* details */)
    }
    return /* caused by */
}

But we already have something to replace short error string with, and something to replace caused by with. The only thing that isn't provided is the details.

Instead of a Formatter, we should have a Detailer, and let the library printing the error handle the rest.

Detailer could look something like...

type Detailer interface {
    Detail() []string
}

Any advantage provided by using p.Printf can be equally achieved using fmt.Sprintf.

A Formatter was wanted to allow for localization. This is still allowed under using Detailer instead, as localization directives may be placed in in the string slice if you chose to do so, just as you got to choose if you wanted to put localization directives in your p.Print statements.

Then, the library printing the error (fmt, golang.org/x/text/message, etc) may format as needed. For the same effect as in the overview.

This is of course simplified quite a lot, I'm writing this at 3:30am and don't want to be thinking too hard!

func printErr(err error) {
    fmt.Println("--- %s", err.Error())

    if det, ok := err.(errors.Detailer); ok {
        for _, line := range det.Detail() {
            fmt.Println("    " + line)
        }
    }

    if wrap, ok := err.(errors.Wrapper); ok {
        causedBy := wrap.Unwrap()
        if causedBy != nil {
            printErr(causedBy)
        }
    }
}

Thank you for reading, I hope you consider this.

@deanveloper
Copy link
Author

deanveloper commented Sep 4, 2018

Some more remarks:

errors.Formatter is bad because this has more chance for programmer error. If the programmer forgets to print Error(), or return the next error, it an extremely negative impact on anything trying to print it. It leaves for too much human error.

Using Detailer instead of Formatter also allows for much more parsable errors. For instance, if I wanted JSON for my error, I can now very easily parse my error to:

{
    "error": /* err.Error() */
    "details": /* err.Details() (if implementing) */
    "wraps": /* err.Unwrap() (if implementing) */
}

To do this with Formatter, you would need to create a custom errors.Printer and assume what is error vs what is details, because it would not be clear as to what's what.

@mpvl
Copy link

mpvl commented Sep 25, 2018

A few comments: wrap is not the same as next. Having a separate "unwrap" and "next-in-chain" method, as, for instance used in errgo, has been proven to confusing in practice. Dave Cheney's pkg/errors doesn't make this distinction mostly because of this confusion. Reporting the next in chain in a way closely coupled to formatting makes it clear which is used for which, solving this issue. Your suggestion discards this distinction again.

I'm not sure how you envision localization can work in your API. In the Formatter API, the Printer will contain the necessary context to localize a string properly. There is no localization information passed to the Detail method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment