From 53f88cd2f161e6ab815ace609a9bce5047a43fa1 Mon Sep 17 00:00:00 2001 From: Tomas Zijdemans Date: Tue, 30 Dec 2025 13:20:39 +0100 Subject: [PATCH] refactor(async): improve deadline() with NaN validation and test coverage - Add NaN input validation with TypeError - Fix docs: clarify MAX_SAFE_INTEGER boundary behavior - Refactor to avoid unnecessary AbortSignal.any() when not needed - Early return original promise when no timeout and no signal - Add tests for: zero timeout, NaN, custom abort reason, signal priority, MAX_SAFE_INTEGER --- async/deadline.ts | 33 +++++++++++++++---- async/deadline_test.ts | 72 ++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 95 insertions(+), 10 deletions(-) diff --git a/async/deadline.ts b/async/deadline.ts index 635d9feeb6a4..e918c7d61339 100644 --- a/async/deadline.ts +++ b/async/deadline.ts @@ -1,6 +1,5 @@ // Copyright 2018-2025 the Deno authors. MIT license. // This module is browser compatible. - import { abortable } from "./abortable.ts"; /** Options for {@linkcode deadline}. */ @@ -16,6 +15,7 @@ export interface DeadlineOptions { * Note: Prefer to use {@linkcode AbortSignal.timeout} instead for the APIs * that accept {@linkcode AbortSignal}. * + * @throws {TypeError} If `ms` is `NaN`. * @throws {DOMException & { name: "TimeoutError" }} If the provided duration * runs out before resolving. * @throws {DOMException & { name: "AbortError" }} If the optional signal is @@ -25,7 +25,7 @@ export interface DeadlineOptions { * @typeParam T The type of the provided and returned promise. * @param p The promise to make rejectable. * @param ms Duration in milliseconds for when the promise should time out. If - * greater than `Number.MAX_SAFE_INTEGER`, the deadline will never expire. + * greater than or equal to `Number.MAX_SAFE_INTEGER`, the deadline will never expire. * @param options Additional options. * @returns A promise that will reject if the provided duration runs out before resolving. * @@ -39,13 +39,32 @@ export interface DeadlineOptions { * const result = await deadline(delayedPromise, 10); * ``` */ -export async function deadline( +export function deadline( p: Promise, ms: number, options: DeadlineOptions = {}, ): Promise { - const signals: AbortSignal[] = []; - if (ms < Number.MAX_SAFE_INTEGER) signals.push(AbortSignal.timeout(ms)); - if (options.signal) signals.push(options.signal); - return await abortable(p, AbortSignal.any(signals)); + if (Number.isNaN(ms)) { + throw new TypeError("Ms must be a number, received NaN"); + } + + const hasTimeout = ms < Number.MAX_SAFE_INTEGER; + const hasSignal = options.signal !== undefined; + + if (!hasTimeout && !hasSignal) return p; + + if (hasTimeout && !hasSignal) { + return abortable(p, AbortSignal.timeout(ms)); + } + if (!hasTimeout && hasSignal) { + return abortable(p, options.signal!); + } + + return abortable( + p, + AbortSignal.any([ + AbortSignal.timeout(ms), + options.signal!, + ]), + ); } diff --git a/async/deadline_test.ts b/async/deadline_test.ts index 72b58bb60f17..c78692793f13 100644 --- a/async/deadline_test.ts +++ b/async/deadline_test.ts @@ -1,5 +1,5 @@ // Copyright 2018-2025 the Deno authors. MIT license. -import { assertEquals, assertRejects } from "@std/assert"; +import { assertEquals, assertRejects, assertThrows } from "@std/assert"; import { delay } from "./delay.ts"; import { deadline } from "./deadline.ts"; @@ -29,6 +29,21 @@ Deno.test("deadline() throws DOMException", async () => { controller.abort(); }); +Deno.test("deadline() with zero timeout rejects immediately", async () => { + const controller = new AbortController(); + const { signal } = controller; + const p = delay(100, { signal }) + .catch(() => {}) + .then(() => "Hello"); + const error = await assertRejects( + () => deadline(p, 0), + DOMException, + "Signal timed out.", + ); + assertEquals(error.name, "TimeoutError"); + controller.abort(); +}); + Deno.test("deadline() throws when promise is rejected", async () => { const controller = new AbortController(); const { signal } = controller; @@ -45,6 +60,15 @@ Deno.test("deadline() throws when promise is rejected", async () => { controller.abort(); }); +Deno.test("deadline() throws TypeError for NaN", () => { + const p = Promise.resolve("Hello"); + assertThrows( + () => deadline(p, NaN), + TypeError, + "Ms must be a number, received NaN", + ); +}); + Deno.test("deadline() handles non-aborted signal", async () => { const controller = new AbortController(); const { signal } = controller; @@ -92,7 +116,49 @@ Deno.test("deadline() handles already aborted signal", async () => { controller.abort(); }); +Deno.test("deadline() propagates custom abort reason", async () => { + const controller = new AbortController(); + const { signal } = controller; + const customError = new Error("Custom abort reason"); + const p = delay(1000, { signal }) + .catch(() => {}) + .then(() => "Hello"); + const abort = new AbortController(); + abort.abort(customError); + await assertRejects( + () => deadline(p, 1000, { signal: abort.signal }), + Error, + "Custom abort reason", + ); + controller.abort(); +}); + +Deno.test("deadline() with both timeout and signal, signal aborts first", async () => { + const controller = new AbortController(); + const { signal } = controller; + const p = delay(500, { signal }) + .catch(() => {}) + .then(() => "Hello"); + const abort = new AbortController(); + setTimeout(() => abort.abort(), 50); + const error = await assertRejects( + () => deadline(p, 1000, { signal: abort.signal }), + DOMException, + "The signal has been aborted", + ); + assertEquals(error.name, "AbortError"); + controller.abort(); +}); + +Deno.test("deadline() with MAX_SAFE_INTEGER does not timeout", async () => { + const result = await deadline( + Promise.resolve("Hello"), + Number.MAX_SAFE_INTEGER, + ); + assertEquals(result, "Hello"); +}); + Deno.test("deadline() supports numbers greater than Number.MAX_SAFE_INTEGER", async () => { - const promise = await deadline(Promise.resolve("Hello"), Infinity); - assertEquals(promise, "Hello"); + const result = await deadline(Promise.resolve("Hello"), Infinity); + assertEquals(result, "Hello"); });