From 8d2960d7ccb756de4260a642d960cd01eaaa2478 Mon Sep 17 00:00:00 2001 From: /usr/bin/cat <109351887+cu8code@users.noreply.github.com> Date: Sat, 16 Nov 2024 20:31:19 +0530 Subject: [PATCH] fix(ext/node): New async setInterval function to improve the nodejs compatibility (#26703) Closes #26499 --- ext/node/polyfills/timers.ts | 88 ++++++++++++++++++- tests/unit_node/timers_test.ts | 151 +++++++++++++++++++++++++++++++++ 2 files changed, 237 insertions(+), 2 deletions(-) diff --git a/ext/node/polyfills/timers.ts b/ext/node/polyfills/timers.ts index 02f69466ee..e826416ed8 100644 --- a/ext/node/polyfills/timers.ts +++ b/ext/node/polyfills/timers.ts @@ -15,10 +15,16 @@ import { setUnrefTimeout, Timeout, } from "ext:deno_node/internal/timers.mjs"; -import { validateFunction } from "ext:deno_node/internal/validators.mjs"; +import { + validateAbortSignal, + validateBoolean, + validateFunction, + validateObject, +} from "ext:deno_node/internal/validators.mjs"; import { promisify } from "ext:deno_node/internal/util.mjs"; export { setUnrefTimeout } from "ext:deno_node/internal/timers.mjs"; import * as timers from "ext:deno_web/02_timers.js"; +import { AbortError } from "ext:deno_node/internal/errors.ts"; const clearTimeout_ = timers.clearTimeout; const clearInterval_ = timers.clearInterval; @@ -89,10 +95,88 @@ export function clearImmediate(immediate: Immediate) { clearTimeout_(immediate._immediateId); } +async function* setIntervalAsync( + after: number, + value: number, + options: { signal?: AbortSignal; ref?: boolean } = { __proto__: null }, +) { + validateObject(options, "options"); + + if (typeof options?.signal !== "undefined") { + validateAbortSignal(options.signal, "options.signal"); + } + + if (typeof options?.ref !== "undefined") { + validateBoolean(options.ref, "options.ref"); + } + + const { signal, ref = true } = options; + + if (signal?.aborted) { + throw new AbortError(undefined, { cause: signal?.reason }); + } + + let onCancel: (() => void) | undefined = undefined; + let interval: Timeout | undefined = undefined; + try { + let notYielded = 0; + let callback: ((value?: object) => void) | undefined = undefined; + let rejectCallback: ((message?: string) => void) | undefined = undefined; + interval = new Timeout( + () => { + notYielded++; + if (callback) { + callback(); + callback = undefined; + rejectCallback = undefined; + } + }, + after, + [], + true, + ref, + ); + if (signal) { + onCancel = () => { + clearInterval(interval); + if (rejectCallback) { + rejectCallback(signal.reason); + callback = undefined; + rejectCallback = undefined; + } + }; + signal.addEventListener("abort", onCancel, { once: true }); + } + while (!signal?.aborted) { + if (notYielded === 0) { + await new Promise((resolve: () => void, reject: () => void) => { + callback = resolve; + rejectCallback = reject; + }); + } + for (; notYielded > 0; notYielded--) { + yield value; + } + } + } catch (error) { + if (signal?.aborted) { + throw new AbortError(undefined, { cause: signal?.reason }); + } + throw error; + } finally { + if (interval) { + clearInterval(interval); + } + if (onCancel) { + signal?.removeEventListener("abort", onCancel); + } + } +} + export const promises = { setTimeout: promisify(setTimeout), setImmediate: promisify(setImmediate), - setInterval: promisify(setInterval), + setInterval: setIntervalAsync, }; promises.scheduler = { diff --git a/tests/unit_node/timers_test.ts b/tests/unit_node/timers_test.ts index 868ba21d64..c7dc30bbb7 100644 --- a/tests/unit_node/timers_test.ts +++ b/tests/unit_node/timers_test.ts @@ -3,6 +3,7 @@ import { assert, fail } from "@std/assert"; import * as timers from "node:timers"; import * as timersPromises from "node:timers/promises"; +import { assertEquals } from "@std/assert"; Deno.test("[node/timers setTimeout]", () => { { @@ -108,3 +109,153 @@ Deno.test("[node/timers setImmediate returns Immediate object]", () => { imm.hasRef(); clearImmediate(imm); }); + +Deno.test({ + name: "setInterval yields correct values at expected intervals", + async fn() { + // Test configuration + const CONFIG = { + expectedValue: 42, + intervalMs: 100, + iterations: 3, + tolerancePercent: 10, + }; + + const { setInterval } = timersPromises; + const results: Array<{ value: number; timestamp: number }> = []; + const startTime = Date.now(); + + const iterator = setInterval(CONFIG.intervalMs, CONFIG.expectedValue); + + for await (const value of iterator) { + results.push({ + value, + timestamp: Date.now(), + }); + if (results.length === CONFIG.iterations) { + break; + } + } + + const values = results.map((r) => r.value); + assertEquals( + values, + Array(CONFIG.iterations).fill(CONFIG.expectedValue), + `Each iteration should yield ${CONFIG.expectedValue}`, + ); + + const intervals = results.slice(1).map((result, index) => ({ + interval: result.timestamp - results[index].timestamp, + iterationNumber: index + 1, + })); + + const toleranceMs = (CONFIG.tolerancePercent / 100) * CONFIG.intervalMs; + const expectedRange = { + min: CONFIG.intervalMs - toleranceMs, + max: CONFIG.intervalMs + toleranceMs, + }; + + intervals.forEach(({ interval, iterationNumber }) => { + const isWithinTolerance = interval >= expectedRange.min && + interval <= expectedRange.max; + + assertEquals( + isWithinTolerance, + true, + `Iteration ${iterationNumber}: Interval ${interval}ms should be within ` + + `${expectedRange.min}ms and ${expectedRange.max}ms ` + + `(${CONFIG.tolerancePercent}% tolerance of ${CONFIG.intervalMs}ms)`, + ); + }); + + const totalDuration = results[results.length - 1].timestamp - startTime; + const expectedDuration = CONFIG.intervalMs * CONFIG.iterations; + const isDurationReasonable = + totalDuration >= (expectedDuration - toleranceMs) && + totalDuration <= (expectedDuration + toleranceMs); + + assertEquals( + isDurationReasonable, + true, + `Total duration ${totalDuration}ms should be close to ${expectedDuration}ms ` + + `(within ${toleranceMs}ms tolerance)`, + ); + + const timestamps = results.map((r) => r.timestamp); + const areTimestampsOrdered = timestamps.every((timestamp, i) => + i === 0 || timestamp > timestamps[i - 1] + ); + + assertEquals( + areTimestampsOrdered, + true, + "Timestamps should be strictly increasing", + ); + }, +}); + +Deno.test({ + name: "setInterval with AbortSignal stops after expected duration", + async fn() { + const INTERVAL_MS = 500; + const TOTAL_DURATION_MS = 3000; + const TOLERANCE_MS = 500; + + const abortController = new AbortController(); + const { setInterval } = timersPromises; + + // Set up abort after specified duration + const abortTimeout = timers.setTimeout(() => { + abortController.abort(); + }, TOTAL_DURATION_MS); + + // Track iterations and timing + const startTime = Date.now(); + const iterations: number[] = []; + + try { + for await ( + const _timestamp of setInterval(INTERVAL_MS, undefined, { + signal: abortController.signal, + }) + ) { + iterations.push(Date.now() - startTime); + } + } catch (error) { + if (error instanceof Error && error.name !== "AbortError") { + throw error; + } + } finally { + timers.clearTimeout(abortTimeout); + } + + // Validate timing + const totalDuration = iterations[iterations.length - 1]; + const isWithinTolerance = + totalDuration >= (TOTAL_DURATION_MS - TOLERANCE_MS) && + totalDuration <= (TOTAL_DURATION_MS + TOLERANCE_MS); + + assertEquals( + isWithinTolerance, + true, + `Total duration ${totalDuration}ms should be within ±${TOLERANCE_MS}ms of ${TOTAL_DURATION_MS}ms`, + ); + + // Validate interval consistency + const intervalDeltas = iterations.slice(1).map((time, i) => + time - iterations[i] + ); + + intervalDeltas.forEach((delta, i) => { + const isIntervalValid = delta >= (INTERVAL_MS - 50) && + delta <= (INTERVAL_MS + 50); + assertEquals( + isIntervalValid, + true, + `Interval ${ + i + 1 + } duration (${delta}ms) should be within ±50ms of ${INTERVAL_MS}ms`, + ); + }); + }, +});