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

proposal: Go 2: adopt Cause and Wrap from github.com/pkg/errors #25675

Closed
VojtechVitek opened this issue May 31, 2018 · 13 comments
Closed

proposal: Go 2: adopt Cause and Wrap from github.com/pkg/errors #25675

VojtechVitek opened this issue May 31, 2018 · 13 comments
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge Proposal v2 A language change or incompatible library change
Milestone

Comments

@VojtechVitek
Copy link

Is there anyone else who'd like to see https://github.com/pkg/errors (or similar) being adopted in Go 2.0?

Forked from #21161 (comment).

Example:

func Chdir(dir string) error {
	if e := syscall.Chdir(dir); e != nil {
		return errors.Wrap(e, "failed to change directory")
	}
	return nil
}

Alternative one-liner example, since errors.Wrap() returns nil on nil error:

func Chdir(dir string) error {
	return errors.Wrap(syscall.Chdir(dir), "failed to change directory")
}
@gopherbot gopherbot added this to the Proposal milestone May 31, 2018
@VojtechVitek
Copy link
Author

I think we could simplify the https://github.com/pkg/errors API a little further and add two new functions only:

func Cause(err error) error
func Wrap(err error, message string) error
- func Errorf(format string, args ...interface{}) error
- func WithMessage(err error, message string) error
- func WithStack(err error) error
- func Wrapf(err error, format string, args ...interface{}) error
  1. assuming the stack traces were configurable globally, ie. via a flag:
func main() {
	if debug {
		errors.CaptureStackTraces = true // false by default
	}
}
  1. and if we kept using fmt package for formatted error messages (fmt.Errorf(), fmt.Wrapf()).

cc @bradfitz @davecheney

@ianlancetaylor ianlancetaylor added the v2 A language change or incompatible library change label May 31, 2018
@jellevandenhooff
Copy link
Contributor

jellevandenhooff commented May 31, 2018

As another point in the design space, a package like https://github.com/pkg/errors but with the simpler API is https://godoc.org/github.com/samsarahq/go/oops.

@ianlancetaylor
Copy link
Contributor

It's a good package, but what's the argument for bringing it into the standard library? Bear in mind https://golang.org/doc/faq#x_in_std .

@jellevandenhooff
Copy link
Contributor

jellevandenhooff commented May 31, 2018

One motivation for standardizing is uniform handling of well-known errors. For example, wrapping io.EOF confuses io.Copy, which ignores io.EOF but not when wrapped since it checks for equality. I don't have a great answer on what to do though, because sometimes you might want a wrapped error to have a different semantic meaning and other times you might want to preserve the original error but just have some added debug information.

@VojtechVitek
Copy link
Author

VojtechVitek commented May 31, 2018

@ianlancetaylor argument for bringing it into the standard library is just standardization of error context across the Go ecosystem. Not everyone uses github.com/pkg/errors as of now obviously.

Example:
I'd like to be able to inspect a stack trace of any incoming error from 3rd party pkgs. Currently, I struggle with some generic errors returned by vendored packages (ie. a variable instantiated during the boot time, returned by many functions/methods). I always end up debugging the underlying package itself (debugger or with injecting errors.Wrap()) instead of having a quick hint about the cause (file:line) in the first place.

@ianlancetaylor
Copy link
Contributor

Thanks. Standardization of error context is a big concept, and I don't think that the errors package addresses all aspects of it. For example, see https://commandcenter.blogspot.com/2017/12/error-handling-in-upspin.html for some additional ideas in this space.

@dsnet
Copy link
Member

dsnet commented Jun 1, 2018

\cc @neild @mpvl

@earthboundkid
Copy link
Contributor

I would also like to see standardization around some common optional interfaces, like IsTemporary() bool. Having it in the stdlib would encourage people to use the same interfaces when applicable.

@earthboundkid
Copy link
Contributor

I like the approach in this blog post, Failure Is Your Domain. The author emphasizes the importance of "owning" your failure modes, but I think that some of the functions mentioned in the article would be good candidates for standard library inclusion.

@nezorflame
Copy link

nezorflame commented Jun 7, 2018

I'd be glad to see standard errors package simply replaced by github.com/pkg/errors (or something with the same purpose), like it was mentioned here: #16968
Standard one doesn't give you anything which fmt.Errorf() doesn't do already.
Never understood why we have both fmt.Errorf() and errors.New() tbh, instead of doing errors.Newf() or fmt.Error().

@deanveloper
Copy link

Is there a reason why this is a Go 2 issue, rather than just including the new functions into the errors package? After all, it should be backwards compatible

@bradfitz
Copy link
Contributor

@deanveloper, because Go isn't a language where we just keep adding stuff all the time. We prefer to think about the end goal and the minimal set of things that would interact well. Once we add stuff, we can't remove it soon or easily, and it might not be what we want in the end. So we prefer to go slowly, even if that's boring and frustrating at times.

@rsc rsc changed the title proposal: Go 2: Error context (github.com/pkg/errors) proposal: Go 2: adopt Cause and Wrap from github.com/pkg/errors Aug 24, 2018
@ianlancetaylor
Copy link
Contributor

Closing in favor of the discussion at https://github.com/golang/go/wiki/Go2ErrorValuesFeedback . I have added this issue to that page.

@golang golang locked and limited conversation to collaborators Sep 25, 2019
@bradfitz bradfitz added the error-handling Language & library change proposals that are about error handling. label Oct 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
error-handling Language & library change proposals that are about error handling. FrozenDueToAge Proposal v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

9 participants