From faff69d9f00cd325d6d0181e7495a70c5f31994f Mon Sep 17 00:00:00 2001 From: Sung Yoon Whang Date: Wed, 15 Mar 2023 22:40:46 -0700 Subject: [PATCH] Enable Errors support for any multi-error (#75) Starting Go 1.20, any multi-error should conform to the standard unwrap method: Unwrap() []error. This changes multierr.Errors() method to support any error that complies to that interface. Fix #70 / GO-1883 --- CHANGELOG.md | 5 +++++ error.go | 20 +------------------ error_post_go120.go | 19 ++++++++++++++++++ error_post_go120_test.go | 42 ++++++++++++++++++++++++++++++++++++++++ error_pre_go120.go | 20 +++++++++++++++++++ error_test.go | 3 ++- 6 files changed, 89 insertions(+), 20 deletions(-) create mode 100644 error_post_go120_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index cfd2e6a..ba87531 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ Releases ======== +Unreleased +==================== +- `Errors` now supports any error that implements multiple-error + interface. + v1.10.0 (2023-03-08) ==================== diff --git a/error.go b/error.go index 4ee4b9f..d78ea58 100644 --- a/error.go +++ b/error.go @@ -194,23 +194,7 @@ type errorGroup interface { // // Callers of this function are free to modify the returned slice. func Errors(err error) []error { - if err == nil { - return nil - } - - // Note that we're casting to multiError, not errorGroup. Our contract is - // that returned errors MAY implement errorGroup. Errors, however, only - // has special behavior for multierr-specific error objects. - // - // This behavior can be expanded in the future but I think it's prudent to - // start with as little as possible in terms of contract and possibility - // of misuse. - eg, ok := err.(*multiError) - if !ok { - return []error{err} - } - - return append(([]error)(nil), eg.Errors()...) + return extractErrors(err) } // multiError is an error that holds one or more errors. @@ -225,8 +209,6 @@ type multiError struct { errors []error } -var _ errorGroup = (*multiError)(nil) - // Errors returns the list of underlying errors. // // This slice MUST NOT be modified. diff --git a/error_post_go120.go b/error_post_go120.go index 0b00bec..a173f9c 100644 --- a/error_post_go120.go +++ b/error_post_go120.go @@ -27,3 +27,22 @@ package multierr func (merr *multiError) Unwrap() []error { return merr.Errors() } + +type multipleErrors interface { + Unwrap() []error +} + +func extractErrors(err error) []error { + if err == nil { + return nil + } + + // check if the given err is an Unwrapable error that + // implements multipleErrors interface. + eg, ok := err.(multipleErrors) + if !ok { + return []error{err} + } + + return append(([]error)(nil), eg.Unwrap()...) +} diff --git a/error_post_go120_test.go b/error_post_go120_test.go new file mode 100644 index 0000000..ed35289 --- /dev/null +++ b/error_post_go120_test.go @@ -0,0 +1,42 @@ +// Copyright (c) 2023 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +//go:build go1.20 +// +build go1.20 + +package multierr + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestErrorsOnErrorsJoin(t *testing.T) { + err1 := errors.New("err1") + err2 := errors.New("err2") + err := errors.Join(err1, err2) + + errs := Errors(err) + assert.Equal(t, 2, len(errs)) + assert.Equal(t, err1, errs[0]) + assert.Equal(t, err2, errs[1]) +} diff --git a/error_pre_go120.go b/error_pre_go120.go index 8da10f1..93872a3 100644 --- a/error_pre_go120.go +++ b/error_pre_go120.go @@ -57,3 +57,23 @@ func (merr *multiError) Is(target error) bool { } return false } + +func extractErrors(err error) []error { + if err == nil { + return nil + } + + // Note that we're casting to multiError, not errorGroup. Our contract is + // that returned errors MAY implement errorGroup. Errors, however, only + // has special behavior for multierr-specific error objects. + // + // This behavior can be expanded in the future but I think it's prudent to + // start with as little as possible in terms of contract and possibility + // of misuse. + eg, ok := err.(*multiError) + if !ok { + return []error{err} + } + + return append(([]error)(nil), eg.Errors()...) +} diff --git a/error_test.go b/error_test.go index 39ca313..f0bb17a 100644 --- a/error_test.go +++ b/error_test.go @@ -386,7 +386,8 @@ func TestErrors(t *testing.T) { dontCast: true, }, { - // We don't yet support non-multierr errors. + // We don't yet support non-multierr errors that do + // not implement Unwrap() []error. give: notMultiErr{}, want: []error{notMultiErr{}}, dontCast: true,