Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lib: propagate AbortSignal.reason as AbortError cause where appropriate #41008

Closed
wants to merge 8 commits into from
8 changes: 4 additions & 4 deletions lib/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ async function once(emitter, name, options = {}) {
const signal = options?.signal;
validateAbortSignal(signal, 'options.signal');
if (signal?.aborted)
throw new AbortError();
throw new AbortError(undefined, { cause: signal?.reason });
return new Promise((resolve, reject) => {
const errorListener = (err) => {
emitter.removeListener(name, resolver);
Expand All @@ -835,7 +835,7 @@ async function once(emitter, name, options = {}) {
function abortListener() {
eventTargetAgnosticRemoveListener(emitter, name, resolver);
eventTargetAgnosticRemoveListener(emitter, 'error', errorListener);
reject(new AbortError());
reject(new AbortError(undefined, { cause: signal?.reason }));
}
if (signal != null) {
eventTargetAgnosticAddListener(
Expand Down Expand Up @@ -888,7 +888,7 @@ function on(emitter, event, options) {
const signal = options?.signal;
validateAbortSignal(signal, 'options.signal');
if (signal?.aborted)
throw new AbortError();
throw new AbortError(undefined, { cause: signal?.reason });

const unconsumedEvents = [];
const unconsumedPromises = [];
Expand Down Expand Up @@ -976,7 +976,7 @@ function on(emitter, event, options) {
return iterator;

function abortListener() {
errorHandler(new AbortError());
errorHandler(new AbortError(undefined, { cause: signal?.reason }));
}

function eventHandler(...args) {
Expand Down
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ function readFileAfterStat(err, stats) {

function checkAborted(signal, callback) {
if (signal?.aborted) {
callback(new AbortError());
callback(new AbortError(undefined, { cause: signal?.reason }));
return true;
}
return false;
Expand Down Expand Up @@ -2050,7 +2050,7 @@ function lutimesSync(path, atime, mtime) {

function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
if (signal?.aborted) {
const abortError = new AbortError();
const abortError = new AbortError(undefined, { cause: signal?.reason });
if (isUserFd) {
callback(abortError);
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/blob.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ class Blob {

job.ondone = (err, ab) => {
if (err !== undefined)
return reject(new AbortError());
return reject(new AbortError(undefined, { cause: err }));
resolve(ab);
};
this[kArrayBufferPromise] =
Expand Down
7 changes: 5 additions & 2 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -821,8 +821,11 @@ function hideInternalStackFrames(error) {
// to make usage of the error in userland and readable-stream easier.
// It is a regular error with `.code` and `.name`.
class AbortError extends Error {
constructor() {
super('The operation was aborted');
constructor(message = 'The operation was aborted', options = undefined) {
if (options !== undefined && typeof options !== 'object') {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
throw new codes.ERR_INVALID_ARG_TYPE('options', 'Object', options);
}
super(message, options);
this.code = 'ABORT_ERR';
this.name = 'AbortError';
}
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ async function fsCall(fn, handle, ...args) {

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

async function writeFileHandle(filehandle, data, signal, encoding) {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/fs/read_file_context.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ class ReadFileContext {
let length;

if (this.signal?.aborted) {
return this.close(new AbortError());
return this.close(
new AbortError(undefined, { cause: this.signal?.reason }));
}
if (this.size === 0) {
buffer = Buffer.allocUnsafeSlow(kReadFileUnknownBufferLength);
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/fs/watchers.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,13 @@ async function* watch(filename, options = {}) {
}

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

const handle = new FSEvent();
let { promise, resolve, reject } = createDeferredPromise();
const oncancel = () => {
handle.close();
reject(new AbortError());
reject(new AbortError(undefined, { cause: signal?.reason }));
};

try {
Expand Down Expand Up @@ -361,7 +361,7 @@ async function* watch(filename, options = {}) {
yield await promise;
({ promise, resolve, reject } = createDeferredPromise());
}
throw new AbortError();
throw new AbortError(undefined, { cause: signal?.reason });
} finally {
handle.close();
signal?.removeEventListener('abort', oncancel);
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/http2/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -1778,7 +1778,9 @@ class ClientHttp2Session extends Http2Session {
const { signal } = options;
if (signal) {
validateAbortSignal(signal, 'options.signal');
const aborter = () => stream.destroy(new AbortError());
const aborter = () => {
stream.destroy(new AbortError(undefined, { cause: signal.reason }));
};
if (signal.aborted) {
aborter();
} else {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/streams/add-abort-signal.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ module.exports.addAbortSignalNoValidate = function(signal, stream) {
return stream;
}
const onAbort = () => {
stream.destroy(new AbortError());
stream.destroy(new AbortError(undefined, { cause: signal.reason }));
};
if (signal.aborted) {
onAbort();
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/streams/duplexify.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ function fromAsyncGen(fn) {
const { chunk, done, cb } = await _promise;
process.nextTick(cb);
if (done) return;
if (signal.aborted) throw new AbortError();
if (signal.aborted)
throw new AbortError(undefined, { cause: signal.reason });
({ promise, resolve } = createDeferredPromise());
yield chunk;
}
Expand Down
4 changes: 3 additions & 1 deletion lib/internal/streams/end-of-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,9 @@ function eos(stream, options, callback) {
// Keep it because cleanup removes it.
const endCallback = callback;
cleanup();
endCallback.call(stream, new AbortError());
endCallback.call(
stream,
new AbortError(undefined, { cause: options.signal.reason }));
};
if (options.signal.aborted) {
process.nextTick(abort);
Expand Down
6 changes: 2 additions & 4 deletions lib/internal/webstreams/adapters.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,7 @@ function newWritableStreamFromStreamWritable(streamWritable) {

const cleanup = finished(streamWritable, (error) => {
if (error?.code === 'ERR_STREAM_PREMATURE_CLOSE') {
const err = new AbortError();
err.cause = error;
const err = new AbortError(undefined, { cause: error });
error = err;
}

Expand Down Expand Up @@ -403,8 +402,7 @@ function newReadableStreamFromStreamReadable(streamReadable) {

const cleanup = finished(streamReadable, (error) => {
if (error?.code === 'ERR_STREAM_PREMATURE_CLOSE') {
const err = new AbortError();
err.cause = error;
const err = new AbortError(undefined, { cause: error });
error = err;
}

Expand Down
5 changes: 3 additions & 2 deletions lib/readline.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,16 @@ Interface.prototype.question[promisify.custom] = function(query, options) {
options = typeof options === 'object' && options !== null ? options : {};

if (options.signal && options.signal.aborted) {
return PromiseReject(new AbortError());
return PromiseReject(
new AbortError(undefined, { cause: options.signal.reason }));
}

return new Promise((resolve, reject) => {
let cb = resolve;

if (options.signal) {
const onAbort = () => {
reject(new AbortError());
reject(new AbortError(undefined, { cause: options.signal.reason }));
};
options.signal.addEventListener('abort', onAbort, { once: true });
cb = (answer) => {
Expand Down
5 changes: 3 additions & 2 deletions lib/readline/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@ class Interface extends _Interface {
if (options?.signal) {
validateAbortSignal(options.signal, 'options.signal');
if (options.signal.aborted) {
return reject(new AbortError());
return reject(
new AbortError(undefined, { cause: options.signal.reason }));
}

const onAbort = () => {
this[kQuestionCancel]();
reject(new AbortError());
reject(new AbortError(undefined, { cause: options.signal.reason }));
};
options.signal.addEventListener('abort', onAbort, { once: true });
cb = (answer) => {
Expand Down
21 changes: 12 additions & 9 deletions lib/timers/promises.js
Original file line number Diff line number Diff line change
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
31 changes: 31 additions & 0 deletions test/parallel/test-errors-aborterror.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Flags: --expose-internals
'use strict';

require('../common');
const {
strictEqual,
throws,
} = require('assert');
const { AbortError } = require('internal/errors');

{
const err = new AbortError();
strictEqual(err.message, 'The operation was aborted');
strictEqual(err.cause, undefined);
}

{
const cause = new Error('boom');
const err = new AbortError('bang', { cause });
strictEqual(err.message, 'bang');
strictEqual(err.cause, cause);
}

{
throws(() => new AbortError('', false), {
code: 'ERR_INVALID_ARG_TYPE'
});
throws(() => new AbortError('', ''), {
code: 'ERR_INVALID_ARG_TYPE'
});
}
9 changes: 9 additions & 0 deletions test/parallel/test-readline-promises-interface.js
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,15 @@ for (let i = 0; i < 12; i++) {
rli.close();
}

(async () => {
const [rli] = getInterface({ terminal });
const signal = AbortSignal.abort('boom');
await assert.rejects(rli.question('hello', { signal }), {
cause: 'boom',
});
rli.close();
})().then(common.mustCall());

// Throw an error when question is executed with an aborted signal
{
const ac = new AbortController();
Expand Down
7 changes: 7 additions & 0 deletions test/parallel/test-timers-immediate-promisified.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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());