From 3db9e5e5afaf4bde69f42d13401c971cd1ee152f Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 6 Feb 2019 00:31:24 +0200 Subject: [PATCH 1/6] Implement Wrapper for errors --- internal/errors/errors.go | 16 +++++++++++++++- protocol/internal/error.go | 16 ++++++++++++---- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 552c3fc..b9280d9 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -4,6 +4,8 @@ import ( "errors" "fmt" "strings" + + "golang.org/x/xerrors" ) type causer interface { @@ -51,6 +53,7 @@ type wrapped struct { var _ error = (*wrapped)(nil) var _ causer = (*wrapped)(nil) +var _ xerrors.Wrapper = (*wrapped)(nil) func (e *wrapped) Error() string { return fmt.Sprintf("%s: %s", e.msg, e.err) @@ -60,6 +63,10 @@ func (e *wrapped) Cause() error { return e.err } +func (e *wrapped) Unwrap() error { + return e.err +} + // Merge merges multiple errors into one. // Merge returns nil if all errors are nil. func Merge(err ...error) error { @@ -72,7 +79,7 @@ func Merge(err ...error) error { if len(errs) == 0 { return nil } - return &merged{s: err} + return &merged{s: errs} } type merged struct { @@ -80,6 +87,7 @@ type merged struct { } var _ error = (*merged)(nil) +var _ xerrors.Wrapper = (*merged)(nil) func (e *merged) Error() string { var m []string @@ -88,3 +96,9 @@ func (e *merged) Error() string { } return strings.Join(m, ": ") } + +// Unwrap returns the first error since there +// is no way to create a chain of errors. +func (e *merged) Unwrap() error { + return e.s[0] +} diff --git a/protocol/internal/error.go b/protocol/internal/error.go index 04d2802..0c56326 100644 --- a/protocol/internal/error.go +++ b/protocol/internal/error.go @@ -2,6 +2,8 @@ package internal import ( "fmt" + + "golang.org/x/xerrors" ) // OpError represents an operational error. @@ -11,13 +13,18 @@ type OpError struct { Err error } +func (e OpError) Error() string { + return fmt.Sprintf("cdp.%s: %s: %s", e.Domain, e.Op, e.Err.Error()) +} + // Cause implements error causer. func (e *OpError) Cause() error { return e.Err } -func (e OpError) Error() string { - return fmt.Sprintf("cdp.%s: %s: %s", e.Domain, e.Op, e.Err.Error()) +// Unwrap implements Wrapper. +func (e *OpError) Unwrap() error { + return e.Err } type causer interface { @@ -25,6 +32,7 @@ type causer interface { } var ( - _ error = (*OpError)(nil) - _ causer = (*OpError)(nil) + _ error = (*OpError)(nil) + _ causer = (*OpError)(nil) + _ xerrors.Wrapper = (*OpError)(nil) ) From d06fa8c1a81f7c5f29d1b82d44d5a772b9faa3f9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Oct 2020 01:48:15 +0300 Subject: [PATCH 2/6] Cleanup error unwrapping, deprecate cdp.ErrorCause, add tests --- error.go | 2 + internal/errors/errors.go | 81 +++++++++++++++++++--------------- internal/errors/errors_test.go | 41 +++++++++++++++++ internal/errors/stdlib.go | 64 +++++++++++++++++++++++++++ protocol/internal/error.go | 13 +++--- session/manager.go | 9 ++-- 6 files changed, 160 insertions(+), 50 deletions(-) create mode 100644 internal/errors/stdlib.go diff --git a/error.go b/error.go index 7cd9345..f2c5847 100644 --- a/error.go +++ b/error.go @@ -6,4 +6,6 @@ import ( // ErrorCause returns the underlying cause for this error, if possible. // If err does not implement causer.Cause(), then err is returned. +// +// Deprecated: Use errors.Unwrap, errors.Is or errors.As instead. func ErrorCause(err error) error { return errors.Cause(err) } diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 815343c..8cd9565 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -4,20 +4,21 @@ import ( "errors" "fmt" "strings" - - "golang.org/x/xerrors" ) -// Causer is an interface to access to its direct cause of the error. -type Causer interface { - Cause() error -} +// Interfaces for common error unwrapping. +type causer interface{ Cause() error } +type wrapper interface{ Unwrap() error } // Cause returns the underlying cause for this error, if possible. -// If err does not implement Causer.Cause(), then err is returned. +// If err does not implement causer.Cause(), then err is returned. +// +// Deprecated: Use errors.Unwrap, errors.Is or errors.As instead. func Cause(err error) error { for err != nil { - if c, ok := err.(Causer); ok { + if c, ok := err.(wrapper); ok { + err = c.Unwrap() + } else if c, ok := err.(causer); ok { err = c.Cause() } else { return err @@ -26,16 +27,6 @@ func Cause(err error) error { return err } -// New returns an error that formats as the given text. -func New(text string) error { - return errors.New(text) -} - -// Errorf wraps New and fmt.Sprintf. -func Errorf(format string, a ...interface{}) error { - return New(fmt.Sprintf(format, a...)) -} - // Wrapf wraps an error with a message. Wrapf returns nil if error is nil. func Wrapf(err error, format string, a ...interface{}) error { if err == nil { @@ -53,20 +44,12 @@ type wrapped struct { } var _ error = (*wrapped)(nil) -var _ Causer = (*wrapped)(nil) -var _ xerrors.Wrapper = (*wrapped)(nil) +var _ causer = (*wrapped)(nil) +var _ wrapper = (*wrapped)(nil) -func (e *wrapped) Error() string { - return fmt.Sprintf("%s: %s", e.msg, e.err) -} - -func (e *wrapped) Cause() error { - return e.err -} - -func (e *wrapped) Unwrap() error { - return e.err -} +func (e *wrapped) Error() string { return fmt.Sprintf("%s: %s", e.msg, e.err.Error()) } +func (e *wrapped) Cause() error { return e.err } +func (e *wrapped) Unwrap() error { return e.err } // Merge merges multiple errors into one. // Merge returns nil if all errors are nil. @@ -88,7 +71,8 @@ type merged struct { } var _ error = (*merged)(nil) -var _ xerrors.Wrapper = (*merged)(nil) +var _ causer = (*merged)(nil) +var _ wrapper = (*merged)(nil) func (e *merged) Error() string { var m []string @@ -98,8 +82,33 @@ func (e *merged) Error() string { return strings.Join(m, ": ") } -// Unwrap returns the first error since there -// is no way to create a chain of errors. -func (e *merged) Unwrap() error { - return e.s[0] +// Unwrap returns only the first error, there is +// no way to create a queue of errors. +func (e *merged) Unwrap() error { return e.s[0] } + +// Cause returns only the first error, there is +// no way to create a queue of errors. +func (e *merged) Cause() error { return e.s[0] } + +// Is runs errors.Is on all merged errors. +func (e *merged) Is(target error) bool { + if target == nil { + return nil == e.s + } + for _, err := range e.s { + if errors.Is(err, target) { + return true + } + } + return false +} + +// As runs errors.As on all merged errors. +func (e *merged) As(target interface{}) bool { + for _, err := range e.s { + if errors.As(err, target) { + return true + } + } + return false } diff --git a/internal/errors/errors_test.go b/internal/errors/errors_test.go index a922d8f..8f4d25c 100644 --- a/internal/errors/errors_test.go +++ b/internal/errors/errors_test.go @@ -50,6 +50,47 @@ func TestMergeError(t *testing.T) { } } +func TestMergeErrorIs(t *testing.T) { + err1 := errors.New("first") + err2 := errors.New("second") + err3 := errors.New("third") + + got := Merge(err1, err2) + + if !errors.Is(got, err1) { + t.Errorf("merged error is not err1, want true, got false") + } + if !errors.Is(got, err2) { + t.Errorf("merged error is not err2, want true, got false") + } + if errors.Is(got, err3) { + t.Errorf("merged error is err3, want false, got true") + } +} + +type testErrorAs struct{ msg string } + +func (e testErrorAs) Error() string { return e.msg } + +func TestMergeErrorAs(t *testing.T) { + err1 := &wrapped{msg: "err1"} + err2 := testErrorAs{msg: "err2"} + + err := Merge(err1, err2) + got1 := &wrapped{} + if !errors.As(err, &got1) { + t.Errorf("merged error as wrapped failed, want true, got false") + } else if got1.msg != "err1" { + t.Errorf("merged error as wrapped did not assign the error, want msg=err1, got msg=%s", got1.msg) + } + got2 := testErrorAs{} + if !errors.As(err, &got2) { + t.Errorf("merged error as testErrorAs failed, want true, got false") + } else if got2.Error() != "err2" { + t.Errorf("merged error as testErrorAs did not assign the error, want msg=err2, got msg=%s", got2.Error()) + } +} + func TestMergeNoError(t *testing.T) { got := Merge(nil, nil) if got != nil { diff --git a/internal/errors/stdlib.go b/internal/errors/stdlib.go new file mode 100644 index 0000000..b921e07 --- /dev/null +++ b/internal/errors/stdlib.go @@ -0,0 +1,64 @@ +// Copyright 2011 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package errors + +import ( + "errors" + "fmt" +) + +// New returns an error that formats as the given text. +// Each call to New returns a distinct error value even if the text is identical. +func New(text string) error { return errors.New(text) } + +// Unwrap returns the result of calling the Unwrap method on err, if err's +// type contains an Unwrap method returning error. +// Otherwise, Unwrap returns nil. +func Unwrap(err error) error { return errors.Unwrap(err) } + +// Is reports whether any error in err's chain matches target. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error is considered to match a target if it is equal to that target or if +// it implements a method Is(error) bool such that Is(target) returns true. +// +// An error type might provide an Is method so it can be treated as equivalent +// to an existing error. For example, if MyError defines +// +// func (m MyError) Is(target error) bool { return target == os.ErrExist } +// +// then Is(MyError{}, os.ErrExist) returns true. See syscall.Errno.Is for +// an example in the standard library. +func Is(err, target error) bool { return errors.Is(err, target) } + +// As finds the first error in err's chain that matches target, and if so, sets +// target to that error value and returns true. Otherwise, it returns false. +// +// The chain consists of err itself followed by the sequence of errors obtained by +// repeatedly calling Unwrap. +// +// An error matches target if the error's concrete value is assignable to the value +// pointed to by target, or if the error has a method As(interface{}) bool such that +// As(target) returns true. In the latter case, the As method is responsible for +// setting target. +// +// An error type might provide an As method so it can be treated as if it were a +// different error type. +// +// As panics if target is not a non-nil pointer to either a type that implements +// error, or to any interface type. +func As(err error, target interface{}) bool { return errors.As(err, target) } + +// Errorf formats according to a format specifier and returns the string as a +// value that satisfies error. +// +// If the format specifier includes a %w verb with an error operand, +// the returned error will implement an Unwrap method returning the operand. It is +// invalid to include more than one %w verb or to supply it with an operand +// that does not implement the error interface. The %w verb is otherwise +// a synonym for %v. +func Errorf(format string, a ...interface{}) error { return fmt.Errorf(format, a...) } diff --git a/protocol/internal/error.go b/protocol/internal/error.go index 0c56326..f614458 100644 --- a/protocol/internal/error.go +++ b/protocol/internal/error.go @@ -2,8 +2,6 @@ package internal import ( "fmt" - - "golang.org/x/xerrors" ) // OpError represents an operational error. @@ -27,12 +25,11 @@ func (e *OpError) Unwrap() error { return e.Err } -type causer interface { - Cause() error -} +type causer interface{ Cause() error } +type wrapper interface{ Unwrap() error } var ( - _ error = (*OpError)(nil) - _ causer = (*OpError)(nil) - _ xerrors.Wrapper = (*OpError)(nil) + _ error = (*OpError)(nil) + _ causer = (*OpError)(nil) + _ wrapper = (*OpError)(nil) ) diff --git a/session/manager.go b/session/manager.go index 4068fa2..ab46364 100644 --- a/session/manager.go +++ b/session/manager.go @@ -70,7 +70,7 @@ func (m *Manager) watch(ev *sessionEvents, created <-chan *session, done, errC c defer ev.Close() isClosing := func(err error) bool { - for e := err;; { + for e := err; e != nil; { // Test if this is an rpcc.closeError. if v, ok := e.(interface{ Closed() bool }); ok && v.Closed() { // Cleanup, the underlying connection was closed @@ -79,11 +79,8 @@ func (m *Manager) watch(ev *sessionEvents, created <-chan *session, done, errC c m.cancel() return true } - if v, ok := e.(errors.Causer); ok { - e = v.Cause() - } else { - break - } + + e = errors.Unwrap(e) } if cdp.ErrorCause(err) == context.Canceled { // Manager was closed. From 614ba56793c5bec98d806710b5738ef6c0da2fc4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Oct 2020 01:49:21 +0300 Subject: [PATCH 3/6] rpcc: Implement Unwrap foor closeError --- rpcc/conn.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/rpcc/conn.go b/rpcc/conn.go index 431a57d..247995c 100644 --- a/rpcc/conn.go +++ b/rpcc/conn.go @@ -25,8 +25,9 @@ func (e *closeError) Error() string { } return e.msg } -func (e *closeError) Closed() bool { return true } -func (e *closeError) Cause() error { return e.err } +func (e *closeError) Closed() bool { return true } +func (e *closeError) Cause() error { return e.err } +func (e *closeError) Unwrap() error { return e.err } var ( // ErrConnClosing indicates that the operation is illegal because From e76f509a89df2d8bdc1b2ff5fea38315fbd7e54b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Oct 2020 01:49:53 +0300 Subject: [PATCH 4/6] travis: Drop Go 1.12, add Go 1.15 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9d72c8a..64eebe0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,9 +6,9 @@ addons: chrome: stable go: - - 1.12.x - 1.13.x - 1.14.x + - 1.15.x - master before_install: From ef428adf5b23dee4d871ae83401a45ca170867fa Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Oct 2020 02:06:55 +0300 Subject: [PATCH 5/6] Cleanup --- internal/errors/errors.go | 5 ++--- internal/errors/stdlib.go | 10 +++++----- session/manager.go | 22 ++++++++++------------ session/manager_test.go | 4 ++-- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/internal/errors/errors.go b/internal/errors/errors.go index 8cd9565..445500d 100644 --- a/internal/errors/errors.go +++ b/internal/errors/errors.go @@ -1,7 +1,6 @@ package errors import ( - "errors" "fmt" "strings" ) @@ -96,7 +95,7 @@ func (e *merged) Is(target error) bool { return nil == e.s } for _, err := range e.s { - if errors.Is(err, target) { + if Is(err, target) { return true } } @@ -106,7 +105,7 @@ func (e *merged) Is(target error) bool { // As runs errors.As on all merged errors. func (e *merged) As(target interface{}) bool { for _, err := range e.s { - if errors.As(err, target) { + if As(err, target) { return true } } diff --git a/internal/errors/stdlib.go b/internal/errors/stdlib.go index b921e07..9a55084 100644 --- a/internal/errors/stdlib.go +++ b/internal/errors/stdlib.go @@ -5,18 +5,18 @@ package errors import ( - "errors" + stderrors "errors" "fmt" ) // New returns an error that formats as the given text. // Each call to New returns a distinct error value even if the text is identical. -func New(text string) error { return errors.New(text) } +func New(text string) error { return stderrors.New(text) } // Unwrap returns the result of calling the Unwrap method on err, if err's // type contains an Unwrap method returning error. // Otherwise, Unwrap returns nil. -func Unwrap(err error) error { return errors.Unwrap(err) } +func Unwrap(err error) error { return stderrors.Unwrap(err) } // Is reports whether any error in err's chain matches target. // @@ -33,7 +33,7 @@ func Unwrap(err error) error { return errors.Unwrap(err) } // // then Is(MyError{}, os.ErrExist) returns true. See syscall.Errno.Is for // an example in the standard library. -func Is(err, target error) bool { return errors.Is(err, target) } +func Is(err, target error) bool { return stderrors.Is(err, target) } // As finds the first error in err's chain that matches target, and if so, sets // target to that error value and returns true. Otherwise, it returns false. @@ -51,7 +51,7 @@ func Is(err, target error) bool { return errors.Is(err, target) } // // As panics if target is not a non-nil pointer to either a type that implements // error, or to any interface type. -func As(err error, target interface{}) bool { return errors.As(err, target) } +func As(err error, target interface{}) bool { return stderrors.As(err, target) } // Errorf formats according to a format specifier and returns the string as a // value that satisfies error. diff --git a/session/manager.go b/session/manager.go index ab46364..ea9ae62 100644 --- a/session/manager.go +++ b/session/manager.go @@ -70,19 +70,17 @@ func (m *Manager) watch(ev *sessionEvents, created <-chan *session, done, errC c defer ev.Close() isClosing := func(err error) bool { - for e := err; e != nil; { - // Test if this is an rpcc.closeError. - if v, ok := e.(interface{ Closed() bool }); ok && v.Closed() { - // Cleanup, the underlying connection was closed - // before the Manager and its context does not - // inherit from rpcc.Conn. - m.cancel() - return true - } - - e = errors.Unwrap(e) + // Test if this is an rpcc.closeError. + var e interface{ Closed() bool } + if ok := errors.As(err, &e); ok && e.Closed() { + // Cleanup, the underlying connection was closed + // before the Manager and its context does not + // inherit from rpcc.Conn. + m.cancel() + return true } - if cdp.ErrorCause(err) == context.Canceled { + + if errors.Is(err, context.Canceled) { // Manager was closed. return true } diff --git a/session/manager_test.go b/session/manager_test.go index 7294171..8c10166 100644 --- a/session/manager_test.go +++ b/session/manager_test.go @@ -64,7 +64,7 @@ func TestManager_ErrorsAreSentOnErrChan(t *testing.T) { detached.err = errors.New("detach nope") detached.markReady() err := <-m.Err() - if err := errors.Cause(err); err != detached.err { + if !errors.Is(err, detached.err) { t.Errorf("got error: %v; want: %v", err, detached.err) } detached.next() @@ -73,7 +73,7 @@ func TestManager_ErrorsAreSentOnErrChan(t *testing.T) { message.err = errors.New("message nope") message.markReady() err = <-m.Err() - if err := errors.Cause(err); err != message.err { + if !errors.Is(err, message.err) { t.Errorf("got error: %v; want: %v", err, message.err) } message.next() From ed729e81057a69e79af035fd3cab576391d259ea Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 9 Oct 2020 02:09:44 +0300 Subject: [PATCH 6/6] Replace an instance of ErrorCause with errors.Is --- session/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session/session.go b/session/session.go index dd7fdbf..3d209eb 100644 --- a/session/session.go +++ b/session/session.go @@ -118,7 +118,7 @@ func dial(ctx context.Context, id target.ID, tc *cdp.Client, detachTimeout time. err := tc.Target.DetachFromTarget(ctx, target.NewDetachFromTargetArgs().SetSessionID(s.ID)) - if err := cdp.ErrorCause(err); err == context.DeadlineExceeded { + if errors.Is(err, context.DeadlineExceeded) { return fmt.Errorf("session: detach timed out for session %s", s.ID) } return errors.Wrapf(err, "session: detach failed for session %s", s.ID)