From 2d8d4516fdb46a6d0163447379bc6c6e2127d119 Mon Sep 17 00:00:00 2001 From: Mickael Remond Date: Fri, 7 Jun 2019 15:23:23 +0200 Subject: [PATCH] Handling basic unrecoverable errors Fix #43 --- auth.go | 6 ++++-- client.go | 9 +++++---- client_manager.go | 19 ++++++++++++++----- cmd/xmpp_echo/xmpp_echo.go | 7 +++++-- conn_error.go | 33 +++++++++++++++++++++++++++++++++ go.mod | 1 + go.sum | 6 ++---- session.go | 2 +- 8 files changed, 65 insertions(+), 18 deletions(-) create mode 100644 conn_error.go diff --git a/auth.go b/auth.go index 36ce19e..8569297 100644 --- a/auth.go +++ b/auth.go @@ -18,7 +18,8 @@ func authSASL(socket io.ReadWriter, decoder *xml.Decoder, f StreamFeatures, user } } if !havePlain { - return fmt.Errorf("PLAIN authentication is not supported by server: %v", f.Mechanisms.Mechanism) + err := fmt.Errorf("PLAIN authentication is not supported by server: %v", f.Mechanisms.Mechanism) + return NewConnError(err, true) } return authPlain(socket, decoder, user, password) @@ -41,7 +42,8 @@ func authPlain(socket io.ReadWriter, decoder *xml.Decoder, user string, password case SASLSuccess: case SASLFailure: // v.Any is type of sub-element in failure, which gives a description of what failed. - return errors.New("auth failure: " + v.Any.Local) + err := errors.New("auth failure: " + v.Any.Local) + return NewConnError(err, true) default: return errors.New("expected SASL success or failure, got " + v.Name()) } diff --git a/client.go b/client.go index 08ea283..ca6c95a 100644 --- a/client.go +++ b/client.go @@ -78,17 +78,18 @@ Setting up the client / Checking the parameters func NewClient(config Config) (c *Client, err error) { // TODO: If option address is nil, use the Jid domain to compose the address if config.Address, err = checkAddress(config.Address); err != nil { - return + return nil, NewConnError(err, true) } if config.Password == "" { err = errors.New("missing password") - return + return nil, NewConnError(err, true) } // Parse JID - if config.parsedJid, err = NewJid(c.config.Jid); err != nil { - return + if config.parsedJid, err = NewJid(config.Jid); err != nil { + err = errors.New("missing jid") + return nil, NewConnError(err, true) } c = new(Client) diff --git a/client_manager.go b/client_manager.go index 2e34e76..662aad4 100644 --- a/client_manager.go +++ b/client_manager.go @@ -1,8 +1,9 @@ package xmpp // import "gosrc.io/xmpp" import ( - "log" "time" + + "golang.org/x/xerrors" ) type PostConnect func(c *Client) @@ -29,7 +30,7 @@ func NewClientManager(client *Client, pc PostConnect) *ClientManager { } // Start launch the connection loop -func (cm *ClientManager) Start() { +func (cm *ClientManager) Start() error { cm.Client.Handler = func(e Event) { switch e.State { case StateConnected: @@ -41,7 +42,8 @@ func (cm *ClientManager) Start() { cm.connect() } } - cm.connect() + + return cm.connect() } // Stop cancels pending operations and terminates existing XMPP client. @@ -52,17 +54,23 @@ func (cm *ClientManager) Stop() { } // connect manages the reconnection loop and apply the define backoff to avoid overloading the server. -func (cm *ClientManager) connect() { +func (cm *ClientManager) connect() error { var backoff Backoff // TODO: Group backoff calculation features with connection manager? for { var err error + // TODO: Make it possible to define logger to log disconnect and reconnection attempts cm.Metrics = initMetrics() // TODO: Test for non recoverable errors (invalid username and password) and return an error // to start caller. We do not want to retry on non recoverable errors. if cm.Client.Session, err = cm.Client.Connect(); err != nil { - log.Printf("Connection error: %v\n", err) + var actualErr ConnError + if xerrors.As(err, &actualErr) { + if actualErr.Permanent { + return xerrors.Errorf("unrecoverable connect error %w", actualErr) + } + } backoff.Wait() } else { break @@ -72,6 +80,7 @@ func (cm *ClientManager) connect() { if cm.PostConnect != nil { cm.PostConnect(cm.Client) } + return nil } // Client Metrics diff --git a/cmd/xmpp_echo/xmpp_echo.go b/cmd/xmpp_echo/xmpp_echo.go index 30adfec..fed97c3 100644 --- a/cmd/xmpp_echo/xmpp_echo.go +++ b/cmd/xmpp_echo/xmpp_echo.go @@ -23,13 +23,16 @@ func main() { client, err := xmpp.NewClient(config) if err != nil { - log.Fatal("Error: ", err) + log.Fatalf("%+v", err) } cm := xmpp.NewClientManager(client, nil) - cm.Start() + err = cm.Start() // connection can be stopped with cm.Stop() // connection state can be checked by reading cm.Client.CurrentState + if err != nil { + log.Fatal(err) + } // Iterator to receive packets coming from our XMPP connection for packet := range client.Recv() { diff --git a/conn_error.go b/conn_error.go new file mode 100644 index 0000000..773da01 --- /dev/null +++ b/conn_error.go @@ -0,0 +1,33 @@ +package xmpp // import "gosrc.io/xmpp" + +import ( + "fmt" + + "golang.org/x/xerrors" +) + +type ConnError struct { + frame xerrors.Frame + err error + // Permanent will be true if error is not recoverable + Permanent bool +} + +func NewConnError(err error, permanent bool) ConnError { + return ConnError{err: err, frame: xerrors.Caller(1), Permanent: permanent} +} + +func (e ConnError) Format(s fmt.State, verb rune) { + xerrors.FormatError(e, s, verb) +} + +func (e ConnError) FormatError(p xerrors.Printer) error { + e.frame.Format(p) + return e.err +} + +func (e ConnError) Error() string { + return fmt.Sprint(e) +} + +func (e ConnError) Unwrap() error { return e.err } diff --git a/go.mod b/go.mod index 68d28b7..e9b9179 100644 --- a/go.mod +++ b/go.mod @@ -6,4 +6,5 @@ require ( github.com/google/go-cmp v0.2.0 github.com/processone/mpg123 v1.0.0 github.com/processone/soundcloud v1.0.0 + golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 ) diff --git a/go.sum b/go.sum index 5dc2bf4..8eef58d 100644 --- a/go.sum +++ b/go.sum @@ -1,12 +1,10 @@ github.com/google/go-cmp v0.2.0 h1:+dTQ8DZQJz0Mb/HjFlkptS1FeQ4cWSnN941F8aEG4SQ= github.com/google/go-cmp v0.2.0/go.mod h1:oXzfMopK8JAjlY9xF4vHSVASa0yLyX7SntLO5aqRK0M= -github.com/processone/mpg123 v0.0.0-20160212185547-a17074c7ddc0 h1:7Sb9wi06laajixrlpOJACEtAsnbDYlH8/bRTfLKAiF4= -github.com/processone/mpg123 v0.0.0-20160212185547-a17074c7ddc0/go.mod h1:e9Ud3cx6fZEPqmEYDqjIsRvgFRPvXWhb0H1Cmu+LUtk= github.com/processone/mpg123 v1.0.0 h1:o2WOyGZRM255or1Zc/LtF/jARn51B+9aQl72Qace0GA= github.com/processone/mpg123 v1.0.0/go.mod h1:X/FeL+h8vD1bYsG9tIWV3M2c4qNTZOficyvPVBP08go= -github.com/processone/soundcloud v0.0.0-20160217145628-430ee371ebce h1:lDqQi4xGLeS5mZYpVeyle5num0mWdyy37QmsQiFSX1Y= -github.com/processone/soundcloud v0.0.0-20160217145628-430ee371ebce/go.mod h1:QfHw5V3JsrdJoYH4aD2+lr18v97eA6wyHaV0Zi+FuOI= github.com/processone/soundcloud v1.0.0 h1:/+i6+Yveb7Y6IFGDSkesYI+HddblzcRTQClazzVHxoE= github.com/processone/soundcloud v1.0.0/go.mod h1:kDLeWpkRtN3C8kIReQdxoiRi92P9xR6yW6qLOJnNWfY= golang.org/x/net v0.0.0-20190110200230-915654e7eabc h1:Yx9JGxI1SBhVLFjpAkWMaO1TF+xyqtHLjZpvQboJGiM= golang.org/x/net v0.0.0-20190110200230-915654e7eabc/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= +golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522 h1:bhOzK9QyoD0ogCnFro1m2mz41+Ib0oOhfJnBp5MR4K4= +golang.org/x/xerrors v0.0.0-20190513163551-3ee3066db522/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/session.go b/session.go index a15fdb3..fd31f1a 100644 --- a/session.go +++ b/session.go @@ -43,7 +43,7 @@ func NewSession(conn net.Conn, o Config) (net.Conn, *Session, error) { } if !s.TlsEnabled && !o.Insecure { - return nil, nil, errors.New("failed to negotiate TLS") + return nil, nil, NewConnError(errors.New("failed to negotiate TLS session"), true) } // auth