FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

lib: aggregate errors to avoid error swallowing · nodejs/node@104dac7 · GitHub

/ node Public

Commit 104dac7

Browse files
aduh95
committed
lib: aggregate errors to avoid error swallowing
Uses `AggregateError` if there are more than one error with the message of the outer error to preserve the current behaviour, or returns the logical OR comparison of the two parameters. PR-URL: #37460 Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com>
1 parent 0ddd75b commit 104dac7

11 files changed

Lines changed: 147 additions & 17 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2336,6 +2336,10 @@ error `UV_ENOSYS`.
23362336
<!-- YAML
23372337
deprecated: v0.4.7
23382338
changes:
2339+
- version: REPLACEME
2340+
pr-url: https://github.com/nodejs/node/pull/37460
2341+
description: The error returned may be an `AggregateError` if more than one
2342+
error is returned.
23392343
- version: v10.0.0
23402344
pr-url: https://github.com/nodejs/node/pull/12562
23412345
description: The `callback` parameter is no longer optional. Not passing
@@ -2349,7 +2353,7 @@ changes:
23492353
* `path` {string|Buffer|URL}
23502354
* `mode` {integer}
23512355
* `callback` {Function}
2352-
* `err` {Error}
2356+
* `err` {Error|AggregateError}
23532357
23542358
Changes the permissions on a symbolic link. No arguments other than a possible
23552359
exception are given to the completion callback.
@@ -2812,6 +2816,10 @@ If `options.withFileTypes` is set to `true`, the `files` array will contain
28122816
<!-- YAML
28132817
added: v0.1.29
28142818
changes:
2819+
- version: REPLACEME
2820+
pr-url: https://github.com/nodejs/node/pull/37460
2821+
description: The error returned may be an `AggregateError` if more than one
2822+
error is returned.
28152823
- version: v15.2.0
28162824
pr-url: https://github.com/nodejs/node/pull/35911
28172825
description: The options argument may include an AbortSignal to abort an
@@ -2843,7 +2851,7 @@ changes:
28432851
* `flag` {string} See [support of file system `flags`][]. **Default:** `'r'`.
28442852
* `signal` {AbortSignal} allows aborting an in-progress readFile
28452853
* `callback` {Function}
2846-
* `err` {Error}
2854+
* `err` {Error|AggregateError}
28472855
* `data` {string|Buffer}
28482856
28492857
Asynchronously reads the entire contents of a file.
@@ -3390,6 +3398,10 @@ example/
33903398
<!-- YAML
33913399
added: v0.8.6
33923400
changes:
3401+
- version: REPLACEME
3402+
pr-url: https://github.com/nodejs/node/pull/37460
3403+
description: The error returned may be an `AggregateError` if more than one
3404+
error is returned.
33933405
- version: v10.0.0
33943406
pr-url: https://github.com/nodejs/node/pull/12562
33953407
description: The `callback` parameter is no longer optional. Not passing
@@ -3403,7 +3415,7 @@ changes:
34033415
* `path` {string|Buffer|URL}
34043416
* `len` {integer} **Default:** `0`
34053417
* `callback` {Function}
3406-
* `err` {Error}
3418+
* `err` {Error|AggregateError}
34073419

34083420
Truncates the file. No arguments other than a possible exception are
34093421
given to the completion callback. A file descriptor can also be passed as the
@@ -3843,6 +3855,10 @@ details.
38433855
<!-- YAML
38443856
added: v0.1.29
38453857
changes:
3858+
- version: REPLACEME
3859+
pr-url: https://github.com/nodejs/node/pull/37460
3860+
description: The error returned may be an `AggregateError` if more than one
3861+
error is returned.
38463862
- version: v15.2.0
38473863
pr-url: https://github.com/nodejs/node/pull/35993
38483864
description: The options argument may include an AbortSignal to abort an
@@ -3883,7 +3899,7 @@ changes:
38833899
* `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
38843900
* `signal` {AbortSignal} allows aborting an in-progress writeFile
38853901
* `callback` {Function}
3886-
* `err` {Error}
3902+
* `err` {Error|AggregateError}
38873903
38883904
When `file` is a filename, asynchronously writes data to the file, replacing the
38893905
file if it already exists. `data` can be a string or a buffer.

‎lib/fs.js‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ const { isArrayBufferView } = require('internal/util/types');
7474
const binding = internalBinding('fs');
7575
const { Buffer } = require('buffer');
7676
const {
77+
aggregateTwoErrors,
7778
codes: {
7879
ERR_FS_FILE_TOO_LARGE,
7980
ERR_INVALID_ARG_VALUE,
@@ -826,7 +827,7 @@ function truncate(path, len, callback) {
826827
const req = new FSReqCallback();
827828
req.oncomplete = function oncomplete(er) {
828829
fs.close(fd, (er2) => {
829-
callback(er || er2);
830+
callback(aggregateTwoErrors(er2, er));
830831
});
831832
};
832833
binding.ftruncate(fd, len, req);
@@ -1296,7 +1297,7 @@ function lchmod(path, mode, callback) {
12961297
// but still try to close, and report closing errors if they occur.
12971298
fs.fchmod(fd, mode, (err) => {
12981299
fs.close(fd, (err2) => {
1299-
callback(err || err2);
1300+
callback(aggregateTwoErrors(err2, err));
13001301
});
13011302
});
13021303
});
@@ -1461,11 +1462,12 @@ function lutimesSync(path, atime, mtime) {
14611462

14621463
function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
14631464
if (signal?.aborted) {
1465+
const abortError = new AbortError();
14641466
if (isUserFd) {
1465-
callback(new AbortError());
1467+
callback(abortError);
14661468
} else {
1467-
fs.close(fd, function() {
1468-
callback(new AbortError());
1469+
fs.close(fd, (err) => {
1470+
callback(aggregateTwoErrors(err, abortError));
14691471
});
14701472
}
14711473
return;
@@ -1476,8 +1478,8 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
14761478
if (isUserFd) {
14771479
callback(writeErr);
14781480
} else {
1479-
fs.close(fd, function close() {
1480-
callback(writeErr);
1481+
fs.close(fd, (err) => {
1482+
callback(aggregateTwoErrors(err, writeErr));
14811483
});
14821484
}
14831485
} else if (written === length) {

‎lib/internal/errors.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
// message may change, the code should not.
1212

1313
const {
14+
AggregateError,
1415
ArrayFrom,
1516
ArrayIsArray,
1617
ArrayPrototypeIncludes,
@@ -36,6 +37,7 @@ const {
3637
RangeError,
3738
ReflectApply,
3839
RegExpPrototypeTest,
40+
SafeArrayIterator,
3941
SafeMap,
4042
SafeWeakMap,
4143
String,
@@ -136,6 +138,24 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
136138
return kNoOverride;
137139
};
138140

141+
const aggregateTwoErrors = hideStackFrames((innerError, outerError) => {
142+
if (innerError && outerError) {
143+
if (ArrayIsArray(outerError.errors)) {
144+
// If `outerError` is already an `AggregateError`.
145+
ArrayPrototypePush(outerError.errors, innerError);
146+
return outerError;
147+
}
148+
// eslint-disable-next-line no-restricted-syntax
149+
const err = new AggregateError(new SafeArrayIterator([
150+
outerError,
151+
innerError,
152+
]), outerError.message);
153+
err.code = outerError.code;
154+
return err;
155+
}
156+
return innerError || outerError;
157+
});
158+
139159
// Lazily loaded
140160
let util;
141161
let assert;
@@ -752,6 +772,7 @@ class AbortError extends Error {
752772
}
753773
module.exports = {
754774
addCodeToName, // Exported for NghttpError
775+
aggregateTwoErrors,
755776
codes,
756777
dnsException,
757778
errnoException,

‎lib/internal/fs/read_file_context.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ const { FSReqCallback, close, read } = internalBinding('fs');
1212

1313
const {
1414
AbortError,
15+
aggregateTwoErrors,
1516
} = require('internal/errors');
1617

1718
// Use 64kb in case the file type is not a regular file and thus do not know the
@@ -50,7 +51,7 @@ function readFileAfterClose(err) {
5051
let buffer = null;
5152

5253
if (context.err || err)
53-
return callback(context.err || err);
54+
return callback(aggregateTwoErrors(err, context.err));
5455

5556
try {
5657
if (context.size === 0)

‎lib/internal/http2/core.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ const {
6565
},
6666
} = require('internal/async_hooks');
6767
const {
68+
aggregateTwoErrors,
6869
codes: {
6970
ERR_HTTP2_ALTSVC_INVALID_ORIGIN,
7071
ERR_HTTP2_ALTSVC_LENGTH,
@@ -2077,7 +2078,7 @@ class Http2Stream extends Duplex {
20772078
let endCheckCallbackErr;
20782079
const done = () => {
20792080
if (waitingForEndCheck || waitingForWriteCallback) return;
2080-
const err = writeCallbackErr || endCheckCallbackErr;
2081+
const err = aggregateTwoErrors(endCheckCallbackErr, writeCallbackErr);
20812082
// writeGeneric does not destroy on error and
20822083
// we cannot enable autoDestroy,
20832084
// so make sure to destroy on error.

‎lib/internal/per_context/primordials.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ function copyPrototype(src, dest, prefix) {
152152

153153
// Create copies of intrinsic objects
154154
[
155+
'AggregateError',
155156
'Array',
156157
'ArrayBuffer',
157158
'BigInt',

‎lib/internal/streams/destroy.js‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
'use strict';
22

33
const {
4-
ERR_MULTIPLE_CALLBACK
5-
} = require('internal/errors').codes;
4+
aggregateTwoErrors,
5+
codes: {
6+
ERR_MULTIPLE_CALLBACK,
7+
},
8+
} = require('internal/errors');
69
const {
710
FunctionPrototypeCall,
811
Symbol,
@@ -56,7 +59,7 @@ function destroy(err, cb) {
5659
// If still constructing then defer calling _destroy.
5760
if (!s.constructed) {
5861
this.once(kDestroy, function(er) {
59-
_destroy(this, err || er, cb);
62+
_destroy(this, aggregateTwoErrors(er, err), cb);
6063
});
6164
} else {
6265
_destroy(this, err, cb);

‎test/message/error_aggregateTwoErrors.js‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const { aggregateTwoErrors } = require('internal/errors');
6+
7+
const originalError = new Error('original');
8+
const err = new Error('second error');
9+
10+
originalError.code = 'ERR0';
11+
err.code = 'ERR1';
12+
13+
throw aggregateTwoErrors(err, originalError);

‎test/message/error_aggregateTwoErrors.out‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
*error_aggregateTwoErrors.js:*
2+
throw aggregateTwoErrors(err, originalError);
3+
^
4+
AggregateError: original
5+
at Object.<anonymous> (*test*message*error_aggregateTwoErrors.js:*:*)
6+
at Module._compile (node:internal/modules/cjs/loader:*:*)
7+
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*:*)
8+
at Module.load (node:internal/modules/cjs/loader:*:*)
9+
at Function.Module._load (node:internal/modules/cjs/loader:*:*)
10+
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*:*)
11+
at node:internal/main/run_main_module:*:* {
12+
code: 'ERR0'
13+
}

‎test/parallel/test-error-aggregateTwoErrors.js‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Flags: --expose-internals
2+
'use strict';
3+
4+
require('../common');
5+
const assert = require('assert');
6+
const { aggregateTwoErrors } = require('internal/errors');
7+
8+
assert.strictEqual(aggregateTwoErrors(null, null), null);
9+
10+
{
11+
const err = new Error();
12+
assert.strictEqual(aggregateTwoErrors(null, err), err);
13+
}
14+
15+
{
16+
const err = new Error();
17+
assert.strictEqual(aggregateTwoErrors(err, null), err);
18+
}
19+
20+
{
21+
const err0 = new Error('original');
22+
const err1 = new Error('second error');
23+
24+
err0.code = 'ERR0';
25+
err1.code = 'ERR1';
26+
27+
const chainedError = aggregateTwoErrors(err1, err0);
28+
assert.strictEqual(chainedError.message, err0.message);
29+
assert.strictEqual(chainedError.code, err0.code);
30+
assert.deepStrictEqual(chainedError.errors, [err0, err1]);
31+
}
32+
33+
{
34+
const err0 = new Error('original');
35+
const err1 = new Error('second error');
36+
const err2 = new Error('third error');
37+
38+
err0.code = 'ERR0';
39+
err1.code = 'ERR1';
40+
err2.code = 'ERR2';
41+
42+
const chainedError = aggregateTwoErrors(err2, aggregateTwoErrors(err1, err0));
43+
assert.strictEqual(chainedError.message, err0.message);
44+
assert.strictEqual(chainedError.code, err0.code);
45+
assert.deepStrictEqual(chainedError.errors, [err0, err1, err2]);
46+
}
47+
48+
{
49+
const err0 = new Error('original');
50+
const err1 = new Error('second error');
51+
52+
err0.code = 'ERR0';
53+
err1.code = 'ERR1';
54+
55+
const chainedError = aggregateTwoErrors(null, aggregateTwoErrors(err1, err0));
56+
assert.strictEqual(chainedError.message, err0.message);
57+
assert.strictEqual(chainedError.code, err0.code);
58+
assert.deepStrictEqual(chainedError.errors, [err0, err1]);
59+
}

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL