Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Best practice error handling infrastructure moving forward #2181

Open
davecgh opened this issue May 1, 2020 · 0 comments
Open

Best practice error handling infrastructure moving forward #2181

davecgh opened this issue May 1, 2020 · 0 comments
Labels
best practices Discussion and details regarding best practices

Comments

@davecgh
Copy link
Member

davecgh commented May 1, 2020

I'm opening this issue to document the results of discussions regarding error handling best practices and the direction we would like to move in when adding new error infrastructure and also for updating the existing infrastructure over time. Note that since the changes involve modifications to the public API of the various packages in ways that are not backwards compatible, updates to existing packages will necessarily come with major module version bumps and therefore they should be done when already bumping a module to a new major version for other reasons as well.

Background

A couple of the primary goals of error handling in the dcrd code base is to both provide detailed human-readable errors as well as a way to unambiguously programmatically distinguish the errors without relying on brittle techniques such as looking for specific error strings.

The way that currently is typically achieved is by providing error types that include fields for a detailed description as well as a specific error code of type ErrorCode int along with exported constants for the various error codes. This approach allows errors to display detailed human-readable errors while simultaneously allowing callers to programmatically type assert the error to the appropriate upper level type and then access the error code field to compare against the exported error code constants from the package.

In code terms, the current approach looks like the following:

package main

import (
	"fmt"
)

type ErrorCode int

const (
	ErrFoo ErrorCode = iota
	ErrBar
)

type Error struct {
	Description string
	Code        ErrorCode
}

func (e Error) Error() string { return e.Description }

func main() {
	var err1 error = Error{Description: "Some human-readable error", Code: ErrFoo}
	if e, ok := err1.(Error); ok && e.Code == ErrFoo {
		fmt.Println("Special handling for ErrFoo")
	} else if e, ok := err1.(Error); ok && e.Code == ErrBar {
		fmt.Println("Special handling for ErrBar")
	}
	fmt.Println(err1)

	var err2 error = Error{Description: "Some other human-readable error", Code: ErrBar}
	if e, ok := err2.(Error); ok && e.Code == ErrFoo {
		fmt.Println("Special handling for ErrFoo")
	} else if e, ok := err2.(Error); ok && e.Code == ErrBar {
		fmt.Println("Special handling for ErrBar")
	}
	fmt.Println(err2)
}

Which results in the output:

Special handling for ErrFoo
Some human-readable error
Special handling for ErrBar
Some other human-readable error

The current approach has served well and does provide the desired properties, however, it is rather verbose and does not work well with wrapping errors on the way up the stack, which often means errors from lower layers either have to be passed back to the caller unmodified, which loses valuable context, or they have to be converted to the error types of the consumer, which either loses the underlying error code, or forces the consumer to also provide an error code for that underlying condition.

Also, it is often necessary to add additional programmatically detectable errors to existing packages, and since the current approach uses integer error codes in an enum, care must be taken to avoid changing their value to maintain a stable API per semver.

As of Go 1.13, the standard library introduced the errors.Is and errors.As functions specifically to address aforementioned error issues by allowing errors to be wrapped on their way up the stack while still being easily detectable by a caller.

However, using integers to represent error codes in structs that ultimately get wrapped in error stacks is not ideal because the zero value for that error code is indistinguishable from an unset error which requires care to handle properly in many contexts. Instead, it is preferable to provide the programmatically detectable condition via a field of type error and provide an Unwrap method so that it works seamlessly with errors.Is and errors.As because an unset error is then nil.

Best practice

Putting it all together, the following code demonstrates the new preferred approach as of Go 1.13. Notice that it is not substantially different in character, but it does provide an easier to work with infrastructure that is harder to misuse:

package main

import (
	"errors"
	"fmt"
	"io"
)

// ErrorKind identifies a kind of error.  It has full support for errors.Is and
// errors.As, so the caller can directly check against an error kind when
// determining the reason for an error.
type ErrorKind string

const (
	// ErrFoo description here.
	ErrFoo = ErrorKind("ErrFoo")

	// ErrBar description here.
	ErrBar = ErrorKind("ErrBar")
)

// Error satisfies the error interface and prints human-readable errors.
func (e ErrorKind) Error() string {
	return string(e)
}

// Error identifies an error related to <description here>. It has
// full support for errors.Is and errors.As, so the caller can ascertain the
// specific reason for the error by checking the underlying error.
type Error struct {
	Description string
	Err         error
	// Any other desired fields with additional details
}

// Error satisfies the error interface and prints human-readable errors.
func (e Error) Error() string {
	return e.Description
}

// Unwrap returns the underlying wrapped error.
func (e Error) Unwrap() error {
	return e.Err
}

// MakeError creates an Error given a set of arguments.
func MakeError(kind ErrorKind, desc string) Error {
	return Error{Err: kind, Description: desc}
}

func main() {
	var err1 error = MakeError(ErrFoo, "Human-readable error 1")
	var err2 error = MakeError(ErrBar, "Human-readable error 2")
	var err3 error = Error{Err: io.EOF, Description: "Human-readable error 3"}
	var err4 error = ErrBar

	fmt.Printf("is err1 ErrFoo? %v\n", errors.Is(err1, ErrFoo))
	fmt.Printf("is err1 ErrBar? %v\n", errors.Is(err1, ErrBar))
	fmt.Printf("is err2 ErrFoo? %v\n", errors.Is(err2, ErrFoo))
	fmt.Printf("is err2 ErrBar? %v\n", errors.Is(err2, ErrBar))
	fmt.Printf("is err3 ErrFoo? %v\n", errors.Is(err3, ErrFoo))
	fmt.Printf("is err3 ErrBar? %v\n", errors.Is(err3, ErrBar))
	fmt.Printf("is err3 io.EOF? %v\n", errors.Is(err3, io.EOF))
	fmt.Printf("is err4 ErrFoo? %v\n", errors.Is(err4, ErrFoo))
	fmt.Printf("is err4 ErrBar? %v\n", errors.Is(err4, ErrBar))

	fmt.Println("err1:", err1)
	fmt.Println("err2:", err2)
	fmt.Println("err3:", err3)
	fmt.Println("err4:", err4)
}

Output:

is err1 ErrFoo? true
is err1 ErrBar? false
is err2 ErrFoo? false
is err2 ErrBar? true
is err3 ErrFoo? false
is err3 ErrBar? false
is err3 io.EOF? true
is err4 ErrFoo? false
is err4 ErrBar? true
err1: Human-readable error 1
err2: Human-readable error 2
err3: Human-readable error 3
err4: ErrBar

Notice that it has the following properties:

  • Supports rich human-readable error messages with context from every level of the error stack including wrapping errors that are not of the package ErrorKind type
  • Allows programmatically detecting error conditions through an arbitrary error stack depth with automatic unwrapping
  • Uses constant error instances for discriminating the type of error (versus package-scoped variables via errors.New that can be mutated by callers)
  • Will not improperly detect an erroneous error if the wrapped error is the zero value
  • Does not need a map of error code constants to human-readable names to directly print the error conditions
  • Allows adding new error conditions at the location in the source that makes the most sense without issue (versus the enum approach where inserting a new condition would change the value of everything after it)
  • Supports type-assertion like behavior via errors.As with automatic unwrapping
@davecgh davecgh added the best practices Discussion and details regarding best practices label May 1, 2020
@davecgh davecgh changed the title Best practice error handling instracture moving forward Best practice error handling infrastructure moving forward May 3, 2020
dnldd added a commit to dnldd/dcrd that referenced this issue Oct 25, 2021
This updates the wire error types to leverage go 1.13 errors.Is/As functionality as well as confirm to the error infrastructure best
practices outlined in decred#2181.
dnldd added a commit to dnldd/dcrd that referenced this issue Oct 25, 2021
This updates the wire error types to leverage go 1.13 errors.Is/As
functionality as well as confirm to the error infrastructure best
practices outlined in decred#2181.
dnldd added a commit to dnldd/dcrd that referenced this issue Nov 1, 2021
This updates the wire error types to leverage go 1.13 errors.Is/As
functionality as well as confirm to the error infrastructure best
practices outlined in decred#2181.
dnldd added a commit to dnldd/dcrd that referenced this issue Nov 4, 2021
This updates the wire error types to leverage go 1.13 errors.Is/As
functionality as well as confirm to the error infrastructure best
practices outlined in decred#2181.
davecgh pushed a commit that referenced this issue Dec 5, 2021
This updates the wire error types to leverage go 1.13 errors.Is/As
functionality as well as confirm to the error infrastructure best
practices outlined in #2181.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
best practices Discussion and details regarding best practices
Projects
None yet
Development

No branches or pull requests

1 participant