Repository: discordjs/voice (ts)

Issue:
Show friendlier error when no encryption packages are installed
At the moment, if you try to use the library without any encryption packages installed and try to play an audio resource, you get a rather unfriendly error:

```
TypeError: secretbox.methods.close is not a function
```

This could definitely be improved!

Pull request:
feat(Secretbox): throw fallback error when no encryption package installed
**Please describe the changes this PR makes and why it should be merged:**

Resolves #128.

If no valid encryption package is installed, the following error is now thrown (rather than the obscure error shown in #128):

```
    throw new Error(`Cannot play audio as no valid encryption package is installed.
    ^

Error: Cannot play audio as no valid encryption package is installed.
- Install sodium, libsodium-wrappers, or tweetnacl.
- Use the generateDependencyReport() function for more information.
```


**Status and versioning classification:**

- Code changes have been tested against the Discord API, or there are no code changes

Patch:
diff --git a/examples/music-bot/src/music/subscription.ts b/examples/music-bot/src/music/subscription.ts
index ab0c4cc..932c9fd 100644
--- a/examples/music-bot/src/music/subscription.ts
+++ b/examples/music-bot/src/music/subscription.ts
@@ -47,12 +47,12 @@ export class MusicSubscription {
 						this.voiceConnection.destroy();
 						// Probably removed from voice channel
 					}
-				} else if (this.voiceConnection.reconnectAttempts < 5) {
+				} else if (this.voiceConnection.rejoinAttempts < 5) {
 					/*
 						The disconnect in this case is recoverable, and we also have <5 repeated attempts so we will reconnect.
 					*/
-					await wait((this.voiceConnection.reconnectAttempts + 1) * 5_000);
-					this.voiceConnection.reconnect();
+					await wait((this.voiceConnection.rejoinAttempts + 1) * 5_000);
+					this.voiceConnection.rejoin();
 				} else {
 					/*
 						The disconnect in this case may be recoverable, but we have no more remaining attempts - destroy.
diff --git a/src/VoiceConnection.ts b/src/VoiceConnection.ts
index b7b1acf..bb9cac7 100644
--- a/src/VoiceConnection.ts
+++ b/src/VoiceConnection.ts
@@ -66,6 +66,10 @@ export enum VoiceConnectionDisconnectReason {
 	 * When a VOICE_SERVER_UPDATE packet is received with a null endpoint, causing the connection to be severed.
 	 */
 	EndpointRemoved,
+	/**
+	 * When a manual disconnect was requested.
+	 */
+	Manual,
 }
 
 /**
@@ -160,10 +164,10 @@ export type VoiceConnectionEvents = {
  */
 export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 	/**
-	 * The number of consecutive reconnect attempts. Initially 0, and increments for each reconnect.
+	 * The number of consecutive rejoin attempts. Initially 0, and increments for each rejoin.
 	 * When a connection is successfully established, it resets to 0.
 	 */
-	public reconnectAttempts: number;
+	public rejoinAttempts: number;
 
 	/**
 	 * The state of the voice connection
@@ -200,7 +204,7 @@ export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 		super();
 
 		this.debug = debug ? (message: string) => this.emit('debug', message) : null;
-		this.reconnectAttempts = 0;
+		this.rejoinAttempts = 0;
 
 		this.onNetworkingClose = this.onNetworkingClose.bind(this);
 		this.onNetworkingStateChange = this.onNetworkingStateChange.bind(this);
@@ -251,7 +255,7 @@ export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 		}
 
 		if (newState.status === VoiceConnectionStatus.Ready) {
-			this.reconnectAttempts = 0;
+			this.rejoinAttempts = 0;
 		}
 
 		// If destroyed, the adapter can also be destroyed so it can be cleaned up by the user
@@ -375,7 +379,7 @@ export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 				...this.state,
 				status: VoiceConnectionStatus.Signalling,
 			};
-			this.reconnectAttempts++;
+			this.rejoinAttempts++;
 			if (!this.state.adapter.sendPayload(createJoinVoiceChannelPayload(this.joinConfig))) {
 				this.state = {
 					...this.state,
@@ -482,34 +486,65 @@ export class VoiceConnection extends TypedEmitter<VoiceConnectionEvents> {
 	}
 
 	/**
-	 * Attempts to reconnect the VoiceConnection if it is in the Disconnected state.
+	 * Disconnects the VoiceConnection, allowing the possibility of rejoining later on.
+	 * @returns - true if the connection was successfully disconnected.
+	 */
+	public disconnect() {
+		if (
+			this.state.status === VoiceConnectionStatus.Destroyed ||
+			this.state.status === VoiceConnectionStatus.Signalling
+		) {
+			return false;
+		}
+		if (!this.state.adapter.sendPayload(createJoinVoiceChannelPayload(this.joinConfig))) {
+			this.state = {
+				adapter: this.state.adapter,
+				subscription: this.state.subscription,
+				status: VoiceConnectionStatus.Disconnected,
+				reason: VoiceConnectionDisconnectReason.AdapterUnavailable,
+			};
+			return false;
+		}
+		this.state = {
+			adapter: this.state.adapter,
+			reason: VoiceConnectionDisconnectReason.Manual,
+			status: VoiceConnectionStatus.Disconnected,
+		};
+		return true;
+	}
+
+	/**
+	 * Attempts to rejoin (better explanation soon:tm:)
 	 *
 	 * @remarks
-	 * Calling this method successfully will automatically increment the `reconnectAttempts` counter,
+	 * Calling this method successfully will automatically increment the `rejoinAttempts` counter,
 	 * which you can use to inform whether or not you'd like to keep attempting to reconnect your
 	 * voice connection.
 	 *
 	 * A state transition from Disconnected to Signalling will be observed when this is called.
 	 */
-	public reconnect() {
-		if (this.state.status !== VoiceConnectionStatus.Disconnected) {
+	public rejoin(joinConfig?: Omit<JoinConfig, 'guildId'>) {
+		if (this.state.status === VoiceConnectionStatus.Destroyed) {
 			return false;
 		}
 
-		this.reconnectAttempts++;
-		if (!this.state.adapter.sendPayload(createJoinVoiceChannelPayload(this.joinConfig))) {
+		this.rejoinAttempts++;
+		Object.assign(this.joinConfig, joinConfig);
+		if (this.state.adapter.sendPayload(createJoinVoiceChannelPayload(this.joinConfig))) {
 			this.state = {
 				...this.state,
-				status: VoiceConnectionStatus.Disconnected,
+				status: VoiceConnectionStatus.Signalling,
 			};
-			return false;
+			return true;
 		}
 
 		this.state = {
-			...this.state,
-			status: VoiceConnectionStatus.Signalling,
+			adapter: this.state.adapter,
+			subscription: this.state.subscription,
+			status: VoiceConnectionStatus.Disconnected,
+			reason: VoiceConnectionDisconnectReason.AdapterUnavailable,
 		};
-		return true;
+		return false;
 	}
 
 	/**
@@ -592,7 +627,13 @@ export function createVoiceConnection(joinConfig: JoinConfig, options: CreateVoi
 	const payload = createJoinVoiceChannelPayload(joinConfig);
 	const existing = getVoiceConnection(joinConfig.guildId);
 	if (existing && existing.state.status !== VoiceConnectionStatus.Destroyed) {
-		if (!existing.state.adapter.sendPayload(payload)) {
+		if (existing.state.status === VoiceConnectionStatus.Disconnected) {
+			existing.rejoin({
+				channelId: joinConfig.channelId,
+				selfDeaf: joinConfig.selfDeaf,
+				selfMute: joinConfig.selfMute,
+			});
+		} else if (!existing.state.adapter.sendPayload(payload)) {
 			existing.state = {
 				...existing.state,
 				status: VoiceConnectionStatus.Disconnected,
diff --git a/src/util/Secretbox.ts b/src/util/Secretbox.ts
index 08e9358..3271da2 100644
--- a/src/util/Secretbox.ts
+++ b/src/util/Secretbox.ts
@@ -20,7 +20,19 @@ const libs = {
 	}),
 };
 
-const methods: Record<any, any> = {};
+const fallbackError = () => {
+	throw new Error(
+		`Cannot play audio as no valid encryption package is installed.
+- Install sodium, libsodium-wrappers, or tweetnacl.
+- Use the generateDependencyReport() function for more information.\n`,
+	);
+};
+
+const methods: Record<any, any> = {
+	open: fallbackError,
+	close: fallbackError,
+	random: fallbackError,
+};
 
 void (async () => {
 	for (const libName of Object.keys(libs)) {


Test patch:
diff --git a/src/__tests__/VoiceConnection.test.ts b/src/__tests__/VoiceConnection.test.ts
index fe3264f..07b9c00 100644
--- a/src/__tests__/VoiceConnection.test.ts
+++ b/src/__tests__/VoiceConnection.test.ts
@@ -3,6 +3,7 @@ import {
 	createVoiceConnection,
 	VoiceConnection,
 	VoiceConnectionConnectingState,
+	VoiceConnectionDisconnectReason,
 	VoiceConnectionSignallingState,
 	VoiceConnectionStatus,
 } from '../VoiceConnection';
@@ -133,6 +134,44 @@ describe('createVoiceConnection', () => {
 		expect(stateSetter).not.toHaveBeenCalled();
 	});
 
+	test('Calls rejoin() on existing disconnected connection', () => {
+		const mockPayload = Symbol('mock') as any;
+
+		DataStore.createJoinVoiceChannelPayload.mockImplementation(() => mockPayload);
+
+		const existingAdapter = createFakeAdapter();
+		const existingJoinConfig = createJoinConfig();
+		const existingVoiceConnection = new VoiceConnection(existingJoinConfig, {
+			debug: false,
+			adapterCreator: existingAdapter.creator,
+		});
+		existingVoiceConnection.state = {
+			status: VoiceConnectionStatus.Disconnected,
+			adapter: existingAdapter,
+			reason: VoiceConnectionDisconnectReason.EndpointRemoved,
+		};
+
+		const rejoinSpy = jest.spyOn(existingVoiceConnection, 'rejoin');
+
+		DataStore.getVoiceConnection.mockImplementation((guildId) =>
+			guildId === existingJoinConfig.guildId ? existingVoiceConnection : null,
+		);
+
+		const newAdapter = createFakeAdapter();
+		const newJoinConfig = createJoinConfig();
+		const { guildId, ...rejoinConfig } = newJoinConfig;
+		const newVoiceConnection = createVoiceConnection(newJoinConfig, {
+			debug: false,
+			adapterCreator: newAdapter.creator,
+		});
+		expect(DataStore.getVoiceConnection).toHaveBeenCalledWith(newJoinConfig.guildId);
+		expect(DataStore.trackVoiceConnection).not.toHaveBeenCalled();
+		expect(DataStore.untrackVoiceConnection).not.toHaveBeenCalled();
+		expect(newAdapter.creator).not.toHaveBeenCalled();
+		expect(rejoinSpy).toHaveBeenCalledWith(rejoinConfig);
+		expect(newVoiceConnection).toBe(existingVoiceConnection);
+	});
+
 	test('Reconfiguring existing connection with adapter failure', () => {
 		const mockPayload = Symbol('mock') as any;
 
@@ -317,7 +356,7 @@ describe('VoiceConnection#onNetworkingClose', () => {
 		expect(adapter.sendPayload).not.toHaveBeenCalled();
 	});
 
-	test('Attempts reconnect for codes != 4014', () => {
+	test('Attempts rejoin for codes != 4014', () => {
 		const dummyPayload = Symbol('dummy') as any;
 		const { voiceConnection, adapter, joinConfig } = createFakeVoiceConnection();
 		DataStore.createJoinVoiceChannelPayload.mockImplementation((config) =>
@@ -326,10 +365,10 @@ describe('VoiceConnection#onNetworkingClose', () => {
 		voiceConnection['onNetworkingClose'](1234);
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Signalling);
 		expect(adapter.sendPayload).toHaveBeenCalledWith(dummyPayload);
-		expect(voiceConnection.reconnectAttempts).toBe(1);
+		expect(voiceConnection.rejoinAttempts).toBe(1);
 	});
 
-	test('Attempts reconnect for codes != 4014 (with adapter failure)', () => {
+	test('Attempts rejoin for codes != 4014 (with adapter failure)', () => {
 		const dummyPayload = Symbol('dummy') as any;
 		const { voiceConnection, adapter, joinConfig } = createFakeVoiceConnection();
 		DataStore.createJoinVoiceChannelPayload.mockImplementation((config) =>
@@ -339,7 +378,7 @@ describe('VoiceConnection#onNetworkingClose', () => {
 		voiceConnection['onNetworkingClose'](1234);
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Disconnected);
 		expect(adapter.sendPayload).toHaveBeenCalledWith(dummyPayload);
-		expect(voiceConnection.reconnectAttempts).toBe(1);
+		expect(voiceConnection.rejoinAttempts).toBe(1);
 	});
 });
 
@@ -437,17 +476,50 @@ describe('VoiceConnection#destroy', () => {
 	});
 });
 
-describe('VoiceConnection#reconnect', () => {
-	test('Does nothing in a non-disconnected state', () => {
+describe('VoiceConnection#disconnect', () => {
+	test('Fails in Destroyed and Signalling states', () => {
 		const { voiceConnection, adapter } = createFakeVoiceConnection();
-		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Signalling);
-		expect(voiceConnection.reconnect()).toBe(false);
-		expect(voiceConnection.reconnectAttempts).toBe(0);
-		expect(adapter.sendPayload).not.toHaveBeenCalled();
+		voiceConnection.state = { status: VoiceConnectionStatus.Destroyed };
+		expect(voiceConnection.disconnect()).toBe(false);
+		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Destroyed);
+
+		voiceConnection.state = { status: VoiceConnectionStatus.Signalling, adapter };
+		expect(voiceConnection.disconnect()).toBe(false);
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Signalling);
 	});
 
-	test('Reconnects in a disconnected state', () => {
+	test('Disconnects - available adapter', () => {
+		const { voiceConnection, adapter } = createFakeVoiceConnection();
+		voiceConnection.state = {
+			status: VoiceConnectionStatus.Ready,
+			adapter,
+			networking: new Networking.Networking({} as any, false),
+		};
+		expect(voiceConnection.disconnect()).toBe(true);
+		expect(voiceConnection.state).toMatchObject({
+			status: VoiceConnectionStatus.Disconnected,
+			reason: VoiceConnectionDisconnectReason.Manual,
+		});
+	});
+
+	test('Disconnects - unavailable adapter', () => {
+		const { voiceConnection, adapter } = createFakeVoiceConnection();
+		voiceConnection.state = {
+			status: VoiceConnectionStatus.Ready,
+			adapter,
+			networking: new Networking.Networking({} as any, false),
+		};
+		adapter.sendPayload.mockImplementation(() => false);
+		expect(voiceConnection.disconnect()).toBe(false);
+		expect(voiceConnection.state).toMatchObject({
+			status: VoiceConnectionStatus.Disconnected,
+			reason: VoiceConnectionDisconnectReason.AdapterUnavailable,
+		});
+	});
+});
+
+describe('VoiceConnection#rejoin', () => {
+	test('Rejoins in a disconnected state', () => {
 		const dummy = Symbol('dummy') as any;
 		DataStore.createJoinVoiceChannelPayload.mockImplementation(() => dummy);
 
@@ -455,10 +527,11 @@ describe('VoiceConnection#reconnect', () => {
 		voiceConnection.state = {
 			...(voiceConnection.state as VoiceConnectionSignallingState),
 			status: VoiceConnectionStatus.Disconnected,
+			reason: VoiceConnectionDisconnectReason.WebSocketClose,
 			closeCode: 1000,
 		};
-		expect(voiceConnection.reconnect()).toBe(true);
-		expect(voiceConnection.reconnectAttempts).toBe(1);
+		expect(voiceConnection.rejoin()).toBe(true);
+		expect(voiceConnection.rejoinAttempts).toBe(1);
 		expect(adapter.sendPayload).toHaveBeenCalledWith(dummy);
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Signalling);
 	});
@@ -471,11 +544,12 @@ describe('VoiceConnection#reconnect', () => {
 		voiceConnection.state = {
 			...(voiceConnection.state as VoiceConnectionSignallingState),
 			status: VoiceConnectionStatus.Disconnected,
+			reason: VoiceConnectionDisconnectReason.WebSocketClose,
 			closeCode: 1000,
 		};
 		adapter.sendPayload.mockReturnValue(false);
-		expect(voiceConnection.reconnect()).toBe(false);
-		expect(voiceConnection.reconnectAttempts).toBe(1);
+		expect(voiceConnection.rejoin()).toBe(false);
+		expect(voiceConnection.rejoinAttempts).toBe(1);
 		expect(adapter.sendPayload).toHaveBeenCalledWith(dummy);
 		expect(voiceConnection.state.status).toBe(VoiceConnectionStatus.Disconnected);
 	});
diff --git a/src/util/__tests__/Secretbox.test.ts b/src/util/__tests__/Secretbox.test.ts
new file mode 100644
index 0000000..9b2075a
--- /dev/null
+++ b/src/util/__tests__/Secretbox.test.ts
@@ -0,0 +1,15 @@
+import { methods } from '../Secretbox';
+jest.mock(
+	'tweetnacl',
+	() => ({
+		secretbox: {
+			// eslint-disable-next-line @typescript-eslint/no-empty-function
+			open() {},
+		},
+	}),
+	{ virtual: true },
+);
+
+test('Does not throw error with a package installed', () => {
+	expect(() => methods.open()).not.toThrowError();
+});