Skip to content

Commit e48716f

Browse files
committed
Remove throwOnTimeout option - timeouts now always throw
Closes #214 Closes #206 Closes #218 Closes #214
1 parent 62efb74 commit e48716f

File tree

6 files changed

+73
-80
lines changed

6 files changed

+73
-80
lines changed

readme.md

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,17 +87,10 @@ Concurrency limit.
8787

8888
Type: `number`
8989

90-
Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses if they haven't already.
90+
Per-operation timeout in milliseconds. Operations will throw a `TimeoutError` if they don't complete within the specified time.
9191

9292
The timeout begins when the operation is dequeued and starts execution, not while it's waiting in the queue.
9393

94-
##### throwOnTimeout
95-
96-
Type: `boolean`\
97-
Default: `false`
98-
99-
Whether or not a timeout is considered an exception.
100-
10194
##### autoStart
10295

10396
Type: `boolean`\

source/index.ts

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {EventEmitter} from 'eventemitter3';
2-
import pTimeout, {TimeoutError} from 'p-timeout';
2+
import pTimeout from 'p-timeout';
33
import {type Queue, type RunFunction} from './queue.js';
44
import PriorityQueue from './priority-queue.js';
55
import {type QueueAddOptions, type Options, type TaskOptions} from './options.js';
@@ -43,21 +43,18 @@ export default class PQueue<QueueType extends Queue<RunFunction, EnqueueOptionsT
4343

4444
#isPaused: boolean;
4545

46-
readonly #throwOnTimeout: boolean;
47-
4846
// Use to assign a unique identifier to a promise function, if not explicitly specified
4947
#idAssigner = 1n;
5048

5149
/**
52-
Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses if they haven't already.
50+
Per-operation timeout in milliseconds. Operations will throw a `TimeoutError` if they don't complete within the specified time.
5351
5452
The timeout begins when the operation is dequeued and starts execution, not while it's waiting in the queue.
5553
5654
Applies to each future operation.
5755
*/
5856
timeout?: number;
5957

60-
// TODO: The `throwOnTimeout` option should affect the return types of `add()` and `addAll()`
6158
constructor(options?: Options<QueueType, EnqueueOptionsType>) {
6259
super();
6360

@@ -88,7 +85,6 @@ export default class PQueue<QueueType extends Queue<RunFunction, EnqueueOptionsT
8885
this.#queueClass = options.queueClass!;
8986
this.concurrency = options.concurrency!;
9087
this.timeout = options.timeout;
91-
this.#throwOnTimeout = options.throwOnTimeout === true;
9288
this.#isPaused = options.autoStart === false;
9389
}
9490

@@ -309,15 +305,13 @@ export default class PQueue<QueueType extends Queue<RunFunction, EnqueueOptionsT
309305
/**
310306
Adds a sync or async task to the queue. Always returns a promise.
311307
*/
312-
async add<TaskResultType>(function_: Task<TaskResultType>, options: {throwOnTimeout: true} & Exclude<EnqueueOptionsType, 'throwOnTimeout'>): Promise<TaskResultType>;
313-
async add<TaskResultType>(function_: Task<TaskResultType>, options?: Partial<EnqueueOptionsType>): Promise<TaskResultType | void>;
314-
async add<TaskResultType>(function_: Task<TaskResultType>, options: Partial<EnqueueOptionsType> = {}): Promise<TaskResultType | void> {
308+
async add<TaskResultType>(function_: Task<TaskResultType>, options?: Partial<EnqueueOptionsType>): Promise<TaskResultType>;
309+
async add<TaskResultType>(function_: Task<TaskResultType>, options: Partial<EnqueueOptionsType> = {}): Promise<TaskResultType> {
315310
// In case `id` is not defined.
316311
options.id ??= (this.#idAssigner++).toString();
317312

318313
options = {
319314
timeout: this.timeout,
320-
throwOnTimeout: this.#throwOnTimeout,
321315
...options,
322316
};
323317

@@ -353,11 +347,6 @@ export default class PQueue<QueueType extends Queue<RunFunction, EnqueueOptionsT
353347
resolve(result);
354348
this.emit('completed', result);
355349
} catch (error: unknown) {
356-
if (error instanceof TimeoutError && !options.throwOnTimeout) {
357-
resolve();
358-
return;
359-
}
360-
361350
reject(error);
362351
this.emit('error', error);
363352
} finally {
@@ -379,18 +368,14 @@ export default class PQueue<QueueType extends Queue<RunFunction, EnqueueOptionsT
379368
380369
@returns A promise that resolves when all functions are resolved.
381370
*/
382-
async addAll<TaskResultsType>(
383-
functions: ReadonlyArray<Task<TaskResultsType>>,
384-
options?: {throwOnTimeout: true} & Partial<Exclude<EnqueueOptionsType, 'throwOnTimeout'>>,
385-
): Promise<TaskResultsType[]>;
386371
async addAll<TaskResultsType>(
387372
functions: ReadonlyArray<Task<TaskResultsType>>,
388373
options?: Partial<EnqueueOptionsType>,
389-
): Promise<Array<TaskResultsType | void>>;
374+
): Promise<TaskResultsType[]>;
390375
async addAll<TaskResultsType>(
391376
functions: ReadonlyArray<Task<TaskResultsType>>,
392377
options?: Partial<EnqueueOptionsType>,
393-
): Promise<Array<TaskResultsType | void>> {
378+
): Promise<TaskResultsType[]> {
394379
return Promise.all(functions.map(async function_ => this.add(function_, options)));
395380
}
396381

source/options.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,11 @@ import {type Queue, type RunFunction} from './queue.js';
22

33
type TimeoutOptions = {
44
/**
5-
Per-operation timeout in milliseconds. Operations fulfill once `timeout` elapses if they haven't already.
5+
Per-operation timeout in milliseconds. Operations will throw a `TimeoutError` if they don't complete within the specified time.
66
77
The timeout begins when the operation is dequeued and starts execution, not while it's waiting in the queue.
88
*/
99
timeout?: number;
10-
11-
/**
12-
Whether or not a timeout is considered an exception.
13-
14-
@default false
15-
*/
16-
throwOnTimeout?: boolean;
1710
};
1811

1912
export type Options<QueueType extends Queue<RunFunction, QueueOptions>, QueueOptions extends QueueAddOptions> = {

test-d/index.test-d.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,4 @@ import PQueue from '../source/index.js';
33

44
const queue = new PQueue();
55

6-
expectType<Promise<string | void>>(queue.add(async () => '🦄'));
7-
expectType<Promise<string>>(queue.add(async () => '🦄', {throwOnTimeout: true}));
6+
expectType<Promise<string>>(queue.add(async () => '🦄'));

test/advanced.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -262,16 +262,12 @@ test('should emit completed / error events', async () => {
262262
});
263263

264264
test('should verify timeout overrides passed to add', async () => {
265-
const queue = new PQueue({timeout: 200, throwOnTimeout: true});
265+
const queue = new PQueue({timeout: 200});
266266

267267
await assert.rejects(queue.add(async () => {
268268
await delay(400);
269269
}));
270270

271-
await queue.add(async () => {
272-
await delay(400);
273-
}, {throwOnTimeout: false});
274-
275271
await queue.add(async () => {
276272
await delay(400);
277273
}, {timeout: 600});

test/basic.ts

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import delay from 'delay';
66
import timeSpan from 'time-span';
77
import randomInt from 'random-int';
88
import pDefer from 'p-defer';
9+
import {TimeoutError} from 'p-timeout';
910
import PQueue from '../source/index.js';
1011

1112
const fixture = Symbol('fixture');
@@ -131,65 +132,91 @@ test('.add() - priority defaults to 0 when undefined', async () => {
131132
assert.deepEqual(result, ['first', 'priority', 'second', 'third']);
132133
});
133134

134-
test('.add() - timeout without throwing', async () => {
135-
const result: string[] = [];
136-
const queue = new PQueue({timeout: 300, throwOnTimeout: false});
137-
queue.add(async () => {
138-
await delay(400);
139-
result.push('🐌');
140-
});
141-
queue.add(async () => {
142-
await delay(250);
143-
result.push('🦆');
144-
});
145-
queue.add(async () => {
146-
await delay(310);
147-
result.push('🐢');
148-
});
149-
queue.add(async () => {
150-
await delay(100);
151-
result.push('🐅');
152-
});
153-
queue.add(async () => {
154-
result.push('⚡️');
135+
test('.add() - timeout always throws', async () => {
136+
const queue = new PQueue({timeout: 300});
137+
const errors: unknown[] = [];
138+
139+
// Task that will timeout
140+
await assert.rejects(
141+
queue.add(async () => {
142+
await delay(400);
143+
return '🐌';
144+
}),
145+
TimeoutError,
146+
'Task exceeding timeout should throw TimeoutError',
147+
);
148+
149+
// Task that completes within timeout
150+
const result = await queue.add(async () => {
151+
await delay(200);
152+
return '🦆';
155153
});
154+
155+
assert.equal(result, '🦆', 'Task within timeout should complete normally');
156+
157+
// Test with very short timeout
158+
await assert.rejects(
159+
queue.add(async () => delay(100), {timeout: 10}),
160+
TimeoutError,
161+
'Short timeout should throw',
162+
);
163+
156164
await queue.onIdle();
157-
assert.deepEqual(result, ['⚡️', '🐅', '🦆']);
158165
});
159166

160-
test.skip('.add() - timeout with throwing', async () => {
161-
const result: string[] = [];
162-
const queue = new PQueue({timeout: 300, throwOnTimeout: true});
163-
await assert.rejects(queue.add(async () => {
164-
await delay(400);
165-
result.push('🐌');
166-
}));
167-
queue.add(async () => {
167+
test('.add() - timeout behavior', async () => {
168+
const queue = new PQueue({timeout: 300});
169+
170+
// Test multiple timeouts
171+
await assert.rejects(
172+
queue.add(async () => {
173+
await delay(400);
174+
return '🐌';
175+
}),
176+
TimeoutError,
177+
);
178+
179+
// Task that completes
180+
const result = await queue.add(async () => {
168181
await delay(200);
169-
result.push('🦆');
182+
return '🦆';
170183
});
184+
assert.equal(result, '🦆');
185+
186+
// Test timeout override
187+
const longResult = await queue.add(async () => {
188+
await delay(400);
189+
return '🐢';
190+
}, {timeout: 500});
191+
assert.equal(longResult, '🐢', 'Task should complete with extended timeout');
192+
171193
await queue.onIdle();
172-
assert.deepEqual(result, ['🦆']);
173194
});
174195

175196
test('.add() - change timeout in between', async () => {
176197
const result: string[] = [];
177198
const initialTimeout = 50;
178199
const newTimeout = 200;
179-
const queue = new PQueue({timeout: initialTimeout, throwOnTimeout: false, concurrency: 2});
180-
queue.add(async () => {
200+
const queue = new PQueue({timeout: initialTimeout, concurrency: 2});
201+
202+
// This task will timeout with initial timeout of 50ms
203+
await assert.rejects(queue.add(async () => {
181204
const {timeout} = queue;
182205
assert.equal(timeout, initialTimeout);
183206
await delay(300);
184207
result.push('🐌');
185-
});
208+
}), TimeoutError);
209+
186210
queue.timeout = newTimeout;
187-
queue.add(async () => {
211+
212+
// This task will complete within the new timeout of 200ms
213+
await queue.add(async () => {
188214
const {timeout} = queue;
189215
assert.equal(timeout, newTimeout);
190216
await delay(100);
191217
result.push('🐅');
192218
});
219+
193220
await queue.onIdle();
194221
assert.deepEqual(result, ['🐅']);
195222
});

0 commit comments

Comments
 (0)