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

Add Go 1.20 errors.Join support #106

Closed
wants to merge 1 commit into from

Conversation

StevenACoffman
Copy link
Contributor

@StevenACoffman StevenACoffman commented Mar 12, 2023

(Edit knz) Informs #99 and cockroachdb/cockroach#97799

(Note: the work in this PR is important but not sufficient to handle the Go 1.20 changes - we need to extend the other components to properly support multierrors from other packages in network serialization/deserialization)

All errors using cockroachdb/errors that are combined using Go 1.20 std lib errors.Join will lose the stacktrace when you fmt.Printf("%+v\n", err) on the joined error. Also, without native errors.Join support, this library is not a drop in replacement for the Go 1.20 stdlib errors package.

This PR adds errors.Join from Go 1.20 stdlib, but with a Format() method so joined errors honor the format verbs. Joined errors are printed verbosely indexed in square brackets like [0], to differentiate them from the parenthetical indexing (0) of wrapped errors.

[0]:
  (1) Another bad thing happened: Something went wrong
    -- Stack trace:
    | [...repeated from below...]
  Wraps: (2) Another bad thing happened: Something went wrong
  Wraps: (3) Something went wrong
    -- Stack trace:main.foo
    | 	_example/main.go:22
    | main.bar
    | 	_example/main.go:26
    | main.main
    | 	_example/main.go:36
    | runtime.main
    | 	/Users/steve/.asdf/installs/golang/1.20.2/go/src/runtime/proc.go:250
  Wraps: (4) Something went wrong
  Error types: (1) *errors.withStack (2) *errors.wrapper (3) *errors.withStack (4) main.ErrMyError
    -- Stack trace:main.main
    | 	_example/main.go:38
[1]:
  (1) Yet another bad thing happened: Something went wrong
    -- Stack trace:
    | [...repeated from below...]
  Wraps: (2) Yet another bad thing happened: Something went wrong
  Wraps: (3) Something went wrong
    -- Stack trace:main.foo
    | 	_example/main.go:22
    | main.baz
    | 	_example/main.go:31
    | main.main
    | 	_example/main.go:40
    | runtime.main
    | 	/Users/steve/.asdf/installs/golang/1.20.2/go/src/runtime/proc.go:250
  Wraps: (4) Something went wrong
  Error types: (1) *errors.withStack (2) *errors.wrapper (3) *errors.withStack (4) main.ErrMyError
    -- Stack trace:main.main
    | 	_example/main.go:40

This PR is not meant to be merged as is, and is more for discussion (and is hopefully helpful rather than just annoying 😬 ).

It currently leans on Go 1.20 std lib pkg fmt's new func FormatString(State, int32) string fmt: add FormatString(State) string #51668, rather than using the cockroachdb/errors internal format forwarding. As a result, this current implementation depends on Go 1.20+.

It is likely that backwards compatibility with pre-Go 1.20 is desirable, so this Format() would need to be reworked to use this library's errbase.Formatter and errbase.SafeFormatter to avoid that.


This change is Reviewable

Signed-off-by: Steve Coffman <steve@khanacademy.org>
@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Mar 12, 2023

CLA assistant check
All committers have signed the CLA.

@StevenACoffman
Copy link
Contributor Author

StevenACoffman commented Mar 12, 2023

This is an example program which generates the example output above to play with how to format output when errors.Join combines with errors.With and errors.WithStack:


// ErrMyError is an error that can be returned from a public API.
type ErrMyError struct {
	Msg string
}

func (e ErrMyError) Error() string {
	return e.Msg
}

func foo() error {
	// Attach stack trace to the sentinel error.
	return errors.WithStack(ErrMyError{Msg: "Something went wrong"})
}

func bar() error {
	withErr := errors.With(foo(), errors.New("Another bad thing happened"))
	return errors.WithStack(withErr)
}

func baz() error {
	withErr := errors.With(foo(), errors.New("Yet another bad thing happened"))
	return errors.WithStack(withErr)
}

func main() {
	myErr := bar()
	fmt.Println("Doing something")
	err := errors.WithStack(myErr)

	myOtherErr := errors.WithStack(baz())

	joinErr := errors.Join(err, myOtherErr)
	fmt.Println("\n\n---withStackBar")
	fmt.Printf("%+v\n", err)
	fmt.Println("\n\n---baz")
	fmt.Printf("%+v\n", myOtherErr)
	fmt.Println("\n\n---joinErr")
	fmt.Printf("%+v\n", joinErr)
}

@healthy-pod healthy-pod requested review from knz and ajwerner March 14, 2023 17:44
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the initial pass. We'll want to extend this PR to properly support the detailed error formatting and the generation of sentry reports.

@lexfrei
Copy link

lexfrei commented Jun 13, 2023

@knz any news on this?

@knz
Copy link
Contributor

knz commented Jun 13, 2023

In progress.

@dhartunian
Copy link
Contributor

@StevenACoffman I incorporated your changes in separate branch PRs where I was compiling 1.20 compatibility work. Closing this PR since it's now merged. Thanks for the contribution. New releases will publish shortly with 1.20 support.

@dhartunian dhartunian closed this Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants