From 945cdb492915345885051416437ce4cfb61a7bce Mon Sep 17 00:00:00 2001 From: Amish Shah Date: Sun, 8 Aug 2021 12:42:53 +0100 Subject: [PATCH 1/5] refactor(Util): use AbortSignal in entersState --- package-lock.json | 17 +++++++------ package.json | 5 +++- src/util/abortAfter.ts | 10 ++++++++ src/util/entersState.ts | 56 ++++++++++++++++++++++------------------- tsconfig.json | 2 +- 5 files changed, 55 insertions(+), 35 deletions(-) create mode 100644 src/util/abortAfter.ts diff --git a/package-lock.json b/package-lock.json index 851b1a27..12a446c7 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 d457a53e..c78a8e0c 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/util/abortAfter.ts b/src/util/abortAfter.ts new file mode 100644 index 00000000..7500cae9 --- /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 { + const ac = new AbortController(); + const timeout = setTimeout(() => ac.abort(), delay); + ac.signal.addEventListener('abort', () => clearTimeout(timeout)); + return ac; +} diff --git a/src/util/entersState.ts b/src/util/entersState.ts index fcc0dcf3..f760de81 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; /** @@ -19,41 +21,43 @@ 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; +export function entersState( + target: AudioPlayer, + status: AudioPlayerStatus, + timeoutOrSignal: number | AbortSignal, +): Promise; /** * 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( +export async function entersState( 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] = + timeoutOrSignal instanceof AbortSignal ? [undefined, timeoutOrSignal] : createDelayedAbort(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); + return target; +} - cleanup = () => { - clearTimeout(timeout); - (target as any).off(status as any, resolve); - (target as any).off('error', reject); - }; - }) - .then(() => target) - .finally(cleanup!); +/** + * Creates an abort controller that will abort after the specified delay. + * @param delay - The delay before aborting + */ +function createDelayedAbort(delay: number): [AbortController, AbortSignal] { + const ac = abortAfter(delay); + return [ac, ac.signal]; } diff --git a/tsconfig.json b/tsconfig.json index 0cf176a4..e86c69e0 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", From b0e3c7e0ae202a88a70b579cbc222658122a6c3b Mon Sep 17 00:00:00 2001 From: Amish Shah Date: Sun, 8 Aug 2021 12:43:28 +0100 Subject: [PATCH 2/5] chore(CI): drop Node v14 from CI --- .github/workflows/docs.yml | 2 +- .github/workflows/lint.yml | 2 +- .github/workflows/test.yml | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 96685e20..7382a031 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 7d441f83..3d66f46f 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/.github/workflows/test.yml b/.github/workflows/test.yml index 316479ec..21cf7cde 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: From c384ffdb2fe9c077df15dcf4928c73ea0e984ec4 Mon Sep 17 00:00:00 2001 From: Amish Shah Date: Sun, 8 Aug 2021 12:46:44 +0100 Subject: [PATCH 3/5] refactor(Util): remove unnecessary function --- src/util/abortAfter.ts | 4 ++-- src/util/entersState.ts | 11 +---------- 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/src/util/abortAfter.ts b/src/util/abortAfter.ts index 7500cae9..8244a9a2 100644 --- a/src/util/abortAfter.ts +++ b/src/util/abortAfter.ts @@ -2,9 +2,9 @@ * 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 { +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; + return [ac, ac.signal]; } diff --git a/src/util/entersState.ts b/src/util/entersState.ts index f760de81..5ea2f1d7 100644 --- a/src/util/entersState.ts +++ b/src/util/entersState.ts @@ -43,7 +43,7 @@ export async function entersState( ) { if (target.state.status !== status) { const [ac, signal] = - timeoutOrSignal instanceof AbortSignal ? [undefined, timeoutOrSignal] : createDelayedAbort(timeoutOrSignal); + timeoutOrSignal instanceof AbortSignal ? [undefined, timeoutOrSignal] : abortAfter(timeoutOrSignal); try { await once(target as EventEmitter, status, { signal }); } finally { @@ -52,12 +52,3 @@ export async function entersState( } return target; } - -/** - * Creates an abort controller that will abort after the specified delay. - * @param delay - The delay before aborting - */ -function createDelayedAbort(delay: number): [AbortController, AbortSignal] { - const ac = abortAfter(delay); - return [ac, ac.signal]; -} From efd3462e22d339792637ea999423fb927c7536e9 Mon Sep 17 00:00:00 2001 From: Amish Shah Date: Sun, 8 Aug 2021 12:53:37 +0100 Subject: [PATCH 4/5] test(Util): add tests for abortAfter --- src/util/__tests__/abortAfter.test.ts | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 src/util/__tests__/abortAfter.test.ts diff --git a/src/util/__tests__/abortAfter.test.ts b/src/util/__tests__/abortAfter.test.ts new file mode 100644 index 00000000..35415584 --- /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); + }); +}); From a693038796af9ce8e6be02d5b50c07e9f9bab48a Mon Sep 17 00:00:00 2001 From: Amish Shah Date: Sun, 8 Aug 2021 13:12:14 +0100 Subject: [PATCH 5/5] test(Util): add test for entersState --- src/util/__tests__/entersState.test.ts | 52 ++++++++++++++++++++++++++ src/util/entersState.ts | 2 +- 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 src/util/__tests__/entersState.test.ts diff --git a/src/util/__tests__/entersState.test.ts b/src/util/__tests__/entersState.test.ts new file mode 100644 index 00000000..93f00159 --- /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); + }); +}); diff --git a/src/util/entersState.ts b/src/util/entersState.ts index 5ea2f1d7..bdd1e428 100644 --- a/src/util/entersState.ts +++ b/src/util/entersState.ts @@ -43,7 +43,7 @@ export async function entersState( ) { if (target.state.status !== status) { const [ac, signal] = - timeoutOrSignal instanceof AbortSignal ? [undefined, timeoutOrSignal] : abortAfter(timeoutOrSignal); + typeof timeoutOrSignal === 'number' ? abortAfter(timeoutOrSignal) : [undefined, timeoutOrSignal]; try { await once(target as EventEmitter, status, { signal }); } finally {