Skip to content

Commit

Permalink
session: Cleanup session.Manager's handling of errors. (#122)
Browse files Browse the repository at this point in the history
* Cleanup session.Manager's handling of errors.

It turns out that cdp.ErrorCause tracks the list of causes and
returns the true cause of the problem. In the case of bug #120,
the topmost err was internal.OpError, and the ErrorCause returns
underlying websocket.CloseError.

Actually there's rpcc.closeError in the middle of cause chain,
but that's simply skipped, and therefore it fails to recognize
the error as closing.

This tracks of every step of Cause and return true if any of
these errors are closing one.

* session: Remove unused import

Co-authored-by: Mathias Fredriksson <[email protected]>
  • Loading branch information
jmuk and mafredri authored Jun 26, 2020
1 parent 93673b7 commit 86e3bb8
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 9 deletions.
9 changes: 5 additions & 4 deletions internal/errors/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,16 @@ import (
"strings"
)

type causer interface {
// Causer is an interface to access to its direct cause of the error.
type Causer interface {
Cause() 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.
func Cause(err error) error {
for err != nil {
if c, ok := err.(causer); ok {
if c, ok := err.(Causer); ok {
err = c.Cause()
} else {
return err
Expand Down Expand Up @@ -50,7 +51,7 @@ type wrapped struct {
}

var _ error = (*wrapped)(nil)
var _ causer = (*wrapped)(nil)
var _ Causer = (*wrapped)(nil)

func (e *wrapped) Error() string {
return fmt.Sprintf("%s: %s", e.msg, e.err)
Expand Down
10 changes: 5 additions & 5 deletions session/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"time"

"github.com/gorilla/websocket"
"github.com/mafredri/cdp"
"github.com/mafredri/cdp/internal/errors"
"github.com/mafredri/cdp/protocol/target"
Expand Down Expand Up @@ -71,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 := range []error{err, cdp.ErrorCause(err)} {
for e := err;; {
// Test if this is an rpcc.closeError.
if v, ok := e.(interface{ Closed() bool }); ok && v.Closed() {
// Cleanup, the underlying connection was closed
Expand All @@ -80,9 +79,10 @@ func (m *Manager) watch(ev *sessionEvents, created <-chan *session, done, errC c
m.cancel()
return true
}
if _, ok := e.(*websocket.CloseError); ok {
m.cancel()
return true
if v, ok := e.(errors.Causer); ok {
e = v.Cause()
} else {
break
}
}
if cdp.ErrorCause(err) == context.Canceled {
Expand Down

0 comments on commit 86e3bb8

Please sign in to comment.