Closed georgysavva closed 3 years ago
@georgysavva Any new information that you want to add to an error must be built around the original error. This creates layers of information around your original error. One layer can then contain a new error. Example:
func WithError(cause error, error error) *withError {
if err == nil {
return nil
}
return &withError{
cause,
error,
}
}
type withError struct {
cause error
error error
}
func (w *withError) Error() string {
return w.error.Error() + ": " + w.cause.Error()
}
func (w *withError) Cause() error {
return w.cause
}
func (w *withError) Unwrap() error {
return w.cause
}
func (w *withError) GetError() error {
return w.error
}
Now you can use that as follows:
err := WithError(errFoo, errBar)
But in your case above I will use the existing errors.WithMessage(errFoo, "barr")
.
Hi. Thanks a lot for all the details and complete example.
But in your case above I will use the existing errors.WithMessage(errFoo, "barr")
There is one more requirement that I forgot to mention: the client should be able to do errors.Is(err, errBar)
and get true
.
I think we can just add new Is
method to the withError
:
func (w *withError) Is(err error) bool {
return errors.Is(err, w.error)
}
Alternativly, we could swap errFoo
and errBar
in the WithError
arguments:
err := WithError(errBar, errFoo)
We would also need to add Format
method to withError
implementation to include all information(stack, text) from the w.error
i.e errFoo
in our example.
Please tell me what do you think about this?
If you are going to wrap errors just implementing Cause() error
is no longer enough, you also need to implement Unwrap() error
.
It looks like you do not actually need to return a whole new errBar
, you’re just trying to include context information about the wrapped error. Use instead. return errors.WithMessage(err, "bar")
.
Avoid making errors at the top level with errors.New()
, the stack trace attached will be useless only pointing to the line of the definition of the error. Instead, you want:
var (
errFoo = errors.New("foo")
)
func Foo() error {
return pkgErrors.WithStack(errFoo)
}
func Bar() error {
err := Foo()
if errors.Is(err, errFoo) {
return pkgErrors.WithMessage(err, "bar")
}
return err
}
Making a new error that contains two errors is just going to complicate things, as you will have to implement Is(error) bool
for both sides of the error, the first, and the second. That’s why you should try as hard as possible to ensure that you’re just wrapping errors like an onion, rather than building a tree or list of errors.
Cause() error is no longer enough, you also need to implement Unwrap() error
That is correct. I have edited my previous example.
Making a new error that contains two errors is just going to complicate things
@puellanivis indeed, it is actually not a good idea to merge multiple errors.
@georgysavva How do you actually get 2 errors in your case? By a goroutine?
And if you have 2 errors, it could also become more errors, right?
@reindert-vetter Hi. No, my case is not about joining two concurrent goroutines errors.
Here is my case:
https://github.com/georgysavva/scany/blob/d03fac1ef3c48f83eea39a48af58b17312a3a45a/sqlscan/sqlscan.go#L57-L60
If err
returned by dbscan.ScanOne
is the NotFound
error I need to return sql.ErrNoRows
error or something that will look like sql.ErrNoRows
so the client of my library could do errors.Is(err, sql.ErrNoRows
) and get true. But It also would be good to preserve all details contained in the initial err
returned by dbscan.ScanOne
such as stacktrace and error messages, so I need to somehow attach them to sql.ErrNoRows
error.
https://github.com/georgysavva/scany/blob/d03fac1ef3c48f83eea39a48af58b17312a3a45a/sqlscan/sqlscan.go#L57-L60 If
err
returned bydbscan.ScanOne
is theNotFound
error I need to returnsql.ErrNoRows
error or something that will look likesql.ErrNoRows
so the client of my library could doerrors.Is(err, sql.ErrNoRows
) and get true. But It also would be good to preserve all details contained in the initialerr
returned bydbscan.ScanOne
such as stacktrace and error messages, so I need to somehow attach them tosql.ErrNoRows
error.
This actually sounds like you want to merge two errors, unfortunately to play well with everything, you will need a lot of boilerplate code than is really a good idea:
// MergeErrors returns an error that composes two errors together.
// If either error is nil, then this just returns the other error.
// (Logically, if both are nil, this returns nil.)
// If both are non-nil then Wrap() and Cause() will return only the primary error.
func MergeErrors(primary error, secondary error) error {
if primary == nil {
return secondary
}
if secondary == nil {
return secondary
}
return &mergedErrors{ primary, secondary }
}
type mergedErrors struct {
primary, secondary error
}
func (e *mergedErrors) Error() string {
return e.primary.Error() + ": " + e.secondary.Error()
}
/* *** go1.13+ errors compat: *** */
func (e *mergedErrors) Unwrap() error {
return e.primary
}
// Is is necessary to test both sides of the error for equality.
func (e *mergedErrors) Is(target error) bool {
return errors.Is(w.primary, target) || errors.Is(w.secondary, target)
}
// As is necessary to test both sides of the error.
// The primary error takes precedence.
func (e *mergedErrors) As(target interface{}) bool {
return errors.As(e.primary, target) || errors.As(e.secondary, target)
}
/* *** pkg/errors compat: *** */
func (e *mergedErrors) Cause() error {
return e.primary
}
// Format is necessary to keep rich output.
func (e *mergedErrors) Format(s fmt.State, verb rune) {
if f, ok := e.primary.(fmt.Formatter); ok {
f.Format(s, verb)
} else {
io.WriteString(s, e.primary.Error())
}
s.Write([]byte(": "))
if f, ok := e.secondary.(fmt.Formatter); ok {
f.Format(s, verb)
} else {
io.WriteString(s, e.secondary.Error())
}
}
// StackTrace is necessary to expose an underlying stackrace.
func (e *mergedErrors) StackTrace() pkgErrors.StackTrace {
type stacktracer interface { StackTrace() pkgErrors.StackTrace }
if st, ok := e.primary.(stacktracer); ok {
return st.StackTrace()
}
if st, ok := e.secondary.(stacktracer); ok {
return st.StackTrace()
}
return nil
}
This is why I highly recommend against any sort of error merging, or naive wrapping. The pkg/errors
compat is the most extensive, and most annoying to keep writing in, but without it fmt.Sprintf("%+v", err)
just won’t do what you expect. The StackTrace
is probably the least important, very few things really use it, though I think some loggers might use it to extract out a stack trace to then format separately, rather than just a raw dump of the verbose error string alone.
But having random features just drop out of the behaviors becuase of wrapping is :cry:
Thanks again for a lot of new information. This is indeed what I was looking for, now everything is clear for me. Closing the issue!
Hi. What is the most idiomatic way to return a new error that was caused by another error and don't lose information (such as stack) about the previous error? Here is an example: