fix(rate-limit): never start the work of a caller aborted before dispatch

changelog.d/fixes/0000-rate-limit-aborted-caller-dispatch.md

@@ -0,0 +1 @@
+- **fix(rate-limit):** `withRateLimit` no longer starts the scheduled work of a caller that was aborted while it waited for its provider slot. With limiter capacity free, Bottleneck dispatches the job before `abandonQueuedJob()` can pull it from the queue, so the work ran with an already-aborted signal; work that only listens for a later `abort` event then held the limiter slot and Bottleneck's ref'd expiration timer for `executionMaxWaitMs` (10 min by default). The dispatched job now rejects without calling the work, as it already did after a queue-wait timeout, and `tests/unit/rate-limit-manager.test.ts` exits right after its last test instead of about 10 minutes later.

open-sse/services/rateLimitManager.ts

@@ -779,6 +779,7 @@ export async function withRateLimit(
   // would kill a 300ms execution that correctly left the queue immediately).
   const wrappedFn = () => {
     if (queueTimedOut) return Promise.reject(queueTimeoutErr);
+    if (signal?.aborted) return Promise.reject(signal.reason);
     if (delayId) {
       clearTimeout(delayId);
       delayId = null;
@@ -793,8 +794,9 @@ export async function withRateLimit(
   const scheduled = limiter.schedule(scheduleOpts, boundFn as unknown as () => Promise<unknown>);
   scheduled.catch(() => {});
   // If timeoutPromise or the abort wins while the job is still QUEUED,
-  // abandonQueuedJob() removes it (see queuedJobCancel.ts); a job already past
-  // QUEUED is kept from calling fn by wrappedFn's queueTimedOut guard.
+  // abandonQueuedJob() removes it (see queuedJobCancel.ts). A job already past
+  // QUEUED (with capacity free, Bottleneck dispatches before that removal runs)
+  // is kept from calling fn by wrappedFn's guards; the race already settled.
 
   try {
     if (signal) {

tests/unit/rate-limit-manager.test.ts

@@ -708,6 +708,45 @@ test("withRateLimit forwards AbortController DOMException without mutating it",
   });
 });
 
+test("withRateLimit never starts the work of a caller that aborted before dispatch", async () => {
+  await rateLimitManager.applyRequestQueueSettings({
+    ...resilienceSettings.DEFAULT_RESILIENCE_SETTINGS.requestQueue,
+    autoEnableApiKeyProviders: false,
+  });
+  const limiters: InstanceType<typeof Bottleneck>[] = [];
+  rateLimitManager.__setLimiterFactoryForTests((options) => {
+    const limiter = new Bottleneck(options);
+    limiters.push(limiter);
+    return limiter;
+  });
+  const connectionId = "conn-abort-before-dispatch";
+  rateLimitManager.enableRateLimitProtection(connectionId);
+  const controller = new AbortController();
+  let calls = 0;
+  const pending = rateLimitManager.withRateLimit(
+    "openai",
+    connectionId,
+    "gpt-4o",
+    async () => {
+      calls++;
+      return "must-not-run";
+    },
+    controller.signal
+  );
+  // withRateLimit is suspended before limiter.schedule() when the abort lands.
+  // With capacity free, Bottleneck dispatches the job inside its submit lock,
+  // so the job leaves QUEUED before abandonQueuedJob() can take that lock.
+  controller.abort();
+
+  await assert.rejects(pending, { name: "AbortError" });
+  assert.equal(limiters.length, 1, "the aborted caller's job never reached a limiter");
+  await waitForCondition(() => {
+    const counts = limiters[0].counts();
+    return counts.RECEIVED + counts.QUEUED + counts.RUNNING + counts.EXECUTING === 0;
+  }, "the aborted caller's job never left the limiter");
+  assert.equal(calls, 0, "work must not start for a caller that already received AbortError");
+});
+
 test("rate limit manager handles soft over-limit warnings and normal header learning", async () => {
   rateLimitManager.enableRateLimitProtection("conn-over-limit");
   rateLimitManager.updateFromHeaders(