Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(wsapi): Gracefully close the websocket connection #1347

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(wsapi): Gracefully close the websocket connection #1347

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 36 additions & 16 deletions wsapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -921,24 +921,11 @@ func (s *Session) CloseWithCode(closeCode int) (err error) {
// this should force stop any reconnecting voice channels too

if s.wsConn != nil {

s.log(LogInformational, "sending close frame")
// To cleanly close a connection, a client should send a close
// frame and wait for the server to close the connection.
s.wsMutex.Lock()
err := s.wsConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(closeCode, ""))
s.wsMutex.Unlock()
if err != nil {
s.log(LogInformational, "error closing websocket, %s", err)
}

// TODO: Wait for Discord to actually close the connection.
time.Sleep(1 * time.Second)
err = s.doCloseHandshake(closeCode)

s.log(LogInformational, "closing gateway websocket")
err = s.wsConn.Close()
if err != nil {
s.log(LogInformational, "error closing websocket, %s", err)
if err1 := s.wsConn.Close(); err1 != nil && err == nil {
err = err1
}

s.wsConn = nil
Expand All @@ -951,3 +938,36 @@ func (s *Session) CloseWithCode(closeCode int) (err error) {

return
}

func (s *Session) doCloseHandshake(closeCode int) error {
FedorLap2006 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a bit of thinking. It might be a good idea to also move this function away from Session into util.go, with websocket prefixed name (e.g. doWebsocketCloseHandshake). Since we can reuse it in our voice websocket handling code.

s.log(LogInformational, "sending close frame")

// To cleanly close a connection, a client should send a close
// frame and wait for the server to close the connection.
s.wsMutex.Lock()
err := s.wsConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(closeCode, ""))
s.wsMutex.Unlock()
if err != nil {
s.log(LogInformational, "error closing websocket, %s", err)
return err
}

// Wait for Discord to actually close the connection.
// To prevent ping, pong and close handlers from setting the read deadline,
Copy link
Collaborator

@FedorLap2006 FedorLap2006 Apr 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setting read deadline

I'm not quite sure what do you refer to. The handlers in websocket by default don't extend the read deadline. And we don't override them (well, close is overriden, but is noop)

// set these handlers to the default.
s.wsConn.SetPingHandler(nil)
s.wsConn.SetPongHandler(nil)
s.wsConn.SetCloseHandler(nil)
s.wsConn.SetReadDeadline(time.Now().Add(5 * time.Second))
for {
if _, _, err = s.wsConn.NextReader(); err != nil {
var closeError *websocket.CloseError
if !errors.As(err, &closeError) {
s.log(LogInformational, "error closing websocket, %s", err)
return err
}
s.log(LogInformational, "received close frame")
return nil
}
}
}