lib: add chainErrors API to avoid error swallowing

Author: aduh95

lib/_http_client.js

@@ -59,7 +59,7 @@ const { Buffer } = require('buffer');
 const { defaultTriggerAsyncIdScope } = require('internal/async_hooks');
 const { URL, urlToHttpOptions, searchParamsSymbol } = require('internal/url');
 const { kOutHeaders, kNeedDrain } = require('internal/http');
-const { connResetException, codes } = require('internal/errors');
+const { chainErrors, connResetException, codes } = require('internal/errors');
 const {
   ERR_HTTP_HEADERS_SENT,
   ERR_INVALID_ARG_TYPE,
@@ -789,14 +789,14 @@ function onSocketNT(req, socket, err) {
       if (!err && req.agent && !socket.destroyed) {
         socket.emit('free');
       } else {
-        finished(socket.destroy(err || req[kError]), (er) => {
-          _destroy(req, er || err);
+        finished(socket.destroy(chainErrors(req[kError], err)), (er) => {
+          _destroy(req, chainErrors(err, er));
         });
         return;
       }
     }
 
-    _destroy(req, err || req[kError]);
+    _destroy(req, chainErrors(req[kError], err));
   } else {
     tickOnSocket(req, socket);
     req._flush();

lib/fs.js

@@ -74,6 +74,7 @@ const { isArrayBufferView } = require('internal/util/types');
 const binding = internalBinding('fs');
 const { Buffer } = require('buffer');
 const {
+  chainErrors,
   codes: {
     ERR_FS_FILE_TOO_LARGE,
     ERR_INVALID_ARG_VALUE,
@@ -826,7 +827,7 @@ function truncate(path, len, callback) {
     const req = new FSReqCallback();
     req.oncomplete = function oncomplete(er) {
       fs.close(fd, (er2) => {
-        callback(er || er2);
+        callback(chainErrors(er2, er));
       });
     };
     binding.ftruncate(fd, len, req);
@@ -1294,7 +1295,7 @@ function lchmod(path, mode, callback) {
     // but still try to close, and report closing errors if they occur.
     fs.fchmod(fd, mode, (err) => {
       fs.close(fd, (err2) => {
-        callback(err || err2);
+        callback(chainErrors(err2, err));
       });
     });
   });
@@ -1462,8 +1463,11 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
     if (isUserFd) {
       callback(lazyDOMException('The operation was aborted', 'AbortError'));
     } else {
-      fs.close(fd, function() {
-        callback(lazyDOMException('The operation was aborted', 'AbortError'));
+      fs.close(fd, (err) => {
+        callback(chainErrors(
+          err,
+          lazyDOMException('The operation was aborted', 'AbortError')
+        ));
       });
     }
     return;
@@ -1474,8 +1478,8 @@ function writeAll(fd, isUserFd, buffer, offset, length, signal, callback) {
       if (isUserFd) {
         callback(writeErr);
       } else {
-        fs.close(fd, function close() {
-          callback(writeErr);
+        fs.close(fd, (err) => {
+          callback(chainErrors(err, writeErr));
         });
       }
     } else if (written === length) {

lib/internal/errors.js

@@ -111,6 +111,9 @@ const prepareStackTrace = (globalThis, error, trace) => {
   // Error: Message
   //     at function (file)
   //     at file
+  //   cause: Error: Message
+  //     at function (file)
+  //     at file
   const errorString = ErrorPrototypeToString(error);
   if (trace.length === 0) {
     return errorString;
@@ -136,6 +139,14 @@ const maybeOverridePrepareStackTrace = (globalThis, error, trace) => {
   return kNoOverride;
 };
 
+const chainErrors = (error, originalError) => {
+  if (error) {
+    error.cause = originalError;
+    return error;
+  }
+  return originalError;
+};
+
 // Lazily loaded
 let util;
 let assert;
@@ -752,6 +763,7 @@ class AbortError extends Error {
 }
 module.exports = {
   addCodeToName, // Exported for NghttpError
+  chainErrors,
   codes,
   dnsException,
   errnoException,

lib/internal/fs/read_file_context.js

@@ -10,7 +10,7 @@ const { Buffer } = require('buffer');
 
 const { FSReqCallback, close, read } = internalBinding('fs');
 
-const { hideStackFrames } = require('internal/errors');
+const { chainErrors, hideStackFrames } = require('internal/errors');
 
 
 let DOMException;
@@ -56,7 +56,7 @@ function readFileAfterClose(err) {
   let buffer = null;
 
   if (context.err || err)
-    return callback(context.err || err);
+    return callback(chainErrors(err, context.err));
 
   try {
     if (context.size === 0)

lib/internal/fs/streams.js

@@ -12,10 +12,13 @@ const {
 } = primordials;
 
 const {
-  ERR_INVALID_ARG_TYPE,
-  ERR_OUT_OF_RANGE,
-  ERR_METHOD_NOT_IMPLEMENTED,
-} = require('internal/errors').codes;
+  chainErrors,
+  codes: {
+    ERR_INVALID_ARG_TYPE,
+    ERR_OUT_OF_RANGE,
+    ERR_METHOD_NOT_IMPLEMENTED,
+  }
+} = require('internal/errors');
 const { deprecate } = require('internal/util');
 const {
   validateFunction,
@@ -276,7 +279,7 @@ ReadStream.prototype._destroy = function(err, cb) {
   // to close while used in a pending read or write operation. Wait for
   // any pending IO (kIsPerformingIO) to complete (kIoDone).
   if (this[kIsPerformingIO]) {
-    this.once(kIoDone, (er) => close(this, err || er, cb));
+    this.once(kIoDone, (er) => close(this, chainErrors(er, err), cb));
   } else {
     close(this, err, cb);
   }
@@ -437,7 +440,7 @@ WriteStream.prototype._destroy = function(err, cb) {
   // to close while used in a pending read or write operation. Wait for
   // any pending IO (kIsPerformingIO) to complete (kIoDone).
   if (this[kIsPerformingIO]) {
-    this.once(kIoDone, (er) => close(this, err || er, cb));
+    this.once(kIoDone, (er) => close(this, chainErrors(er, err), cb));
   } else {
     close(this, err, cb);
   }

lib/internal/http2/core.js

@@ -65,6 +65,7 @@ const {
   },
 } = require('internal/async_hooks');
 const {
+  chainErrors,
   codes: {
     ERR_HTTP2_ALTSVC_INVALID_ORIGIN,
     ERR_HTTP2_ALTSVC_LENGTH,
@@ -2077,7 +2078,7 @@ class Http2Stream extends Duplex {
     let endCheckCallbackErr;
     const done = () => {
       if (waitingForEndCheck || waitingForWriteCallback) return;
-      const err = writeCallbackErr || endCheckCallbackErr;
+      const err = chainErrors(endCheckCallbackErr, writeCallbackErr);
       // writeGeneric does not destroy on error and
       // we cannot enable autoDestroy,
       // so make sure to destroy on error.

lib/internal/streams/destroy.js

@@ -1,8 +1,11 @@
 'use strict';
 
 const {
-  ERR_MULTIPLE_CALLBACK
-} = require('internal/errors').codes;
+  chainErrors,
+  codes: {
+    ERR_MULTIPLE_CALLBACK,
+  },
+} = require('internal/errors');
 const {
   FunctionPrototypeCall,
   Symbol,
@@ -56,7 +59,7 @@ function destroy(err, cb) {
   // If still constructing then defer calling _destroy.
   if (!s.constructed) {
     this.once(kDestroy, function(er) {
-      _destroy(this, err || er, cb);
+      _destroy(this, chainErrors(er, err), cb);
     });
   } else {
     _destroy(this, err, cb);

test/message/error_chainErrors.js

@@ -0,0 +1,10 @@
+// Flags: --expose-internals
+'use strict';
+
+require('../common');
+const { chainErrors } = require('internal/errors');
+
+const originalError = new Error('original');
+const err = new Error('second error');
+
+throw chainErrors(err, originalError);

test/message/error_chainErrors.out

@@ -0,0 +1,20 @@
+*/error_chainErrors.js:*
+throw chainErrors(err, originalError);
+^
+Error: second error
+    at Object.<anonymous> (*test*message*error_chainErrors.js:*:*)
+    at Module._compile (node:internal/modules/cjs/loader:*:*)
+    at Object.Module._extensions..js (node:internal/modules/cjs/loader:*:*)
+    at Module.load (node:internal/modules/cjs/loader:*:*)
+    at Function.Module._load (node:internal/modules/cjs/loader:*:*)
+    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*:*)
+    at node:internal/main/run_main_module:*:* {
+  cause: Error: original
+      at Object.<anonymous> (*test*message*error_chainErrors.js:*:*)
+      at Module._compile (node:internal/modules/cjs/loader:*:*)
+      at Object.Module._extensions..js (node:internal/modules/cjs/loader:*:*)
+      at Module.load (node:internal/modules/cjs/loader:*:*)
+      at Function.Module._load (node:internal/modules/cjs/loader:*:*)
+      at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:*:*)
+      at node:internal/main/run_main_module:*:*
+}

test/parallel/test-error-chainErrors.js

@@ -0,0 +1,27 @@
+// Flags: --expose-internals
+'use strict';
+
+require('../common');
+const assert = require('assert');
+const { chainErrors } = require('internal/errors');
+
+assert.strictEqual(chainErrors(null, null), null);
+
+{
+  const err = new Error();
+  assert.strictEqual(chainErrors(null, err), err);
+}
+
+{
+  const err = new Error();
+  assert.strictEqual(chainErrors(err, null), err);
+}
+
+{
+  const originalError = new Error('original');
+  const err = new Error('second error');
+
+  const chainedError = chainErrors(err, originalError);
+  assert.strictEqual(chainedError, err);
+  assert.strictEqual(chainedError.cause, originalError);
+}