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.

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

Fix the problem of reconnecting when moved to another VC #1271

Merged
merged 5 commits into from
Dec 15, 2022
Merged

Fix the problem of reconnecting when moved to another VC #1271

merged 5 commits into from
Dec 15, 2022

Conversation

nonnonstop
Copy link
Contributor

Fixes #849.

Description

When the bot was moved to another vc, the connection was disconnected with 4014 Disconnected.

Then, VOICE_SERVER_UPDATE is received after 4014 Disconnected.

2022/10/30 14:47:23 [DG2] voice.go:356:wsListen() received 4014 manual disconnection
2022/10/30 14:47:23 [DG2] voice.go:167:Close() called
2022/10/30 14:47:23 [DG2] voice.go:176:Close() closing v.close
2022/10/30 14:47:23 [DG2] voice.go:182:Close() closing udp
2022/10/30 14:47:23 [DG3] wsapi.go:547:onEvent() Op: 0, Seq: 6, Type: VOICE_STATE_UPDATE, Data: [hidden]
2022/10/30 14:47:23 [DG3] wsapi.go:547:onEvent() Op: 0, Seq: 7, Type: VOICE_SERVER_UPDATE, Data: {"token":"[hidden]","guild_id":"[hidden]","endpoint":"brazil7541.discord.media:443"}
2022/10/30 14:47:23 [DG2] wsapi.go:763:onVoiceServerUpdate() called

However, the VoiceConnection has been deleted when the VOICE_SERVER_UPDATE is received due to the following code. (L364)

discordgo/voice.go

Lines 355 to 370 in afab840

if websocket.IsCloseError(err, 4014) {
v.log(LogInformational, "received 4014 manual disconnection")
// Abandon the voice WS connection
v.Lock()
v.wsConn = nil
v.Unlock()
v.session.Lock()
delete(v.session.VoiceConnections, v.GuildID)
v.session.Unlock()
v.Close()
return
}

Since the VoiceConnection does not exist, onVoiceServerUpdate does not reconnect.

discordgo/wsapi.go

Lines 761 to 772 in afab840

func (s *Session) onVoiceServerUpdate(st *VoiceServerUpdate) {
s.log(LogInformational, "called")
s.RLock()
voice, exists := s.VoiceConnections[st.GuildID]
s.RUnlock()
// If no VoiceConnection exists, just skip this
if !exists {
return
}

This is the cause of #849.

How to fix

Delay the time between receiving 4014 Disconnected and deleting the connection.
If VOICE_SERVER_UPDATE is received during that time, the connection is not deleted.

2022/10/30 15:17:13 [DG2] voice.go:356:wsListen() received 4014 manual disconnection
2022/10/30 15:17:13 [DG3] wsapi.go:547:onEvent() Op: 0, Seq: 7, Type: VOICE_SERVER_UPDATE, Data: {"token":"[hidden]","guild_id":"[hidden]","endpoint":"brazil2241.discord.media:443"}
2022/10/30 15:17:13 [DG2] wsapi.go:763:onVoiceServerUpdate() called
2022/10/30 15:17:13 [DG2] voice.go:167:Close() called
2022/10/30 15:17:13 [DG2] voice.go:176:Close() closing v.close
2022/10/30 15:17:13 [DG2] voice.go:182:Close() closing udp
2022/10/30 15:17:13 [DG2] voice.go:280:open() called
2022/10/30 15:17:13 [DG2] voice.go:306:open() connecting to voice endpoint wss://brazil2241.discord.media:443
2022/10/30 15:17:13 [DG2] voice.go:348:wsListen() called
2022/10/30 15:17:13 [DG3] voice.go:417:onEvent() received: {"op":8,"d":{"v":1,"heartbeat_interval":55000}}
2022/10/30 15:17:13 [DG3] voice.go:494:onEvent() unknown voice operation, 8, {"v":1,"heartbeat_interval":55000}
2022/10/30 15:17:14 [DG2] voice.go:372:wsListen() successfully reconnected due to code 4014

If not (e.g. kicked by admin), the connection is deleted.

2022/10/30 15:18:24 [DG2] voice.go:356:wsListen() received 4014 manual disconnection
2022/10/30 15:18:29 [DG2] voice.go:376:wsListen() disconnect due to code 4014
2022/10/30 15:18:29 [DG2] voice.go:167:Close() called
2022/10/30 15:18:29 [DG2] voice.go:176:Close() closing v.close
2022/10/30 15:18:29 [DG2] voice.go:182:Close() closing udp

@FedorLap2006 FedorLap2006 added fix Pull requests and issues related to bug fixes and structural inconsistencies rest REST API related issues and pull requests voice Voice related issues and pull requests and removed rest REST API related issues and pull requests labels Oct 30, 2022
Copy link
Collaborator

@FedorLap2006 FedorLap2006 left a comment

Choose a reason for hiding this comment

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

LGTM, however I'm not fully sure about this for based retry solution

P.S. Thanks for fixing such longstanding issue!

voice.go Outdated Show resolved Hide resolved
voice.go Outdated Show resolved Hide resolved
voice.go Outdated Show resolved Hide resolved
nonnonstop and others added 2 commits November 7, 2022 12:41
Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
Copy link
Collaborator

@FedorLap2006 FedorLap2006 left a comment

Choose a reason for hiding this comment

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

Sorry for a long wait.

voice.go Outdated Show resolved Hide resolved
nonnonstop and others added 2 commits December 3, 2022 09:27
Co-authored-by: Fedor Lapshin <fe.lap.prog@gmail.com>
Copy link
Collaborator

@FedorLap2006 FedorLap2006 left a comment

Choose a reason for hiding this comment

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

LGTM.

P.S. About the To Do comment. I've marked this loop, since we will probably replace it with a proper solution, using our event system, sometime in the future, alongisde other unrefactored stuff.

@FedorLap2006
Copy link
Collaborator

FedorLap2006 commented Dec 15, 2022

Anyhow, thank you for your work and patience.

@FedorLap2006 FedorLap2006 merged commit 9effc92 into bwmarrin:master Dec 15, 2022
@nonnonstop nonnonstop deleted the fix-vc-move branch December 17, 2022 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests and issues related to bug fixes and structural inconsistencies voice Voice related issues and pull requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use VoiceConnection when the bot is moved to another vc by a User
2 participants