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(VoiceConnection): do not reconnect if close code is 4014 (Disconnected) #1544

Closed
wants to merge 2 commits into from

Conversation

babiabeo
Copy link
Contributor

@babiabeo babiabeo commented Sep 22, 2024

Fixes: #1437

As described in the docs, voice should not reconnect if close code is 4014 (Disconnected).

@Snazzah
Copy link
Contributor

Snazzah commented Sep 23, 2024

I'll test this later but there are some possible issues:

  1. .disconnect() with reconnecting = false will update the voice state and disconnect the client, this is only bad because things like voice server updates (changing the voice region) would also give a 4014, which we wouldn't want to full disconnect from and would rather wait for the next VOICE_SERVER_UPDATE event. On a 4014, data.endpoint would be set to null which would signal to .connect that it should wait for an event:
if (!data.endpoint) {
  return; // Endpoint null, wait next update.
}
  1. VOICE_STATE_UPDATEs would already update/disconnect the connection if the client was kicked or if the channel was removed.
  2. A failed reconnect would properly disconnect the client if it came to it, emitting the disconnect event.

I've tried kicking the client from a VC and it seemed to disconnect just fine because of the VOICE_STATE_UPDATE, maybe there should be a separate timeout for that update waiting.

@bsian03
Copy link
Collaborator

bsian03 commented Sep 23, 2024

I didn't read that essay, but ideally the only change here would be just to disconnect, existing functions shouldn't change unless there is a clear benefit or if it is absolutely necessary nvm read context and understand this a bit more now

a 4014 here in this case means you shouldn't reconnect to that websocket. reconnecting to the voice channel on the other hand is an entirely different question. Assuming the disconnect was not intentional client side, we should reconnect to the voice channel on a new websocket

@babiabeo
Copy link
Contributor Author

I see. It seems that I misunderstood the meaning of reconnecting here. Will close this PR for better solution.

@babiabeo babiabeo closed this Sep 23, 2024
@babiabeo babiabeo deleted the fix-disconnect-event branch September 23, 2024 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

VoiceConnection disconnect event not emitting
3 participants