From cd55a59ff947f7f279b19e085fe936b9a23f1df7 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 22:47:34 -0800 Subject: [PATCH 01/13] Refactor open/close logic. Support OnConnect and OnDisconnect. --- discord.go | 48 ++------ examples/new_basic/new_basic.go | 3 + structs.go | 19 ++-- wsapi.go | 193 ++++++++++++++++---------------- 4 files changed, 122 insertions(+), 141 deletions(-) diff --git a/discord.go b/discord.go index e77ec94..57c7583 100644 --- a/discord.go +++ b/discord.go @@ -111,20 +111,27 @@ func New(args ...interface{}) (s *Session, err error) { } } - // TODO: Add code here to fetch authenticated user info like settings, - // avatar, User ID, etc. If fails, return error. + // The Session is now able to have RestAPI methods called on it. + // It is recommended that you now call OpenAndListen so that events + // will begin to trigger. - // Open websocket connection + return +} + +// OpenAndListen is a helper method that opens the websocket connection, +// does the required handshake and then immediately begins listening. +// This is the preferred way to start listening for events and is safe +// to be called inside an OnDisconnect handler. +func (s *Session) OpenAndListen() (err error) { + // Open websocket connection. err = s.Open() if err != nil { - fmt.Println(err) return } // Do websocket handshake. err = s.Handshake() if err != nil { - fmt.Println(err) return } @@ -133,34 +140,3 @@ func New(args ...interface{}) (s *Session, err error) { return } - -// Close closes a Discord session -// TODO: Add support for Voice WS/UDP connections -func (s *Session) Close() { - - s.DataReady = false - - if s.heartbeatChan != nil { - select { - case <-s.heartbeatChan: - break - default: - close(s.heartbeatChan) - } - s.heartbeatChan = nil - } - - if s.listenChan != nil { - select { - case <-s.listenChan: - break - default: - close(s.listenChan) - } - s.listenChan = nil - } - - if s.wsConn != nil { - s.wsConn.Close() - } -} diff --git a/examples/new_basic/new_basic.go b/examples/new_basic/new_basic.go index 88fca12..752ce78 100644 --- a/examples/new_basic/new_basic.go +++ b/examples/new_basic/new_basic.go @@ -31,6 +31,9 @@ func main() { // Register messageCreate as a callback for the OnMessageCreate event. dg.OnMessageCreate = messageCreate + // Open the websocket and begin listening. + dg.OpenAndListen() + // Simple way to keep program running until any key press. var input string fmt.Scanln(&input) diff --git a/structs.go b/structs.go index 091eda2..5445ee8 100644 --- a/structs.go +++ b/structs.go @@ -23,10 +23,19 @@ import ( // token : The authentication token returned from Discord // Debug : If set to ture debug logging will be displayed. type Session struct { + sync.Mutex + // General configurable settings. Token string // Authentication token for this session Debug bool // Debug for printing JSON request/responses + // Settable Callback functions for Internal Events + // OnConnect is called when the websocket connection opens. + OnConnect func(*Session) + // OnDisconnect is called when the websocket connection closes. + // This is a good handler to add reconnection logic to. + OnDisconnect func(*Session) + // Settable Callback functions for Websocket Events OnEvent func(*Session, *Event) OnReady func(*Session, *Ready) @@ -81,14 +90,8 @@ type Session struct { StateEnabled bool StateMaxMessageCount int - // Mutex/Bools for locks that prevent accidents. - // TODO: Add channels. - - heartbeatLock sync.Mutex - heartbeatChan chan struct{} - - listenLock sync.Mutex - listenChan chan struct{} + // When nil, the session is not listening. + listening chan interface{} } // A VoiceRegion stores data for a specific voice region server. diff --git a/wsapi.go b/wsapi.go index 3f8bafc..df4439a 100644 --- a/wsapi.go +++ b/wsapi.go @@ -11,6 +11,7 @@ package discordgo import ( + "errors" "fmt" "runtime" "time" @@ -20,6 +21,20 @@ import ( // Open opens a websocket connection to Discord. func (s *Session) Open() (err error) { + s.Lock() + defer func() { + s.Unlock() + // Fire OnConnect after we have unlocked the mutex, + // otherwise we may deadlock. + if err == nil && s.OnConnect != nil { + s.OnConnect(s) + } + }() + + if s.wsConn != nil { + err = errors.New("Web socket already opened.") + return + } // Get the gateway to use for the Websocket connection g, err := s.Gateway() @@ -30,6 +45,37 @@ func (s *Session) Open() (err error) { // TODO: See if there's a use for the http response. // conn, response, err := websocket.DefaultDialer.Dial(session.Gateway, nil) s.wsConn, _, err = websocket.DefaultDialer.Dial(g, nil) + if err != nil { + return + } + + return +} + +// Close closes a websocket and stops all listening/heartbeat goroutines. +func (s *Session) Close() (err error) { + s.Lock() + defer func() { + s.Unlock() + // Fire OnDisconnect after we have unlocked the mutex + // otherwise we may deadlock, especially in reconnect logic. + if err == nil && s.OnDisconnect != nil { + s.OnDisconnect(s) + } + }() + + s.DataReady = false + + if s.listening != nil { + close(s.listening) + s.listening = nil + } + + if s.wsConn != nil { + err = s.wsConn.Close() + s.wsConn = nil + } + return } @@ -53,12 +99,9 @@ type handshakeOp struct { } // Handshake sends the client data to Discord during websocket initial connection. -func (s *Session) Handshake() (err error) { - // maybe this is SendOrigin? not sure the right name here - +func (s *Session) Handshake() error { data := handshakeOp{2, handshakeData{3, s.Token, handshakeProperties{runtime.GOOS, "Discordgo v" + VERSION, "", "", ""}}} - err = s.wsConn.WriteJSON(data) - return + return s.wsConn.WriteJSON(data) } type updateStatusGame struct { @@ -79,6 +122,11 @@ type updateStatusOp struct { // If idle>0 then set status to idle. If game>0 then set game. // if otherwise, set status to active, and no game. func (s *Session) UpdateStatus(idle int, game string) (err error) { + s.RLock() + defer s.RUnlock() + if s.wsConn == nil { + return errors.New("No websocket connection exists.") + } var usd updateStatusData if idle > 0 { @@ -96,65 +144,36 @@ func (s *Session) UpdateStatus(idle int, game string) (err error) { // Listen starts listening to the websocket connection for events. func (s *Session) Listen() (err error) { - // TODO: need a channel or something to communicate - // to this so I can tell it to stop listening - + s.Lock() if s.wsConn == nil { - fmt.Println("No websocket connection exists.") - return // TODO need to return an error. + s.Unlock() + return errors.New("No websocket connection exists.") + } + if s.listening != nil { + s.Unlock() + return errors.New("Already listening to websocket.") } - // Make sure Listen is not already running - s.listenLock.Lock() - if s.listenChan != nil { - s.listenLock.Unlock() - return - } - s.listenChan = make(chan struct{}) - s.listenLock.Unlock() + s.listening = make(chan interface{}) - // this is ugly. - defer func() { - if s.listenChan == nil { - return - } - select { - case <-s.listenChan: - break - default: - close(s.listenChan) - } - s.listenChan = nil - }() + s.Unlock() - // this is ugly. - defer func() { - if s.heartbeatChan == nil { - return - } - select { - case <-s.heartbeatChan: - break - default: - close(s.heartbeatChan) - } - s.listenChan = nil - }() + // Keep a reference, as s.listening can be nilled out. + listening := s.listening for { messageType, message, err := s.wsConn.ReadMessage() if err != nil { - fmt.Println("Websocket Listen Error", err) - // TODO Log error - break + // Defer so we get better log ordering. + defer s.Close() + return fmt.Errorf("Websocket Listen Error", err) } - go s.event(messageType, message) - // If our chan gets closed, exit out of this loop. - // TODO: Can we make this smarter, using select - // and some other trickery? http://www.goinggo.net/2013/10/my-channel-select-bug.html - if s.listenChan == nil { - return nil + select { + case <-listening: + return + default: + go s.event(messageType, message) } } @@ -192,8 +211,8 @@ func (s *Session) event(messageType int, message []byte) (err error) { } switch e.Type { - case "READY": + s.DataReady = true var st *Ready if err = unmarshalEvent(e, &st); err == nil { if s.StateEnabled { @@ -202,8 +221,8 @@ func (s *Session) event(messageType int, message []byte) (err error) { if s.OnReady != nil { s.OnReady(s, st) } - go s.Heartbeat(st.HeartbeatInterval) } + go s.heartbeat(st.HeartbeatInterval) if s.OnReady != nil { return } @@ -541,58 +560,38 @@ func (s *Session) event(messageType int, message []byte) (err error) { return } -// Heartbeat sends regular heartbeats to Discord so it knows the client +func (s *Session) sendHeartbeat() error { + return s.wsConn.WriteJSON(map[string]int{ + "op": 1, + "d": int(time.Now().Unix()), + }) +} + +// heartbeat sends regular heartbeats to Discord so it knows the client // is still connected. If you do not send these heartbeats Discord will // disconnect the websocket connection after a few seconds. -func (s *Session) Heartbeat(i time.Duration) { +func (s *Session) heartbeat(i time.Duration) { + // Keep a reference, as s.listening can be nilled out. + listening := s.listening - if s.wsConn == nil { - fmt.Println("No websocket connection exists.") - return // TODO need to return/log an error. - } - - // Make sure Heartbeat is not already running - s.heartbeatLock.Lock() - if s.heartbeatChan != nil { - s.heartbeatLock.Unlock() + // Send first heartbeat immediately because lag could put the + // first heartbeat outside the required heartbeat interval window. + err := s.sendHeartbeat() + if err != nil { + fmt.Println("Error sending initial heartbeat:", err) return } - s.heartbeatChan = make(chan struct{}) - s.heartbeatLock.Unlock() - // this is ugly. - defer func() { - if s.heartbeatChan == nil { - return - } - select { - case <-s.heartbeatChan: - break - default: - close(s.heartbeatChan) - } - s.listenChan = nil - }() - - // send first heartbeat immediately because lag could put the - // first heartbeat outside the required heartbeat interval window ticker := time.NewTicker(i * time.Millisecond) for { - - err := s.wsConn.WriteJSON(map[string]int{ - "op": 1, - "d": int(time.Now().Unix()), - }) - if err != nil { - fmt.Println("error sending data heartbeat:", err) - s.DataReady = false - return // TODO log error? - } - s.DataReady = true - select { case <-ticker.C: - case <-s.heartbeatChan: + err := s.sendHeartbeat() + if err != nil { + fmt.Println("Error sending heartbeat:", err) + return + } + case <-listening: return } } From 7a9b9428ee4c56257f561397f070e2814f2c9863 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 22:52:42 -0800 Subject: [PATCH 02/13] Re-add old todo. --- wsapi.go | 1 + 1 file changed, 1 insertion(+) diff --git a/wsapi.go b/wsapi.go index df4439a..183986f 100644 --- a/wsapi.go +++ b/wsapi.go @@ -53,6 +53,7 @@ func (s *Session) Open() (err error) { } // Close closes a websocket and stops all listening/heartbeat goroutines. +// TODO: Add support for Voice WS/UDP connections func (s *Session) Close() (err error) { s.Lock() defer func() { From 2a9538e9f16e67f50b1426eb80b0f0a801eb0b90 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 23:01:20 -0800 Subject: [PATCH 03/13] I must stop doing last minute changes. --- structs.go | 2 +- wsapi.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/structs.go b/structs.go index 5445ee8..68866ca 100644 --- a/structs.go +++ b/structs.go @@ -23,7 +23,7 @@ import ( // token : The authentication token returned from Discord // Debug : If set to ture debug logging will be displayed. type Session struct { - sync.Mutex + sync.RWMutex // General configurable settings. Token string // Authentication token for this session diff --git a/wsapi.go b/wsapi.go index 183986f..c3ab448 100644 --- a/wsapi.go +++ b/wsapi.go @@ -163,8 +163,9 @@ func (s *Session) Listen() (err error) { listening := s.listening for { - messageType, message, err := s.wsConn.ReadMessage() - if err != nil { + messageType, message, err1 := s.wsConn.ReadMessage() + if err1 != nil { + err = err1 // Defer so we get better log ordering. defer s.Close() return fmt.Errorf("Websocket Listen Error", err) From 13086b8da3a8894f7e62f9dc8f3525ff4c49dc23 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 23:14:42 -0800 Subject: [PATCH 04/13] Fix tests. :ok_hand: --- discord_test.go | 42 +++++++++++++++++++++++++++++++++--------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/discord_test.go b/discord_test.go index a9ddc50..77620d7 100644 --- a/discord_test.go +++ b/discord_test.go @@ -76,10 +76,13 @@ func TestNew(t *testing.T) { // TestInvalidToken tests the New() function with an invalid token func TestInvalidToken(t *testing.T) { + d, err := New("asjkldhflkjasdh") + if err != nil { + t.Fatalf("New(InvalidToken) returned error: %+v", err) + } - _, err := New("asjkldhflkjasdh") - if err == nil { - t.Errorf("New(InvalidToken) returned nil error.") + if err = d.OpenAndListen(); err == nil { + t.Fatalf("New(InvalidToken), d.OpenAndListen did not fail.") } } @@ -130,6 +133,10 @@ func TestNewUserPass(t *testing.T) { t.Fatal("New(user,pass), d.Token is empty, should be a valid Token.") } + if err = d.OpenAndListen(); err != nil { + t.Fatalf("New(user,pass), d.OpenAndListen failed: %+v", err) + } + if !waitBoolEqual(10*time.Second, &d.DataReady, true) { t.Fatal("New(user,pass), d.DataReady is false after 10 seconds. Should be true.") } @@ -141,12 +148,6 @@ func TestNewUserPass(t *testing.T) { } } -func TestClose(t *testing.T) { - if dg != nil { - dg.Close() - } -} - // TestNewToken tests the New() function with a Token. This should return // the same as the TestNewUserPass function. func TestNewToken(t *testing.T) { @@ -168,6 +169,10 @@ func TestNewToken(t *testing.T) { t.Fatal("New(envToken), d.Token is empty, should be a valid Token.") } + if err = d.OpenAndListen(); err != nil { + t.Fatalf("New(envToken), d.OpenAndListen failed: %+v", err) + } + if !waitBoolEqual(10*time.Second, &d.DataReady, true) { t.Fatal("New(envToken), d.DataReady is false after 10 seconds. Should be true.") } @@ -176,3 +181,22 @@ func TestNewToken(t *testing.T) { dg = d } + +func TestClose(t *testing.T) { + if envToken == "" { + t.Skip("Skipping TestClose, DG_TOKEN not set") + } + + d, err := New(envToken) + if err != nil { + t.Fatalf("TestClose, New(envToken) returned error: %+v", err) + } + + if err = d.OpenAndListen(); err != nil { + t.Fatalf("TestClose, d.OpenAndListen failed: %+v", err) + } + + if err = d.Close(); err != nil { + t.Fatalf("TestClose, d.Close failed: %+v", err) + } +} From 5f9326d1656633051ac202b08fc97d8522660159 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 23:38:17 -0800 Subject: [PATCH 05/13] Clean up tests further. --- discord_test.go | 45 ++++++++++++--------------------------------- restapi_test.go | 38 ++++++++++++++------------------------ 2 files changed, 26 insertions(+), 57 deletions(-) diff --git a/discord_test.go b/discord_test.go index 77620d7..28c21e9 100644 --- a/discord_test.go +++ b/discord_test.go @@ -21,6 +21,16 @@ var ( envAdmin string = os.Getenv("DG_ADMIN") // User ID of admin user to use for tests ) +func init() { + if envEmail == "" || envPassword == "" || envToken == "" { + return + } + + if d, err := New(envEmail, envPassword, envToken); err == nil { + dg = d + } +} + ////////////////////////////////////////////////////////////////////////////// //////////////////////////////////////////// HELPER FUNCTIONS USED FOR TESTING @@ -76,15 +86,10 @@ func TestNew(t *testing.T) { // TestInvalidToken tests the New() function with an invalid token func TestInvalidToken(t *testing.T) { - d, err := New("asjkldhflkjasdh") + _, err := New("asjkldhflkjasdh") if err != nil { t.Fatalf("New(InvalidToken) returned error: %+v", err) } - - if err = d.OpenAndListen(); err == nil { - t.Fatalf("New(InvalidToken), d.OpenAndListen did not fail.") - } - } // TestInvalidUserPass tests the New() function with an invalid Email and Pass @@ -132,20 +137,6 @@ func TestNewUserPass(t *testing.T) { if d.Token == "" { t.Fatal("New(user,pass), d.Token is empty, should be a valid Token.") } - - if err = d.OpenAndListen(); err != nil { - t.Fatalf("New(user,pass), d.OpenAndListen failed: %+v", err) - } - - if !waitBoolEqual(10*time.Second, &d.DataReady, true) { - t.Fatal("New(user,pass), d.DataReady is false after 10 seconds. Should be true.") - } - - t.Log("Successfully connected to Discord via New(user,pass).") - dg = d - if envToken == "" { - envToken = dg.Token - } } // TestNewToken tests the New() function with a Token. This should return @@ -168,21 +159,9 @@ func TestNewToken(t *testing.T) { if d.Token == "" { t.Fatal("New(envToken), d.Token is empty, should be a valid Token.") } - - if err = d.OpenAndListen(); err != nil { - t.Fatalf("New(envToken), d.OpenAndListen failed: %+v", err) - } - - if !waitBoolEqual(10*time.Second, &d.DataReady, true) { - t.Fatal("New(envToken), d.DataReady is false after 10 seconds. Should be true.") - } - - t.Log("Successfully connected to Discord via New(token).") - dg = d - } -func TestClose(t *testing.T) { +func TestOpenClose(t *testing.T) { if envToken == "" { t.Skip("Skipping TestClose, DG_TOKEN not set") } diff --git a/restapi_test.go b/restapi_test.go index b24b61a..d0f3e80 100644 --- a/restapi_test.go +++ b/restapi_test.go @@ -10,8 +10,8 @@ import ( // TestLogout tests the Logout() function. This should not return an error. func TestLogout(t *testing.T) { - if dg == nil || dg.Token == "" { - t.Skip("Cannot test logout, dg.Token not set.") + if dg == nil { + t.Skip("Cannot TestLogout, dg not set.") } err := dg.Logout() @@ -21,8 +21,8 @@ func TestLogout(t *testing.T) { } func TestUserAvatar(t *testing.T) { - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot TestUserAvatar, dg not set.") } a, err := dg.UserAvatar("@me") @@ -39,14 +39,8 @@ func TestUserAvatar(t *testing.T) { } func TestUserUpdate(t *testing.T) { - - if envEmail == "" || envPassword == "" { - t.Skip("Skipping, DG_USERNAME or DG_PASSWORD not set") - return - } - - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot test logout, dg not set.") } u, err := dg.User("@me") @@ -73,9 +67,8 @@ func TestUserUpdate(t *testing.T) { //func (s *Session) UserChannelCreate(recipientID string) (st *Channel, err error) { func TestUserChannelCreate(t *testing.T) { - - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot TestUserChannelCreate, dg not set.") } if envAdmin == "" { @@ -91,9 +84,8 @@ func TestUserChannelCreate(t *testing.T) { } func TestUserChannels(t *testing.T) { - - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot TestUserChannels, dg not set.") } _, err := dg.UserChannels() @@ -103,9 +95,8 @@ func TestUserChannels(t *testing.T) { } func TestUserGuilds(t *testing.T) { - - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot TestUserGuilds, dg not set.") } _, err := dg.UserGuilds() @@ -115,9 +106,8 @@ func TestUserGuilds(t *testing.T) { } func TestUserSettings(t *testing.T) { - - if !isConnected() { - t.Skip("Skipped, Not connected to Discord.") + if dg == nil { + t.Skip("Cannot TestUserSettings, dg not set.") } _, err := dg.UserSettings() From 2cbd402bae9f7507f18bc935989654ec813688a9 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 23:45:36 -0800 Subject: [PATCH 06/13] Make InvalidToken test useful again. --- discord_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/discord_test.go b/discord_test.go index 28c21e9..1fe7e90 100644 --- a/discord_test.go +++ b/discord_test.go @@ -86,10 +86,16 @@ func TestNew(t *testing.T) { // TestInvalidToken tests the New() function with an invalid token func TestInvalidToken(t *testing.T) { - _, err := New("asjkldhflkjasdh") + d, err := New("asjkldhflkjasdh") if err != nil { t.Fatalf("New(InvalidToken) returned error: %+v", err) } + + // New with just a token does not do any communication, so attempt an api call. + _, err = d.UserSettings() + if err == nil { + t.Errorf("New(InvalidToken), d.UserSettings returned nil error.") + } } // TestInvalidUserPass tests the New() function with an invalid Email and Pass From 7d984e7df69779067f2c8f665b652c6db23d551f Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Wed, 20 Jan 2016 23:48:30 -0800 Subject: [PATCH 07/13] Add test for u/p/token login. --- discord_test.go | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/discord_test.go b/discord_test.go index 1fe7e90..2dde3f7 100644 --- a/discord_test.go +++ b/discord_test.go @@ -122,8 +122,7 @@ func TestInvalidPass(t *testing.T) { } // TestNewUserPass tests the New() function with a username and password. -// This should return a valid Session{}, a valid Session.Token, and open -// a websocket connection to Discord. +// This should return a valid Session{}, a valid Session.Token. func TestNewUserPass(t *testing.T) { if envEmail == "" || envPassword == "" { @@ -167,6 +166,29 @@ func TestNewToken(t *testing.T) { } } +// TestNewUserPassToken tests the New() function with a username, password and token. +// This should return the same as the TestNewUserPass function. +func TestNewUserPassToken(t *testing.T) { + + if envEmail == "" || envPassword == "" || envToken == "" { + t.Skip("Skipping New(username,password,token), DG_EMAIL, DG_PASSWORD or DG_TOKEN not set") + return + } + + d, err := New(envEmail, envPassword, envToken) + if err != nil { + t.Fatalf("New(user,pass,token) returned error: %+v", err) + } + + if d == nil { + t.Fatal("New(user,pass,token), d is nil, should be Session{}") + } + + if d.Token == "" { + t.Fatal("New(user,pass,token), d.Token is empty, should be a valid Token.") + } +} + func TestOpenClose(t *testing.T) { if envToken == "" { t.Skip("Skipping TestClose, DG_TOKEN not set") From c4869d7a43e5966389976c9e447ed768ef8ec3e2 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 09:19:20 -0800 Subject: [PATCH 08/13] Remove handshake. --- discord.go | 6 -- examples/api_basic/api_basic.go | 6 -- wsapi.go | 98 +++++++++++++++++---------------- 3 files changed, 50 insertions(+), 60 deletions(-) diff --git a/discord.go b/discord.go index 57c7583..6014d87 100644 --- a/discord.go +++ b/discord.go @@ -129,12 +129,6 @@ func (s *Session) OpenAndListen() (err error) { return } - // Do websocket handshake. - err = s.Handshake() - if err != nil { - return - } - // Listen for events. go s.Listen() diff --git a/examples/api_basic/api_basic.go b/examples/api_basic/api_basic.go index 890fa9f..8c8fcd2 100644 --- a/examples/api_basic/api_basic.go +++ b/examples/api_basic/api_basic.go @@ -40,12 +40,6 @@ func main() { fmt.Println(err) } - // Do websocket handshake. - err = dg.Handshake() - if err != nil { - fmt.Println(err) - } - // Listen for events. go dg.Listen() diff --git a/wsapi.go b/wsapi.go index c3ab448..bc0b6ef 100644 --- a/wsapi.go +++ b/wsapi.go @@ -19,15 +19,31 @@ import ( "github.com/gorilla/websocket" ) +type handshakeProperties struct { + OS string `json:"$os"` + Browser string `json:"$browser"` + Device string `json:"$device"` + Referer string `json:"$referer"` + ReferringDomain string `json:"$referring_domain"` +} + +type handshakeData struct { + Version int `json:"v"` + Token string `json:"token"` + Properties handshakeProperties `json:"properties"` +} + +type handshakeOp struct { + Op int `json:"op"` + Data handshakeData `json:"d"` +} + // Open opens a websocket connection to Discord. func (s *Session) Open() (err error) { s.Lock() defer func() { - s.Unlock() - // Fire OnConnect after we have unlocked the mutex, - // otherwise we may deadlock. - if err == nil && s.OnConnect != nil { - s.OnConnect(s) + if err != nil { + s.Unlock() } }() @@ -49,6 +65,17 @@ func (s *Session) Open() (err error) { return } + err = s.wsConn.WriteJSON(handshakeOp{2, handshakeData{3, s.Token, handshakeProperties{runtime.GOOS, "Discordgo v" + VERSION, "", "", ""}}}) + if err != nil { + return + } + + s.Unlock() + + if s.OnConnect != nil { + s.OnConnect(s) + } + return } @@ -56,14 +83,6 @@ func (s *Session) Open() (err error) { // TODO: Add support for Voice WS/UDP connections func (s *Session) Close() (err error) { s.Lock() - defer func() { - s.Unlock() - // Fire OnDisconnect after we have unlocked the mutex - // otherwise we may deadlock, especially in reconnect logic. - if err == nil && s.OnDisconnect != nil { - s.OnDisconnect(s) - } - }() s.DataReady = false @@ -77,34 +96,15 @@ func (s *Session) Close() (err error) { s.wsConn = nil } + s.Unlock() + + if s.OnDisconnect != nil { + s.OnDisconnect(s) + } + return } -type handshakeProperties struct { - OS string `json:"$os"` - Browser string `json:"$browser"` - Device string `json:"$device"` - Referer string `json:"$referer"` - ReferringDomain string `json:"$referring_domain"` -} - -type handshakeData struct { - Version int `json:"v"` - Token string `json:"token"` - Properties handshakeProperties `json:"properties"` -} - -type handshakeOp struct { - Op int `json:"op"` - Data handshakeData `json:"d"` -} - -// Handshake sends the client data to Discord during websocket initial connection. -func (s *Session) Handshake() error { - data := handshakeOp{2, handshakeData{3, s.Token, handshakeProperties{runtime.GOOS, "Discordgo v" + VERSION, "", "", ""}}} - return s.wsConn.WriteJSON(data) -} - type updateStatusGame struct { Name string `json:"name"` } @@ -137,27 +137,27 @@ func (s *Session) UpdateStatus(idle int, game string) (err error) { usd.Game = &updateStatusGame{game} } - data := updateStatusOp{3, usd} - err = s.wsConn.WriteJSON(data) + err = s.wsConn.WriteJSON(updateStatusOp{3, usd}) return } // Listen starts listening to the websocket connection for events. func (s *Session) Listen() (err error) { - s.Lock() + s.RLock() + if s.wsConn == nil { - s.Unlock() + s.RUnlock() return errors.New("No websocket connection exists.") } if s.listening != nil { - s.Unlock() + s.RUnlock() return errors.New("Already listening to websocket.") } s.listening = make(chan interface{}) - s.Unlock() + s.RUnlock() // Keep a reference, as s.listening can be nilled out. listening := s.listening @@ -562,11 +562,13 @@ func (s *Session) event(messageType int, message []byte) (err error) { return } +type heartbeatOp struct { + Op int `json:"op"` + Data int `json:"d"` +} + func (s *Session) sendHeartbeat() error { - return s.wsConn.WriteJSON(map[string]int{ - "op": 1, - "d": int(time.Now().Unix()), - }) + return s.wsConn.WriteJSON(heartbeatOp{1, int(time.Now().Unix())}) } // heartbeat sends regular heartbeats to Discord so it knows the client From e8d8f0321459d4538bf38ac91cd2e883c6b55a7b Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 09:33:05 -0800 Subject: [PATCH 09/13] Clean up ready timing, fix potential race with starting heartbeats. --- wsapi.go | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/wsapi.go b/wsapi.go index bc0b6ef..b984e18 100644 --- a/wsapi.go +++ b/wsapi.go @@ -214,9 +214,9 @@ func (s *Session) event(messageType int, message []byte) (err error) { switch e.Type { case "READY": - s.DataReady = true var st *Ready if err = unmarshalEvent(e, &st); err == nil { + go s.heartbeat(st.HeartbeatInterval) if s.StateEnabled { s.State.OnReady(st) } @@ -224,7 +224,6 @@ func (s *Session) event(messageType int, message []byte) (err error) { s.OnReady(s, st) } } - go s.heartbeat(st.HeartbeatInterval) if s.OnReady != nil { return } @@ -575,9 +574,21 @@ func (s *Session) sendHeartbeat() error { // is still connected. If you do not send these heartbeats Discord will // disconnect the websocket connection after a few seconds. func (s *Session) heartbeat(i time.Duration) { + s.Lock() + + // We have been closed between the first Ready event, abort. + if s.listening == nil { + s.Unlock() + return + } + // Keep a reference, as s.listening can be nilled out. listening := s.listening + s.DataReady = true + + s.Unlock() + // Send first heartbeat immediately because lag could put the // first heartbeat outside the required heartbeat interval window. err := s.sendHeartbeat() From 8b39278c3ed6e5105181c84d5bebd2a07c4c28a8 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 10:58:13 -0800 Subject: [PATCH 10/13] Get rid of Listen. --- discord.go | 17 --------- discord_test.go | 4 +- examples/api_basic/api_basic.go | 3 -- examples/new_basic/new_basic.go | 2 +- wsapi.go | 68 ++++++++++++++------------------- 5 files changed, 31 insertions(+), 63 deletions(-) diff --git a/discord.go b/discord.go index 6014d87..624aeb9 100644 --- a/discord.go +++ b/discord.go @@ -117,20 +117,3 @@ func New(args ...interface{}) (s *Session, err error) { return } - -// OpenAndListen is a helper method that opens the websocket connection, -// does the required handshake and then immediately begins listening. -// This is the preferred way to start listening for events and is safe -// to be called inside an OnDisconnect handler. -func (s *Session) OpenAndListen() (err error) { - // Open websocket connection. - err = s.Open() - if err != nil { - return - } - - // Listen for events. - go s.Listen() - - return -} diff --git a/discord_test.go b/discord_test.go index 2dde3f7..deed4f4 100644 --- a/discord_test.go +++ b/discord_test.go @@ -199,8 +199,8 @@ func TestOpenClose(t *testing.T) { t.Fatalf("TestClose, New(envToken) returned error: %+v", err) } - if err = d.OpenAndListen(); err != nil { - t.Fatalf("TestClose, d.OpenAndListen failed: %+v", err) + if err = d.Open(); err != nil { + t.Fatalf("TestClose, d.Open failed: %+v", err) } if err = d.Close(); err != nil { diff --git a/examples/api_basic/api_basic.go b/examples/api_basic/api_basic.go index 8c8fcd2..aea4e56 100644 --- a/examples/api_basic/api_basic.go +++ b/examples/api_basic/api_basic.go @@ -40,9 +40,6 @@ func main() { fmt.Println(err) } - // Listen for events. - go dg.Listen() - // Simple way to keep program running until any key press. var input string fmt.Scanln(&input) diff --git a/examples/new_basic/new_basic.go b/examples/new_basic/new_basic.go index 752ce78..43492ec 100644 --- a/examples/new_basic/new_basic.go +++ b/examples/new_basic/new_basic.go @@ -32,7 +32,7 @@ func main() { dg.OnMessageCreate = messageCreate // Open the websocket and begin listening. - dg.OpenAndListen() + dg.Open() // Simple way to keep program running until any key press. var input string diff --git a/wsapi.go b/wsapi.go index b984e18..51432a3 100644 --- a/wsapi.go +++ b/wsapi.go @@ -70,6 +70,11 @@ func (s *Session) Open() (err error) { return } + // Create listening outside of listen, as it needs to happen inside the mutex + // lock. + s.listening = make(chan interface{}) + go s.listen(s.listening) + s.Unlock() if s.OnConnect != nil { @@ -105,6 +110,29 @@ func (s *Session) Close() (err error) { return } +// listen polls the websocket connection for events, it will stop when +// the listening channel is closed, or an error occurs. +func (s *Session) listen(listening <-chan interface{}) { + for { + messageType, message, err := s.wsConn.ReadMessage() + if err != nil { + // There has been an error reading, Close() the websocket so that + // OnDisconnect is fired. + s.Close() + return + } + + select { + case <-listening: + return + default: + go s.event(messageType, message) + } + } + + return +} + type updateStatusGame struct { Name string `json:"name"` } @@ -142,46 +170,6 @@ func (s *Session) UpdateStatus(idle int, game string) (err error) { return } -// Listen starts listening to the websocket connection for events. -func (s *Session) Listen() (err error) { - s.RLock() - - if s.wsConn == nil { - s.RUnlock() - return errors.New("No websocket connection exists.") - } - if s.listening != nil { - s.RUnlock() - return errors.New("Already listening to websocket.") - } - - s.listening = make(chan interface{}) - - s.RUnlock() - - // Keep a reference, as s.listening can be nilled out. - listening := s.listening - - for { - messageType, message, err1 := s.wsConn.ReadMessage() - if err1 != nil { - err = err1 - // Defer so we get better log ordering. - defer s.Close() - return fmt.Errorf("Websocket Listen Error", err) - } - - select { - case <-listening: - return - default: - go s.event(messageType, message) - } - } - - return -} - // Not sure how needed this is and where it would be best to call it. // somewhere. From bf5ca4d69061d82a77fc7c3792b88a3e5add3acb Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 10:59:30 -0800 Subject: [PATCH 11/13] Fix comment. --- discord.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/discord.go b/discord.go index 624aeb9..6b3f6eb 100644 --- a/discord.go +++ b/discord.go @@ -112,8 +112,7 @@ func New(args ...interface{}) (s *Session, err error) { } // The Session is now able to have RestAPI methods called on it. - // It is recommended that you now call OpenAndListen so that events - // will begin to trigger. + // It is recommended that you now call Open() so that events will trigger. return } From d6d6c749ea708e0be85fdb685192234e402050d9 Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 11:11:18 -0800 Subject: [PATCH 12/13] Prevent listen from reading from nulled out web socket. --- wsapi.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/wsapi.go b/wsapi.go index 51432a3..0199f13 100644 --- a/wsapi.go +++ b/wsapi.go @@ -73,7 +73,7 @@ func (s *Session) Open() (err error) { // Create listening outside of listen, as it needs to happen inside the mutex // lock. s.listening = make(chan interface{}) - go s.listen(s.listening) + go s.listen(s.wsConn, s.listening) s.Unlock() @@ -112,9 +112,9 @@ func (s *Session) Close() (err error) { // listen polls the websocket connection for events, it will stop when // the listening channel is closed, or an error occurs. -func (s *Session) listen(listening <-chan interface{}) { +func (s *Session) listen(wsConn *websocket.Conn, listening <-chan interface{}) { for { - messageType, message, err := s.wsConn.ReadMessage() + messageType, message, err := wsConn.ReadMessage() if err != nil { // There has been an error reading, Close() the websocket so that // OnDisconnect is fired. From ddddfa3644707877f57ae8218675b3cdfc6ffffe Mon Sep 17 00:00:00 2001 From: Chris Rhodes Date: Thu, 21 Jan 2016 11:15:44 -0800 Subject: [PATCH 13/13] Prevent heartbeat from sending on nil websocket. --- wsapi.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/wsapi.go b/wsapi.go index 0199f13..342efdd 100644 --- a/wsapi.go +++ b/wsapi.go @@ -204,7 +204,7 @@ func (s *Session) event(messageType int, message []byte) (err error) { case "READY": var st *Ready if err = unmarshalEvent(e, &st); err == nil { - go s.heartbeat(st.HeartbeatInterval) + go s.heartbeat(s.wsConn, s.listening, st.HeartbeatInterval) if s.StateEnabled { s.State.OnReady(st) } @@ -554,32 +554,25 @@ type heartbeatOp struct { Data int `json:"d"` } -func (s *Session) sendHeartbeat() error { - return s.wsConn.WriteJSON(heartbeatOp{1, int(time.Now().Unix())}) +func (s *Session) sendHeartbeat(wsConn *websocket.Conn) error { + return wsConn.WriteJSON(heartbeatOp{1, int(time.Now().Unix())}) } // heartbeat sends regular heartbeats to Discord so it knows the client // is still connected. If you do not send these heartbeats Discord will // disconnect the websocket connection after a few seconds. -func (s *Session) heartbeat(i time.Duration) { - s.Lock() - - // We have been closed between the first Ready event, abort. - if s.listening == nil { - s.Unlock() +func (s *Session) heartbeat(wsConn *websocket.Conn, listening <-chan interface{}, i time.Duration) { + if listening == nil || wsConn == nil { return } - // Keep a reference, as s.listening can be nilled out. - listening := s.listening - + s.Lock() s.DataReady = true - s.Unlock() // Send first heartbeat immediately because lag could put the // first heartbeat outside the required heartbeat interval window. - err := s.sendHeartbeat() + err := s.sendHeartbeat(wsConn) if err != nil { fmt.Println("Error sending initial heartbeat:", err) return @@ -589,7 +582,7 @@ func (s *Session) heartbeat(i time.Duration) { for { select { case <-ticker.C: - err := s.sendHeartbeat() + err := s.sendHeartbeat(wsConn) if err != nil { fmt.Println("Error sending heartbeat:", err) return