Repository: discordjs/voice
Pull request #139 (2021-06-25 06:28:25)

fix(VoiceConnection): stuck on signalling state when rejoining the same channel on ready state
**Please describe the changes this PR makes and why it should be merged:**

When using `<VoiceConnection>.rejoin()` with the same channel on ready state, the networking state is not changed. This makes the connection state stuck on the signalling status.

**Status and versioning classification:**
- Code changes have been tested against the Discord API, or there are no code changes

<!--
Please move lines that apply to you out of the comment:
- This PR changes the library's interface (methods or parameters added)
- This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
- This PR **only** includes non-code changes, like changes to documentation, README, etc.
-->


codecov[bot]: # [Codecov](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs) Report
> Merging [#139](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs) (f81922d) into [main](https://codecov.io/gh/discordjs/voice/commit/408999d78962b860f3dd712716c8c76e4464dc37?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs) (408999d) will **increase** coverage by `0.03%`.
> The diff coverage is `83.33%`.

[![Impacted file tree graph](https://codecov.io/gh/discordjs/voice/pull/139/graphs/tree.svg?width=650&height=150&src=pr&token=u7oQ23UoxX&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs)](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs)

```diff
@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   67.63%   67.67%   +0.03%     
==========================================
  Files          19       19              
  Lines         893      897       +4     
  Branches      212      215       +3     
==========================================
+ Hits          604      607       +3     
- Misses        287      288       +1     
  Partials        2        2              
```


| [Impacted Files](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs) | Coverage Δ | |
|---|---|---|
| [src/VoiceConnection.ts](https://codecov.io/gh/discordjs/voice/pull/139/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs#diff-c3JjL1ZvaWNlQ29ubmVjdGlvbi50cw==) | `85.18% <83.33%> (-0.32%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs). Last update [408999d...f81922d](https://codecov.io/gh/discordjs/voice/pull/139?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=discordjs).

skick1234: > If there is a new voice server when Discord receives the new voice state, the voice connection automatically re-enters the connecting state even if it's already in the Ready state.

I read the signalling status description so I thought it's necessary to check it.

Anyway, I removed it :>


Patch:
diff --git a/src/VoiceConnection.ts b/src/VoiceConnection.ts
index 24d2ffc..53bf94d 100644
--- a/src/VoiceConnection.ts
+++ b/src/VoiceConnection.ts
@@ -531,13 +531,17 @@ export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 			return false;
 		}
 
-		this.rejoinAttempts++;
+		const notReady = this.state.status !== VoiceConnectionStatus.Ready;
+
+		if (notReady) this.rejoinAttempts++;
 		Object.assign(this.joinConfig, joinConfig);
 		if (this.state.adapter.sendPayload(createJoinVoiceChannelPayload(this.joinConfig))) {
-			this.state = {
-				...this.state,
-				status: VoiceConnectionStatus.Signalling,
-			};
+			if (notReady) {
+				this.state = {
+					...this.state,
+					status: VoiceConnectionStatus.Signalling,
+				};
+			}
 			return true;
 		}
 


Test patch:
diff --git a/src/__tests__/VoiceConnection.test.ts b/src/__tests__/VoiceConnection.test.ts
index 07b9c00..1066c4e 100644
--- a/src/__tests__/VoiceConnection.test.ts
+++ b/src/__tests__/VoiceConnection.test.ts
@@ -4,6 +4,7 @@ import {
 	VoiceConnection,
 	VoiceConnectionConnectingState,
 	VoiceConnectionDisconnectReason,
+	VoiceConnectionReadyState,
 	VoiceConnectionSignallingState,
 	VoiceConnectionStatus,
 } from '../VoiceConnection';
@@ -536,6 +537,21 @@ describe('VoiceConnection#rejoin', () => {
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Signalling);
 	});
 
+	test('Rejoins in a ready state', () => {
+		const dummy = Symbol('dummy') as any;
+		DataStore.createJoinVoiceChannelPayload.mockImplementation(() => dummy);
+
+		const { voiceConnection, adapter } = createFakeVoiceConnection();
+		voiceConnection.state = {
+			...(voiceConnection.state as VoiceConnectionReadyState),
+			status: VoiceConnectionStatus.Ready,
+		};
+		expect(voiceConnection.rejoin()).toBe(true);
+		expect(voiceConnection.rejoinAttempts).toBe(0);
+		expect(adapter.sendPayload).toHaveBeenCalledWith(dummy);
+		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Ready);
+	});
+
 	test('Stays in the disconnected state when the adapter fails', () => {
 		const dummy = Symbol('dummy') as any;
 		DataStore.createJoinVoiceChannelPayload.mockImplementation(() => dummy);