fix(rest): prevent AbortSignal event listener leak

[Sim-hu]

Summary

• Add { once: true } to the abort event listener on the user-provided AbortSignal in makeNetworkRequest (packages/rest/src/lib/handlers/Shared.ts)
• Without this, every request adds a new listener that is never removed. When users reuse the same AbortSignal across multiple requests, listeners accumulate, each capturing a reference to the local AbortController and preventing it from being garbage collected
• The once option ensures the listener is automatically removed after firing and allows the captured controller to be garbage collected when it is no longer needed

Test plan

• Verify the @discordjs/rest package compiles without type errors (tsc --noEmit passes)
• Confirm that aborting a request via a user-provided signal still correctly aborts the internal controller
• Confirm that after a request completes, the listener is no longer present on the user signal (can be verified by checking getEventListeners count in Node.js or by observing memory profiles under repeated requests with a shared signal)

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
discord-js	Skipped		Mar 20, 2026 2:19pm
discord-js-guide	Skipped		Mar 20, 2026 2:19pm

[coderabbitai]

📝 Walkthrough

Walkthrough

The makeNetworkRequest function in Shared.ts now registers its abort signal listener with the { once: true } option, ensuring the listener fires only once per request instead of potentially multiple times if the signal triggers repeatedly.

Changes

Cohort / File(s)	Summary
Abort Signal Listener packages/rest/src/lib/handlers/Shared.ts	Modified abort listener registration to use { once: true } option, preventing repeated invocations of abort logic per request.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: preventing an AbortSignal event listener leak by using the 'once' option, which directly aligns with the changeset.
Description check	✅ Passed	The description is directly related to the changeset, explaining the problem, the solution, and providing a comprehensive test plan for the single-line change.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

📝 Coding Plan

• Generate coding plan for human review comments

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Tip

You can get early access to new features in CodeRabbit.

Enable the early_access setting to enable early access features such as new models, tools, and more.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/rest/src/lib/handlers/Shared.ts (1)

74-80: ⚠️ Potential issue | 🟠 Major

{ once: true } does not prevent listener retention on non-aborted requests

Line 79 only auto-removes the listener if abort is dispatched. For requests that complete successfully or with errors where the user signal never aborts, the listener remains attached on a reused signal and retains the per-request controller closure. Retries reuse the same requestData object (including its signal) in SequentialHandler.ts and BurstHandler.ts, accumulating listeners on long-lived signals. Explicitly unregister the listener in the finally block.

Proposed fix

 export async function makeNetworkRequest(
 	manager: REST,
 	routeId: RouteData,
 	url: string,
 	options: RequestInit,
 	requestData: HandlerRequestData,
 	retries: number,
 ) {
 	const controller = new AbortController();
+	const userSignal = requestData.signal;
+	let onAbort: (() => void) | null = null;
 	const timeout = setTimeout(
 		() => controller.abort(),
 		normalizeTimeout(manager.options.timeout, routeId.bucketRoute, requestData.body),
 	);
-	if (requestData.signal) {
+	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(), { once: true });
+		if (userSignal.aborted) controller.abort();
+		else {
+			onAbort = () => controller.abort();
+			userSignal.addEventListener('abort', onAbort, { once: true });
+		}
 	}
@@
 	} finally {
 		clearTimeout(timeout);
+		if (userSignal && onAbort) {
+			userSignal.removeEventListener('abort', onAbort);
+		}
 	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/rest/src/lib/handlers/Shared.ts` around lines 74 - 80, The abort
listener added to requestData.signal via
requestData.signal.addEventListener('abort', () => controller.abort(), { once:
true }) only removes itself when the signal is aborted, leaking the per-request
controller closure for requests that complete normally or error without abort;
modify the code path that creates the listener (in Shared.ts where controller is
created and requestData.signal is referenced) to capture the listener function
reference, add it with addEventListener, and always remove it in the request
cleanup/finally block (call requestData.signal.removeEventListener with that
listener) so retries that reuse requestData (e.g., SequentialHandler.ts and
BurstHandler.ts) do not accumulate listeners or retain closures.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/rest/src/lib/handlers/Shared.ts`:
- Around line 74-80: The abort listener added to requestData.signal via
requestData.signal.addEventListener('abort', () => controller.abort(), { once:
true }) only removes itself when the signal is aborted, leaking the per-request
controller closure for requests that complete normally or error without abort;
modify the code path that creates the listener (in Shared.ts where controller is
created and requestData.signal is referenced) to capture the listener function
reference, add it with addEventListener, and always remove it in the request
cleanup/finally block (call requestData.signal.removeEventListener with that
listener) so retries that reuse requestData (e.g., SequentialHandler.ts and
BurstHandler.ts) do not accumulate listeners or retain closures.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 72931abe-1507-46b3-ac5e-32d37407171b

📥 Commits

Reviewing files that changed from the base of the PR and between 2a06721 and 93cf4f1.

📒 Files selected for processing (1)

• packages/rest/src/lib/handlers/Shared.ts