Skip to content

Commit

Permalink
timers: propagate signal.reason in awaitable timers
Browse files Browse the repository at this point in the history
Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: #41008
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
  • Loading branch information
jasnell authored and danielleadams committed Dec 13, 2021
1 parent 12023df commit e795547
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 9 deletions.
21 changes: 12 additions & 9 deletions lib/timers/promises.js
Expand Up @@ -24,10 +24,10 @@ const {
validateObject,
} = require('internal/validators');

function cancelListenerHandler(clear, reject) {
function cancelListenerHandler(clear, reject, signal) {
if (!this._destroyed) {
clear(this);
reject(new AbortError());
reject(new AbortError(undefined, { cause: signal?.reason }));
}
}

Expand Down Expand Up @@ -57,7 +57,7 @@ function setTimeout(after, value, options = {}) {
// to 12.x, then this can be converted to use optional chaining to
// simplify the check.
if (signal && signal.aborted) {
return PromiseReject(new AbortError());
return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
}
let oncancel;
const ret = new Promise((resolve, reject) => {
Expand All @@ -66,7 +66,7 @@ function setTimeout(after, value, options = {}) {
if (signal) {
oncancel = FunctionPrototypeBind(cancelListenerHandler,
// eslint-disable-next-line no-undef
timeout, clearTimeout, reject);
timeout, clearTimeout, reject, signal);
signal.addEventListener('abort', oncancel);
}
});
Expand Down Expand Up @@ -101,7 +101,7 @@ function setImmediate(value, options = {}) {
// to 12.x, then this can be converted to use optional chaining to
// simplify the check.
if (signal && signal.aborted) {
return PromiseReject(new AbortError());
return PromiseReject(new AbortError(undefined, { cause: signal.reason }));
}
let oncancel;
const ret = new Promise((resolve, reject) => {
Expand All @@ -110,7 +110,8 @@ function setImmediate(value, options = {}) {
if (signal) {
oncancel = FunctionPrototypeBind(cancelListenerHandler,
// eslint-disable-next-line no-undef
immediate, clearImmediate, reject);
immediate, clearImmediate, reject,
signal);
signal.addEventListener('abort', oncancel);
}
});
Expand All @@ -127,7 +128,7 @@ async function* setInterval(after, value, options = {}) {
validateBoolean(ref, 'options.ref');

if (signal?.aborted)
throw new AbortError();
throw new AbortError(undefined, { cause: signal?.reason });

let onCancel;
let interval;
Expand All @@ -147,7 +148,9 @@ async function* setInterval(after, value, options = {}) {
// eslint-disable-next-line no-undef
clearInterval(interval);
if (callback) {
callback(PromiseReject(new AbortError()));
callback(
PromiseReject(
new AbortError(undefined, { cause: signal.reason })));
callback = undefined;
}
};
Expand All @@ -162,7 +165,7 @@ async function* setInterval(after, value, options = {}) {
yield value;
}
}
throw new AbortError();
throw new AbortError(undefined, { cause: signal?.reason });
} finally {
// eslint-disable-next-line no-undef
clearInterval(interval);
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-timers-immediate-promisified.js
Expand Up @@ -97,3 +97,10 @@ process.on('multipleResolves', common.mustNotCall());
assert.strictEqual(stderr, '');
}));
}

(async () => {
const signal = AbortSignal.abort('boom');
await assert.rejects(timerPromises.setImmediate(undefined, { signal }), {
cause: 'boom',
});
})().then(common.mustCall());
12 changes: 12 additions & 0 deletions test/parallel/test-timers-interval-promisified.js
Expand Up @@ -246,3 +246,15 @@ process.on('multipleResolves', common.mustNotCall());
setPromiseTimeout(time_unit * 3).then(() => post = true),
]).then(common.mustCall());
}

(async () => {
const signal = AbortSignal.abort('boom');
try {
const iterable = timerPromises.setInterval(2, undefined, { signal });
// eslint-disable-next-line no-unused-vars
for await (const _ of iterable) {}
assert.fail('should have failed');
} catch (err) {
assert.strictEqual(err.cause, 'boom');
}
})().then(common.mustCall());
7 changes: 7 additions & 0 deletions test/parallel/test-timers-timeout-promisified.js
Expand Up @@ -97,3 +97,10 @@ process.on('multipleResolves', common.mustNotCall());
assert.strictEqual(stderr, '');
}));
}

(async () => {
const signal = AbortSignal.abort('boom');
await assert.rejects(timerPromises.setTimeout(1, undefined, { signal }), {
cause: 'boom',
});
})().then(common.mustCall());

0 comments on commit e795547

Please sign in to comment.