Repository: discordjs/voice (ts)

Issue:
Refactor to use AbortController in entersState
- Should up minimum Node.js version to v16
- Be backwards-compatible - if a time is provided in entersState, create an AbortController from that

Pull request:
feat(Util): support AbortSignal in entersState
**Please describe the changes this PR makes and why it should be merged:**

Resolves #169 

Allows users to pass in an AbortSignal to `entersState` if they'd like more control than just a timeout over cancelling the operation. Still allows for passing a delay as a number, but the minimum Node.js version has been upped to v16.

```ts
// Still backwards-compatible
await entersState(vc, VoiceConnectionStatus.Ready, 1000);

// New support for AbortSignals
// entersState will abort once 'someEvent' is emitted
const ac = new AbortController();
eventEmitter.once('someEvent', () => ac.abort());
await entersState(vc, VoiceConnectionStatus.Ready, ac.signal);
```

**Status and versioning classification:**

- Code changes have been tested against the Discord API, or there are no code changes
- This PR changes the library's interface (methods or parameters added)
- This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

Patch:
diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml
index 96685e2..7382a03 100644
--- a/.github/workflows/docs.yml
+++ b/.github/workflows/docs.yml
@@ -4,7 +4,7 @@ jobs:
   docs:
     strategy:
       matrix:
-        node: ['14', '16']
+        node: ['16']
     name: Documentation (Node v${{ matrix.node }})
     runs-on: ubuntu-latest
     steps:
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 7d441f8..3d66f46 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -4,7 +4,7 @@ jobs:
   test:
     strategy:
       matrix:
-        node: ['14', '16']
+        node: ['16']
     name: ESLint (Node v${{ matrix.node }})
     runs-on: ubuntu-latest
     steps:
diff --git a/package-lock.json b/package-lock.json
index 851b1a2..12a446c 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -23,7 +23,7 @@
 				"@commitlint/config-angular": "^12.1.4",
 				"@favware/rollup-type-bundler": "^1.0.2",
 				"@types/jest": "^26.0.23",
-				"@types/node": "^15.12.2",
+				"@types/node": "^16.4.13",
 				"@typescript-eslint/eslint-plugin": "^4.26.1",
 				"@typescript-eslint/parser": "^4.26.1",
 				"babel-jest": "^27.0.2",
@@ -39,6 +39,9 @@
 				"prettier": "^2.3.1",
 				"typedoc": "^0.20.34",
 				"typescript": "~4.2.2"
+			},
+			"engines": {
+				"node": ">=16.0.0"
 			}
 		},
 		"node_modules/@babel/code-frame": {
@@ -2787,9 +2790,9 @@
 			"dev": true
 		},
 		"node_modules/@types/node": {
-			"version": "15.12.2",
-			"resolved": "https://registry.npmjs.org/@types/node/-/node-15.12.2.tgz",
-			"integrity": "sha512-zjQ69G564OCIWIOHSXyQEEDpdpGl+G348RAKY0XXy9Z5kU9Vzv1GMNnkar/ZJ8dzXB3COzD9Mo9NtRZ4xfgUww=="
+			"version": "16.4.13",
+			"resolved": "https://registry.npmjs.org/@types/node/-/node-16.4.13.tgz",
+			"integrity": "sha512-bLL69sKtd25w7p1nvg9pigE4gtKVpGTPojBFLMkGHXuUgap2sLqQt2qUnqmVCDfzGUL0DRNZP+1prIZJbMeAXg=="
 		},
 		"node_modules/@types/normalize-package-data": {
 			"version": "2.4.0",
@@ -12953,9 +12956,9 @@
 			"dev": true
 		},
 		"@types/node": {
-			"version": "15.12.2",
-			"resolved": "https://registry.npmjs.org/@types/node/-/node-15.12.2.tgz",
-			"integrity": "sha512-zjQ69G564OCIWIOHSXyQEEDpdpGl+G348RAKY0XXy9Z5kU9Vzv1GMNnkar/ZJ8dzXB3COzD9Mo9NtRZ4xfgUww=="
+			"version": "16.4.13",
+			"resolved": "https://registry.npmjs.org/@types/node/-/node-16.4.13.tgz",
+			"integrity": "sha512-bLL69sKtd25w7p1nvg9pigE4gtKVpGTPojBFLMkGHXuUgap2sLqQt2qUnqmVCDfzGUL0DRNZP+1prIZJbMeAXg=="
 		},
 		"@types/normalize-package-data": {
 			"version": "2.4.0",
diff --git a/package.json b/package.json
index d457a53..c78a8e0 100644
--- a/package.json
+++ b/package.json
@@ -38,6 +38,9 @@
 	"files": [
 		"dist/*"
 	],
+	"engines": {
+		"node": ">=16.0.0"
+	},
 	"dependencies": {
 		"@types/ws": "^7.4.4",
 		"discord-api-types": "^0.22.0",
@@ -53,7 +56,7 @@
 		"@commitlint/config-angular": "^12.1.4",
 		"@favware/rollup-type-bundler": "^1.0.2",
 		"@types/jest": "^26.0.23",
-		"@types/node": "^15.12.2",
+		"@types/node": "^16.4.13",
 		"@typescript-eslint/eslint-plugin": "^4.26.1",
 		"@typescript-eslint/parser": "^4.26.1",
 		"babel-jest": "^27.0.2",
diff --git a/src/audio/AudioResource.ts b/src/audio/AudioResource.ts
index ccbfca5..4a79a04 100644
--- a/src/audio/AudioResource.ts
+++ b/src/audio/AudioResource.ts
@@ -1,5 +1,5 @@
 import { Edge, findPipeline, StreamType, TransformerType } from './TransformerGraph';
-import { once, pipeline, Readable } from 'stream';
+import { pipeline, Readable } from 'stream';
 import { noop } from '../util/util';
 import { VolumeTransformer, opus } from 'prism-media';
 import { AudioPlayer, SILENCE_FRAME } from './AudioPlayer';
@@ -109,9 +109,7 @@ export class AudioResource<T = unknown> {
 			}
 		}
 
-		once(this.playStream, 'readable')
-			.then(() => (this.started = true))
-			.catch(noop);
+		this.playStream.once('readable', () => (this.started = true));
 	}
 
 	/**
diff --git a/src/util/abortAfter.ts b/src/util/abortAfter.ts
new file mode 100644
index 0000000..8244a9a
--- /dev/null
+++ b/src/util/abortAfter.ts
@@ -0,0 +1,10 @@
+/**
+ * Creates an abort controller that aborts after the given time.
+ * @param delay - The time in milliseconds to wait before aborting
+ */
+export function abortAfter(delay: number): [AbortController, AbortSignal] {
+	const ac = new AbortController();
+	const timeout = setTimeout(() => ac.abort(), delay);
+	ac.signal.addEventListener('abort', () => clearTimeout(timeout));
+	return [ac, ac.signal];
+}
diff --git a/src/util/entersState.ts b/src/util/entersState.ts
index fcc0dcf..bdd1e42 100644
--- a/src/util/entersState.ts
+++ b/src/util/entersState.ts
@@ -1,17 +1,19 @@
 import { VoiceConnection, VoiceConnectionStatus } from '../VoiceConnection';
 import { AudioPlayer, AudioPlayerStatus } from '../audio/AudioPlayer';
+import { abortAfter } from './abortAfter';
+import EventEmitter, { once } from 'events';
 
 /**
  * Allows a voice connection a specified amount of time to enter a given state, otherwise rejects with an error.
  *
  * @param target - The voice connection that we want to observe the state change for
  * @param status - The status that the voice connection should be in
- * @param maxTime - The maximum time we are allowing for this to occur
+ * @param timeoutOrSignal - The maximum time we are allowing for this to occur, or a signal that will abort the operation
  */
 export function entersState(
 	target: VoiceConnection,
 	status: VoiceConnectionStatus,
-	maxTime: number,
+	timeoutOrSignal: number | AbortSignal,
 ): Promise<VoiceConnection>;
 
 /**
@@ -19,41 +21,34 @@ export function entersState(
  *
  * @param target - The audio player that we want to observe the state change for
  * @param status - The status that the audio player should be in
- * @param maxTime - The maximum time we are allowing for this to occur
+ * @param timeoutOrSignal - The maximum time we are allowing for this to occur, or a signal that will abort the operation
  */
-export function entersState(target: AudioPlayer, status: AudioPlayerStatus, maxTime: number): Promise<AudioPlayer>;
+export function entersState(
+	target: AudioPlayer,
+	status: AudioPlayerStatus,
+	timeoutOrSignal: number | AbortSignal,
+): Promise<AudioPlayer>;
 
 /**
  * Allows a target a specified amount of time to enter a given state, otherwise rejects with an error.
  *
  * @param target - The object that we want to observe the state change for
  * @param status - The status that the target should be in
- * @param maxTime - The maximum time we are allowing for this to occur
+ * @param timeoutOrSignal - The maximum time we are allowing for this to occur, or a signal that will abort the operation
  */
-export function entersState<T extends VoiceConnection | AudioPlayer>(
+export async function entersState<T extends VoiceConnection | AudioPlayer>(
 	target: T,
 	status: VoiceConnectionStatus | AudioPlayerStatus,
-	maxTime: number,
+	timeoutOrSignal: number | AbortSignal,
 ) {
-	if (target.state.status === status) {
-		return Promise.resolve(target);
+	if (target.state.status !== status) {
+		const [ac, signal] =
+			typeof timeoutOrSignal === 'number' ? abortAfter(timeoutOrSignal) : [undefined, timeoutOrSignal];
+		try {
+			await once(target as EventEmitter, status, { signal });
+		} finally {
+			ac?.abort();
+		}
 	}
-	let cleanup: () => void;
-	return new Promise((resolve, reject) => {
-		const timeout = setTimeout(
-			() => reject(new Error(`Did not enter state ${status as string} within ${maxTime}ms`)),
-			maxTime,
-		);
-
-		(target as any).once(status as any, resolve);
-		(target as any).once('error', reject);
-
-		cleanup = () => {
-			clearTimeout(timeout);
-			(target as any).off(status as any, resolve);
-			(target as any).off('error', reject);
-		};
-	})
-		.then(() => target)
-		.finally(cleanup!);
+	return target;
 }
diff --git a/tsconfig.json b/tsconfig.json
index 0cf176a..e86c69e 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -6,7 +6,7 @@
 		"alwaysStrict": true,
 		"pretty": true,
 		"target": "es2019",
-		"lib": ["ESNext"],
+		"lib": ["ESNext", "DOM"],
 		"sourceMap": true,
 		"inlineSources": true,
 		"module": "commonjs",


Test patch:
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 316479e..21cf7cd 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -4,7 +4,7 @@ jobs:
   test:
     strategy:
       matrix:
-        node: ['14', '16']
+        node: ['16']
     name: Test (Node v${{ matrix.node }})
     runs-on: ubuntu-latest
     steps:
diff --git a/src/util/__tests__/abortAfter.test.ts b/src/util/__tests__/abortAfter.test.ts
new file mode 100644
index 0000000..3541558
--- /dev/null
+++ b/src/util/__tests__/abortAfter.test.ts
@@ -0,0 +1,24 @@
+import { abortAfter } from '../abortAfter';
+
+jest.useFakeTimers();
+
+const clearTimeoutSpy = jest.spyOn(global, 'clearTimeout');
+
+describe('abortAfter', () => {
+	test('Aborts after the given delay', () => {
+		const [ac, signal] = abortAfter(100);
+		expect(ac.signal).toBe(signal);
+		expect(signal.aborted).toBe(false);
+		jest.runAllTimers();
+		expect(signal.aborted).toBe(true);
+	});
+
+	test('Cleans up when manually aborted', () => {
+		const [ac, signal] = abortAfter(100);
+		expect(ac.signal).toBe(signal);
+		expect(signal.aborted).toBe(false);
+		clearTimeoutSpy.mockClear();
+		ac.abort();
+		expect(clearTimeoutSpy).toHaveBeenCalledTimes(1);
+	});
+});
diff --git a/src/util/__tests__/entersState.test.ts b/src/util/__tests__/entersState.test.ts
new file mode 100644
index 0000000..93f0015
--- /dev/null
+++ b/src/util/__tests__/entersState.test.ts
@@ -0,0 +1,52 @@
+import EventEmitter from 'events';
+import { VoiceConnection, VoiceConnectionStatus } from '../../VoiceConnection';
+import { entersState } from '../entersState';
+
+function createFakeVoiceConnection(status = VoiceConnectionStatus.Signalling) {
+	const vc = new EventEmitter() as any;
+	vc.state = { status };
+	return vc as VoiceConnection;
+}
+
+beforeEach(() => {
+	jest.useFakeTimers();
+});
+
+describe('entersState', () => {
+	test('Returns the target once the state has been entered before timeout', async () => {
+		jest.useRealTimers();
+		const vc = createFakeVoiceConnection();
+		process.nextTick(() => vc.emit(VoiceConnectionStatus.Ready, null as any, null as any));
+		const result = await entersState(vc, VoiceConnectionStatus.Ready, 1000);
+		expect(result).toBe(vc);
+	});
+
+	test('Rejects once the timeout is exceeded', async () => {
+		const vc = createFakeVoiceConnection();
+		const promise = entersState(vc, VoiceConnectionStatus.Ready, 1000);
+		jest.runAllTimers();
+		await expect(promise).rejects.toThrowError();
+	});
+
+	test('Returns the target once the state has been entered before signal is aborted', async () => {
+		jest.useRealTimers();
+		const vc = createFakeVoiceConnection();
+		const ac = new AbortController();
+		process.nextTick(() => vc.emit(VoiceConnectionStatus.Ready, null as any, null as any));
+		const result = await entersState(vc, VoiceConnectionStatus.Ready, ac.signal);
+		expect(result).toBe(vc);
+	});
+
+	test('Rejects once the signal is aborted', async () => {
+		const vc = createFakeVoiceConnection();
+		const ac = new AbortController();
+		const promise = entersState(vc, VoiceConnectionStatus.Ready, ac.signal);
+		ac.abort();
+		await expect(promise).rejects.toThrowError();
+	});
+
+	test('Resolves immediately when target already in desired state', async () => {
+		const vc = createFakeVoiceConnection();
+		await expect(entersState(vc, VoiceConnectionStatus.Signalling, 1000)).resolves.toBe(vc);
+	});
+});