mirror of
https://github.com/discordjs/discord.js.git
synced 2026-09-17 08:47:27 +00:00
fix: prevent AbortSignal event listener leak (#11594)
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
/* eslint-disable id-length */
|
||||
/* eslint-disable promise/prefer-await-to-then */
|
||||
import { getEventListeners } from 'node:events';
|
||||
import { MockAgent, setGlobalDispatcher } from 'undici';
|
||||
import type { Interceptable, MockInterceptor } from 'undici/types/mock-interceptor.js';
|
||||
import { beforeEach, afterEach, test, expect, vitest } from 'vitest';
|
||||
@@ -549,6 +550,22 @@ test('malformedRequest', async () => {
|
||||
await expect(api.get('/malformedRequest')).rejects.toBeInstanceOf(DiscordAPIError);
|
||||
});
|
||||
|
||||
test('remove abort listeners after requests complete', async () => {
|
||||
mockPool
|
||||
.intercept({
|
||||
path: genPath('/abort-listener-cleanup'),
|
||||
method: 'GET',
|
||||
})
|
||||
.reply(200, { message: 'Hello World' }, responseOptions)
|
||||
.times(2);
|
||||
|
||||
const controller = new AbortController();
|
||||
for (let index = 0; index < 2; index++) {
|
||||
await api.get('/abort-listener-cleanup', { signal: controller.signal });
|
||||
expect(getEventListeners(controller.signal, 'abort')).toHaveLength(0);
|
||||
}
|
||||
});
|
||||
|
||||
// TODO: flaky due to changes in undici
|
||||
test.skip('abort', async () => {
|
||||
mockPool
|
||||
|
||||
@@ -71,12 +71,17 @@ export async function makeNetworkRequest(
|
||||
() => controller.abort(),
|
||||
normalizeTimeout(manager.options.timeout, routeId.bucketRoute, requestData.body),
|
||||
);
|
||||
if (requestData.signal) {
|
||||
const userSignal = requestData.signal;
|
||||
let onUserAbort: (() => void) | undefined;
|
||||
if (userSignal) {
|
||||
// If the user signal was aborted, abort the controller, else abort the local signal.
|
||||
// The reason why we don't re-use the user's signal, is because users may use the same signal for multiple
|
||||
// requests, and we do not want to cause unexpected side-effects.
|
||||
if (requestData.signal.aborted) controller.abort();
|
||||
else requestData.signal.addEventListener('abort', () => controller.abort());
|
||||
if (userSignal.aborted) controller.abort();
|
||||
else {
|
||||
onUserAbort = () => controller.abort();
|
||||
userSignal.addEventListener('abort', onUserAbort);
|
||||
}
|
||||
}
|
||||
|
||||
let res: ResponseLike;
|
||||
@@ -108,6 +113,7 @@ export async function makeNetworkRequest(
|
||||
throw error;
|
||||
} finally {
|
||||
clearTimeout(timeout);
|
||||
if (onUserAbort) userSignal!.removeEventListener('abort', onUserAbort);
|
||||
}
|
||||
|
||||
if (manager.listenerCount(RESTEvents.Response)) {
|
||||
|
||||
Reference in New Issue
Block a user