Fix rate limiter delaying tasks in a fresh window when `intervalCap > 1` · sindresorhus/p-queue@89a10bb · GitHub
Skip to content

Repository navigation

Commit 89a10bb

Browse files
sindresorhussindresorhus
committed
Fix rate limiter delaying tasks in a fresh window when intervalCap > 1
Fixes #251
1 parent 98ccb48 commit 89a10bb

4 files changed

Lines changed: 177 additions & 25 deletions

File tree

‎source/index.ts‎

Lines changed: 20 additions & 25 deletions

‎test/advanced.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -999,6 +999,92 @@ test('interval should be maintained when using await between adds (issue #182)',
999999
}
10001000
});
10011001

1002+
test('task added in a fresh window runs immediately after queue went idle mid-window (issue #251)', async () => {
1003+
const queue = new PQueue({intervalCap: 2, interval: 200});
1004+
1005+
// Window [0, 200): consume slot 1 at t≈0.
1006+
queue.add(() => undefined);
1007+
1008+
// Consume slot 2 later in the window at t≈120.
1009+
await delay(120);
1010+
queue.add(() => undefined);
1011+
await queue.onIdle();
1012+
1013+
// Now past the window boundary at t≈220. Both slots of the fresh window should be available immediately.
1014+
await delay(100);
1015+
const start = Date.now();
1016+
queue.add(() => undefined);
1017+
await queue.add(() => undefined);
1018+
const elapsed = Date.now() - start;
1019+
1020+
assert.ok(elapsed < 50, `Both tasks should start immediately in the new window, but waited ${elapsed}ms`);
1021+
});
1022+
1023+
test('fresh window after idle still enforces the cap for the following task (issue #251)', async () => {
1024+
const queue = new PQueue({intervalCap: 2, interval: 200});
1025+
1026+
// Consume both slots of the first window, then let the queue drain.
1027+
queue.add(() => undefined);
1028+
await delay(120);
1029+
queue.add(() => undefined);
1030+
await queue.onIdle();
1031+
1032+
// Past the boundary: the fresh window grants exactly two slots, so a third task must wait for the next window.
1033+
await delay(100);
1034+
const timestamps: number[] = [];
1035+
const start = Date.now();
1036+
queue.add(() => {
1037+
timestamps.push(Date.now() - start);
1038+
});
1039+
queue.add(() => {
1040+
timestamps.push(Date.now() - start);
1041+
});
1042+
await queue.add(() => {
1043+
timestamps.push(Date.now() - start);
1044+
});
1045+
1046+
assert.ok(timestamps[0] < 50, `First task should be immediate, waited ${timestamps[0]}ms`);
1047+
assert.ok(timestamps[1] < 50, `Second task should be immediate, waited ${timestamps[1]}ms`);
1048+
assert.ok(timestamps[2] >= 170, `Third task should wait for the next window, but ran after ${timestamps[2]}ms`);
1049+
});
1050+
1051+
test('carryoverIntervalCount does not break the fresh window after idle mid-window (issue #251)', async () => {
1052+
const queue = new PQueue({intervalCap: 2, interval: 200, carryoverIntervalCount: true});
1053+
1054+
queue.add(() => undefined);
1055+
await delay(120);
1056+
queue.add(() => undefined);
1057+
await queue.onIdle();
1058+
1059+
// With no pending tasks, carrying the count over resets it to zero, so the fresh window is fully available.
1060+
await delay(100);
1061+
const start = Date.now();
1062+
queue.add(() => undefined);
1063+
await queue.add(() => undefined);
1064+
const elapsed = Date.now() - start;
1065+
1066+
assert.ok(elapsed < 50, `Both tasks should start immediately in the new window, but waited ${elapsed}ms`);
1067+
});
1068+
1069+
test('task added after going idle but before the boundary still waits for the current window (issue #251)', async () => {
1070+
const queue = new PQueue({intervalCap: 2, interval: 200});
1071+
const windowStart = Date.now();
1072+
1073+
// Fill both slots spread across the window, after which the queue goes idle.
1074+
queue.add(() => undefined);
1075+
await delay(120);
1076+
queue.add(() => undefined);
1077+
await queue.onIdle();
1078+
1079+
// Added while still inside the window [0, 200): the cap is reached, so it must wait for the boundary, not run immediately. This guards against the reset firing before the window has actually expired.
1080+
const start = Date.now();
1081+
await queue.add(() => undefined);
1082+
const waited = Date.now() - start;
1083+
1084+
assert.ok(waited >= 40, `Task should wait for the current window to end, but only waited ${waited}ms`);
1085+
assert.ok(Date.now() - windowStart >= 190, 'Task should run at the window boundary');
1086+
});
1087+
10021088
test('interval maintained when queue becomes empty multiple times', async () => {
10031089
const queue = new PQueue({
10041090
intervalCap: 1,

‎test/rate-limit.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,56 @@ test('rate-limit rapid pause/start cycles', async () => {
4343
assert.equal(results.length, 4);
4444
});
4545

46+
test('rate-limit preserves the interval count for re-entrant task additions', async () => {
47+
const queue = new PQueue({
48+
interval: 100,
49+
intervalCap: 2,
50+
});
51+
52+
const starts: number[] = [];
53+
const addTask = (index: number): void => {
54+
queue.add(() => {
55+
starts.push(index);
56+
57+
if (index < 4) {
58+
addTask(index + 1);
59+
}
60+
});
61+
};
62+
63+
addTask(0);
64+
assert.deepEqual(starts, [0, 1]);
65+
66+
await queue.onIdle();
67+
assert.deepEqual(starts, [0, 1, 2, 3, 4]);
68+
});
69+
70+
test('rate-limit preserves the interval count for re-entrant active listeners', async () => {
71+
const queue = new PQueue({
72+
interval: 100,
73+
intervalCap: 2,
74+
});
75+
76+
const starts: number[] = [];
77+
let nextTask = 0;
78+
queue.on('active', () => {
79+
if (nextTask < 5) {
80+
const taskIndex = nextTask++;
81+
queue.add(() => {
82+
starts.push(taskIndex);
83+
});
84+
}
85+
});
86+
87+
queue.add(() => {
88+
starts.push(-1);
89+
});
90+
assert.equal(starts.length, 2);
91+
92+
await queue.onIdle();
93+
assert.equal(starts.length, 6);
94+
});
95+
4696
test('rate-limit edge case with zero-interval', async () => {
4797
// Zero interval should effectively disable rate limiting
4898
const queue = new PQueue({

‎test/strict.ts‎

Lines changed: 21 additions & 0 deletions

0 commit comments

Comments
 (0)