timers: fix validation

PR-URL: #54404 Reviewed-By: Claudio Wunder <cwunder@gnome.org>
nodejs · Aug 25, 2024 · 6d654dd · 6d654dd
1 parent 4e68b54
commit 6d654dd
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 66 deletions.
diff --git a/doc/api/timers.md b/doc/api/timers.md
@@ -532,9 +532,8 @@ added:
 An experimental API defined by the [Scheduling APIs][] draft specification
 being developed as a standard Web Platform API.
 
-Calling `timersPromises.scheduler.wait(delay, options)` is roughly equivalent
-to calling `timersPromises.setTimeout(delay, undefined, options)` except that
-the `ref` option is not supported.
+Calling `timersPromises.scheduler.wait(delay, options)` is equivalent
+to calling `timersPromises.setTimeout(delay, undefined, options)`.
 
 ```mjs
 import { scheduler } from 'node:timers/promises';

diff --git a/lib/timers/promises.js b/lib/timers/promises.js
@@ -24,7 +24,6 @@ const {
   AbortError,
   codes: {
     ERR_ILLEGAL_CONSTRUCTOR,
-    ERR_INVALID_ARG_TYPE,
     ERR_INVALID_THIS,
   },
 } = require('internal/errors');
@@ -33,6 +32,7 @@ const {
   validateAbortSignal,
   validateBoolean,
   validateObject,
+  validateNumber,
 } = require('internal/validators');
 
 const {
@@ -50,34 +50,33 @@ function cancelListenerHandler(clear, reject, signal) {
 }
 
 function setTimeout(after, value, options = kEmptyObject) {
-  const args = value !== undefined ? [value] : value;
-  if (options == null || typeof options !== 'object') {
-    return PromiseReject(
-      new ERR_INVALID_ARG_TYPE(
-        'options',
-        'Object',
-        options));
-  }
-  const { signal, ref = true } = options;
   try {
-    validateAbortSignal(signal, 'options.signal');
+    if (typeof after !== 'undefined') {
+      validateNumber(after, 'delay');
+    }
+
+    validateObject(options, 'options');
+
+    if (typeof options?.signal !== 'undefined') {
+      validateAbortSignal(options.signal, 'options.signal');
+    }
+
+    if (typeof options?.ref !== 'undefined') {
+      validateBoolean(options.ref, 'options.ref');
+    }
   } catch (err) {
     return PromiseReject(err);
   }
-  if (typeof ref !== 'boolean') {
-    return PromiseReject(
-      new ERR_INVALID_ARG_TYPE(
-        'options.ref',
-        'boolean',
-        ref));
-  }
+
+  const { signal, ref = true } = options;
 
   if (signal?.aborted) {
     return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
   }
+
   let oncancel;
   const ret = new Promise((resolve, reject) => {
-    const timeout = new Timeout(resolve, after, args, false, ref);
+    const timeout = new Timeout(resolve, after, [value], false, ref);
     insert(timeout, timeout._idleTimeout);
     if (signal) {
       oncancel = FunctionPrototypeBind(cancelListenerHandler,
@@ -93,30 +92,26 @@ function setTimeout(after, value, options = kEmptyObject) {
 }
 
 function setImmediate(value, options = kEmptyObject) {
-  if (options == null || typeof options !== 'object') {
-    return PromiseReject(
-      new ERR_INVALID_ARG_TYPE(
-        'options',
-        'Object',
-        options));
-  }
-  const { signal, ref = true } = options;
   try {
-    validateAbortSignal(signal, 'options.signal');
+    validateObject(options, 'options');
+
+    if (typeof options?.signal !== 'undefined') {
+      validateAbortSignal(options.signal, 'options.signal');
+    }
+
+    if (typeof options?.ref !== 'undefined') {
+      validateBoolean(options.ref, 'options.ref');
+    }
   } catch (err) {
     return PromiseReject(err);
   }
-  if (typeof ref !== 'boolean') {
-    return PromiseReject(
-      new ERR_INVALID_ARG_TYPE(
-        'options.ref',
-        'boolean',
-        ref));
-  }
+
+  const { signal, ref = true } = options;
 
   if (signal?.aborted) {
     return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
   }
+
   let oncancel;
   const ret = new Promise((resolve, reject) => {
     const immediate = new Immediate(resolve, [value]);
@@ -136,13 +131,29 @@ function setImmediate(value, options = kEmptyObject) {
 }
 
 async function* setInterval(after, value, options = kEmptyObject) {
-  validateObject(options, 'options');
+  try {
+    if (typeof after !== 'undefined') {
+      validateNumber(after, 'delay');
+    }
+
+    validateObject(options, 'options');
+
+    if (typeof options?.signal !== 'undefined') {
+      validateAbortSignal(options.signal, 'options.signal');
+    }
+
+    if (typeof options?.ref !== 'undefined') {
+      validateBoolean(options.ref, 'options.ref');
+    }
+  } catch (err) {
+    return PromiseReject(err);
+  }
+
   const { signal, ref = true } = options;
-  validateAbortSignal(signal, 'options.signal');
-  validateBoolean(ref, 'options.ref');
 
-  if (signal?.aborted)
+  if (signal?.aborted) {
     throw new AbortError(undefined, { cause: signal?.reason });
+  }
 
   let onCancel;
   let interval;
@@ -216,7 +227,7 @@ class Scheduler {
   wait(delay, options) {
     if (!this[kScheduler])
       throw new ERR_INVALID_THIS('Scheduler');
-    return setTimeout(delay, undefined, { signal: options?.signal });
+    return setTimeout(delay, undefined, options);
   }
 }
 

diff --git a/test/parallel/test-timers-timeout-promisified.js b/test/parallel/test-timers-timeout-promisified.js
@@ -63,29 +63,21 @@ process.on('multipleResolves', common.mustNotCall());
 }
 
 {
-  Promise.all(
-    [1, '', false, Infinity].map(
-      (i) => assert.rejects(setPromiseTimeout(10, null, i), {
-        code: 'ERR_INVALID_ARG_TYPE'
-      })
-    )
-  ).then(common.mustCall());
-
-  Promise.all(
-    [1, '', false, Infinity, null, {}].map(
-      (signal) => assert.rejects(setPromiseTimeout(10, null, { signal }), {
-        code: 'ERR_INVALID_ARG_TYPE'
-      })
-    )
-  ).then(common.mustCall());
-
-  Promise.all(
-    [1, '', Infinity, null, {}].map(
-      (ref) => assert.rejects(setPromiseTimeout(10, null, { ref }), {
-        code: 'ERR_INVALID_ARG_TYPE'
-      })
-    )
-  ).then(common.mustCall());
+  for (const delay of ['', false]) {
+    assert.rejects(() => setPromiseTimeout(delay, null, {}), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
+  }
+
+  for (const options of [1, '', false, Infinity]) {
+    assert.rejects(() => setPromiseTimeout(10, null, options), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
+  }
+
+  for (const signal of [1, '', false, Infinity, null, {}]) {
+    assert.rejects(() => setPromiseTimeout(10, null, { signal }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
+  }
+
+  for (const ref of [1, '', Infinity, null, {}]) {
+    assert.rejects(() => setPromiseTimeout(10, null, { ref }), /ERR_INVALID_ARG_TYPE/).then(common.mustCall());
+  }
 }
 
 {