From 4951996f3ac75196db1712bacb1841dcacb33eb1 Mon Sep 17 00:00:00 2001 From: Liam Stanley Date: Fri, 31 Mar 2017 14:30:52 -0400 Subject: [PATCH] the start of more refactoring --- .travis.yml | 9 ++-- client.go | 105 ++++++--------------------------------------- client_test.go | 47 ++++---------------- conn.go | 113 +++++++++---------------------------------------- 4 files changed, 46 insertions(+), 228 deletions(-) diff --git a/.travis.yml b/.travis.yml index f7c4d4a..08fadc5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,15 @@ language: go go: - 1.6 -- 1.7 +- 1.8 - tip before_install: -- go get -u -v github.com/mattn/goveralls -- go get -u -v github.com/golang/lint/golint +- go get -v github.com/mattn/goveralls +- go get -v github.com/golang/lint/golint script: -- $HOME/gopath/bin/goveralls -service=travis-ci -v +- $HOME/gopath/bin/goveralls -service=travis-ci - $HOME/gopath/bin/golint -min_confidence 0.9 -set_exit_status +- go test -v -race - go tool vet -v -all . branches: only: diff --git a/client.go b/client.go index 683c4f5..ff19861 100644 --- a/client.go +++ b/client.go @@ -15,8 +15,6 @@ import ( "strings" "sync" "time" - - "golang.org/x/net/context" ) // Client contains all of the information necessary to run a single IRC @@ -43,11 +41,7 @@ type Client struct { // conn is a net.Conn reference to the IRC server. conn *ircConn - // tries represents the internal reconnect count to the IRC server. - tries int - // reconnecting is true if the client is reconnecting, used so multiple - // threads aren't trying to reconnect at the same time. - reconnecting bool + // cmux is the mux used for connections/disconnections from the server, // so multiple threads aren't trying to connect at the same time, and // vice versa. @@ -55,13 +49,6 @@ type Client struct { // debug is used if a writer is supplied for Client.Config.Debugger. debug *log.Logger - - // Below are functions used to close out goroutines opened by the client. - closeRead context.CancelFunc - closeSend context.CancelFunc - closeExec context.CancelFunc - closePing context.CancelFunc - closeLoop context.CancelFunc } // Config contains configuration options for an IRC client @@ -110,9 +97,6 @@ type Config struct { // socket creation to the server. SSL must be enabled for this to be used. // This only has an affect during the dial process. TLSConfig *tls.Config - // Retries is the number of times the client will attempt to reconnect - // to the server after the last disconnect. - Retries int // AllowFlood allows the client to bypass the rate limit of outbound // messages. AllowFlood bool @@ -139,19 +123,12 @@ type Config struct { // response to a CTCP VERSION, if default CTCP replies have not been // overwritten or a VERSION handler was already supplied. Version string - // ReconnectDelay is the a duration of time to delay before attempting a - // reconnection. Defaults to 10s (minimum of 5s). This is ignored if - // Reconnect() is called directly. - ReconnectDelay time.Duration // PingDelay is the frequency between when the client sends keep-alive // ping's to the server, and awaits a response (timing out if the server // doesn't respond in time). This must be between 20-600 seconds. See // Client.Lag() if you want to determine the delay between the server // and the client. PingDelay time.Duration - // HandleError if supplied, is called when one is disconnected from the - // server, with a given error. - HandleError func(error) // disableTracking disables all channel and user-level tracking. Useful // for highly embedded scripts with single purposes. @@ -246,90 +223,32 @@ func (c *Client) String() string { } return fmt.Sprintf( - "", - c.initTime.String(), c.Handlers.Len(), connected, c.reconnecting, c.tries, + "", c.initTime.String(), c.Handlers.Len(), connected, ) } -// cleanup is used to close out all threads used by the client, like read and -// write loops. -func (c *Client) cleanup(all bool) { - c.cmux.Lock() - - // Close any connections they have open. - if c.conn != nil { - c.conn.Close() - } - - c.flushTx() - - if c.closeRead != nil { - c.closeRead() - } - if c.closeSend != nil { - c.closeSend() - } - if c.closeExec != nil { - c.closeExec() - } - - if all { - if c.closeLoop != nil { - c.closeLoop() - } - } - - c.cmux.Unlock() -} - -// quit is the underlying wrapper to quit from the network and cleanup. -func (c *Client) quit(sendMessage bool) { - if sendMessage { - c.Send(&Event{Command: QUIT, Trailing: "disconnecting..."}) - } - - c.RunHandlers(&Event{Command: DISCONNECTED, Trailing: c.Server()}) - c.cleanup(false) -} - -// Quit disconnects from the server. -func (c *Client) Quit() { - c.quit(true) -} - -// QuitWithMessage disconnects from the server with a given message. -func (c *Client) QuitWithMessage(message string) { - c.Send(&Event{Command: QUIT, Trailing: message}) - c.quit(false) -} - -// Stop exits the clients main loop and any other goroutines created by +// Close exits the clients main loop and any other goroutines created by // the client itself. This does not include handlers, as they will run for -// any incoming events prior to when Stop() or Quit() was called, until the +// any incoming events prior to when Close() or Quit() was called, until the // event queue is empty and execution has completed for those handlers. This // means that you are responsible to ensure that your handlers due not // execute forever. Use Client.Quit() first if you want to disconnect the // client from the server/connection gracefully. -func (c *Client) Stop() { - c.quit(false) +func (c *Client) Close(sendQuit bool) { + if sendQuit { + c.Send(&Event{Command: QUIT, Trailing: "closing"}) + } + + _ = c.conn.Close() c.RunHandlers(&Event{Command: STOPPED, Trailing: c.Server()}) } -// Loop reads from the events channel and sends the events to be handled for -// every message it receives. -func (c *Client) Loop() { - var ctx context.Context - ctx, c.closeLoop = context.WithCancel(context.Background()) - - <-ctx.Done() -} - -func (c *Client) execLoop(ctx context.Context) { +func (c *Client) execLoop(done chan struct{}) { for { select { case event := <-c.rx: c.RunHandlers(event) - case <-ctx.Done(): + case <-done: return } } diff --git a/client_test.go b/client_test.go index deab715..d708dfc 100644 --- a/client_test.go +++ b/client_test.go @@ -5,7 +5,6 @@ package girc_test import ( - "fmt" "log" "os" "strings" @@ -15,25 +14,15 @@ import ( // The bare-minimum needed to get started with girc. Just connects and idles. func Example_bare() { - errHandler := func(err error) { - if err == nil { - return - } - - log.Fatalf("error occurred: %s", err) - } - client := girc.New(girc.Config{ - Server: "irc.byteirc.org", - Port: 6667, - Nick: "test", - User: "user", - Out: os.Stdout, - HandleError: errHandler, + Server: "irc.byteirc.org", + Port: 6667, + Nick: "test", + User: "user", + Debug: os.Stdout, }) - errHandler(client.Connect()) - client.Loop() + log.Fatal(client.Connect()) } // Very simple example that connects, joins a channel, and responds to @@ -58,11 +47,7 @@ func Example_simple() { } }) - if err := client.Connect(); err != nil { - log.Fatalf("an error occurred while attempting to connect to %s: %s", client.Server(), err) - } - - client.Loop() + log.Fatal(client.Connect()) } // Another basic example, however with this, we add simple ! @@ -74,6 +59,7 @@ func Example_commands() { Nick: "test", User: "user", Name: "Example bot", + Out: os.Stdout, }) client.Handlers.Add(girc.CONNECTED, func(c *girc.Client, e girc.Event) { @@ -87,27 +73,12 @@ func Example_commands() { } if strings.HasPrefix(e.Trailing, "!stop") { - c.Quit() - c.Stop() + c.Close(true) return } - - if strings.HasPrefix(e.Trailing, "!restart") { - go c.Reconnect() - return - } - }) - - // Log ALL events. - client.Handlers.Add(girc.ALLEVENTS, func(c *girc.Client, e girc.Event) { - // The use of girc.StripRaw() is to get rid of any potential - // non-printable characters. - fmt.Println(girc.StripRaw(e.String())) }) if err := client.Connect(); err != nil { log.Fatalf("an error occurred while attempting to connect to %s: %s", client.Server(), err) } - - client.Loop() } diff --git a/conn.go b/conn.go index 157d5da..de3d831 100644 --- a/conn.go +++ b/conn.go @@ -13,7 +13,6 @@ import ( "net/url" "time" - "golang.org/x/net/context" "golang.org/x/net/proxy" ) @@ -162,9 +161,6 @@ func (c *ircConn) Close() error { // Connect attempts to connect to the given IRC server func (c *Client) Connect() error { - // Clean up any old running stuff. - c.cleanup(false) - // We want to be the only one handling connects/disconnects right now. c.cmux.Lock() @@ -183,15 +179,15 @@ func (c *Client) Connect() error { c.cmux.Unlock() // Start read loop to process messages from the server. - var rctx, ectx, sctx, pctx context.Context - rctx, c.closeRead = context.WithCancel(context.Background()) - ectx, c.closeExec = context.WithCancel(context.Background()) - sctx, c.closeSend = context.WithCancel(context.Background()) - pctx, c.closePing = context.WithCancel(context.Background()) - go c.execLoop(ectx) - go c.readLoop(rctx) - go c.pingLoop(pctx) - go c.sendLoop(sctx) + errs := make(chan error, 4) + done := make(chan struct{}, 4) + defer close(errs) + defer close(done) + + go c.execLoop(done) + go c.readLoop(errs, done) + go c.pingLoop(errs, done) + go c.sendLoop(errs, done) // Send a virtual event allowing hooks for successful socket connection. c.RunHandlers(&Event{Command: INITIALIZED, Trailing: c.Server()}) @@ -215,87 +211,18 @@ func (c *Client) Connect() error { // support. c.listCAP() - // Consider the connection a success at this point. - c.tries = 0 - c.reconnecting = false - - return nil -} - -// reconnect is the internal wrapper for reconnecting to the IRC server (if -// requested.) -func (c *Client) reconnect(remoteInvoked bool) (err error) { - if c.reconnecting { - return nil - } - c.reconnecting = true - defer func() { - c.reconnecting = false - }() - - c.cleanup(false) - - if c.Config.ReconnectDelay < (5 * time.Second) { - c.Config.ReconnectDelay = 5 * time.Second - } - - if c.Config.Retries < 1 && !remoteInvoked { - return ErrDisconnected - } - - if !remoteInvoked { - // Delay so we're not slaughtering the server with a bunch of - // connections. - c.debug.Printf("reconnecting to %s in %s", c.Server(), c.Config.ReconnectDelay) - time.Sleep(c.Config.ReconnectDelay) - } - - for err = c.Connect(); err != nil && c.tries < c.Config.Retries; c.tries++ { - c.debug.Printf("reconnecting to %s in %s (%d tries)", c.Server(), c.Config.ReconnectDelay, c.tries) - time.Sleep(c.Config.ReconnectDelay) - } - - if err != nil { - // Too many errors at this point. - c.cleanup(false) - } - - return err -} - -// Reconnect checks to make sure we want to, and then attempts to reconnect -// to the server. This will ignore the reconnect delay. -func (c *Client) Reconnect() error { - return c.reconnect(true) -} - -func (c *Client) disconnectHandler(err error) { - if err != nil { - c.debug.Println("disconnecting due to error: " + err.Error()) - } - - rerr := c.reconnect(false) - if rerr != nil { - c.debug.Println("error: " + rerr.Error()) - if c.Config.HandleError != nil { - if c.Config.Retries < 1 { - c.Config.HandleError(err) - } - - c.Config.HandleError(rerr) - } - } + return <-errs } // readLoop sets a timeout of 300 seconds, and then attempts to read from the // IRC server. If there is an error, it calls Reconnect. -func (c *Client) readLoop(ctx context.Context) { +func (c *Client) readLoop(errs chan error, done chan struct{}) { var event *Event var err error for { select { - case <-ctx.Done(): + case <-done: return default: // c.conn.sock.SetDeadline(time.Now().Add(300 * time.Second)) @@ -304,8 +231,7 @@ func (c *Client) readLoop(ctx context.Context) { // Attempt a reconnect (if applicable). If it fails, send // the error to c.Config.HandleError to be dealt with, if // the handler exists. - c.disconnectHandler(err) - return + errs <- err } c.rx <- event @@ -344,12 +270,12 @@ func (c *ircConn) rate(chars int) time.Duration { return 0 } -func (c *Client) sendLoop(ctx context.Context) { +func (c *Client) sendLoop(errs chan error, done chan struct{}) { var err error for { select { - case <-ctx.Done(): + case <-done: return case event := <-c.tx: // Log the event. @@ -376,7 +302,8 @@ func (c *Client) sendLoop(ctx context.Context) { } if err != nil { - c.disconnectHandler(err) + errs <- err + return } } } @@ -397,7 +324,7 @@ func (c *Client) flushTx() { // before receiving a PONG back. var ErrTimedOut = errors.New("timed out during ping to server") -func (c *Client) pingLoop(ctx context.Context) { +func (c *Client) pingLoop(errs chan error, done chan struct{}) { c.conn.lastPing = time.Now() c.conn.lastPong = time.Now() @@ -410,13 +337,13 @@ func (c *Client) pingLoop(ctx context.Context) { for { select { - case <-ctx.Done(): + case <-done: return case <-tick.C: if time.Since(c.conn.lastPong) > c.Config.PingDelay+(60*time.Second) { // It's 60 seconds over what out ping delay is, connection // has probably dropped. - c.disconnectHandler(ErrTimedOut) + errs <- ErrTimedOut return }