process: improve `--redirect-warnings` handling

1) Acquiring the file descriptor is not observable anymore when using
   the `--redirect-warnings` flag.
2) If `fs.appendFile` fails, the warning is now redirected to the
   default output.
3) The code is smaller and simpler.

PR-URL: https://github.com/nodejs/node/pull/24965
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>

1 files changed, 34 insertions, 64 deletions

diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js
index 9416f15383..4d606b9389 100644
--- a/lib/internal/process/warning.js
+++ b/lib/internal/process/warning.js
@@ -5,84 +5,50 @@ const { ERR_INVALID_ARG_TYPE } = require('internal/errors').codes;
 
 exports.setup = setupProcessWarnings;
 
-let options;
-function lazyOption(name) {
-  if (!options) {
-    options = require('internal/options');
+// Lazily loaded
+let fs;
+let fd;
+let warningFile;
+
+function lazyOption() {
+  // This will load `warningFile` only once. If the flag is not set,
+  // `warningFile` will be set to an empty string.
+  if (warningFile === undefined) {
+    warningFile = require('internal/options')
+                  .getOptionValue('--redirect-warnings');
   }
-  return options.getOptionValue(name);
+  return warningFile;
 }
 
-var cachedFd;
-var acquiringFd = false;
-function nop() {}
-
-// Lazily loaded
-var fs = null;
-
 function writeOut(message) {
   if (console && typeof console.error === 'function')
     return console.error(message);
   process._rawDebug(message);
 }
 
-function onClose(fd) {
-  return () => {
-    if (fs === null) fs = require('fs');
+function writeToFile(message) {
+  if (fd === undefined) {
+    fs = require('fs');
     try {
-      fs.closeSync(fd);
-    } catch {}
-  };
-}
-
-function onOpen(cb) {
-  return (err, fd) => {
-    acquiringFd = false;
-    if (fd !== undefined) {
-      cachedFd = fd;
-      process.on('exit', onClose(fd));
-    }
-    cb(err, fd);
-    process.emit('_node_warning_fd_acquired', err, fd);
-  };
-}
-
-function onAcquired(message) {
-  // make a best effort attempt at writing the message
-  // to the fd. Errors are ignored at this point.
-  return (err, fd) => {
-    if (err)
+      fd = fs.openSync(warningFile, 'a');
+    } catch {
       return writeOut(message);
-    if (fs === null) fs = require('fs');
-    fs.appendFile(fd, `${message}\n`, nop);
-  };
-}
-
-function acquireFd(warningFile, cb) {
-  if (cachedFd === undefined && !acquiringFd) {
-    acquiringFd = true;
-    if (fs === null) fs = require('fs');
-    fs.open(warningFile, 'a', onOpen(cb));
-  } else if (cachedFd !== undefined && !acquiringFd) {
-    cb(null, cachedFd);
-  } else {
-    process.once('_node_warning_fd_acquired', cb);
-  }
-}
-
-function output(message) {
-  const warningFile = lazyOption('--redirect-warnings');
-  if (warningFile) {
-    acquireFd(warningFile, onAcquired(message));
-    return;
+    }
+    process.on('exit', () => {
+      try {
+        fs.closeSync(fd);
+      } catch {}
+    });
   }
-  writeOut(message);
+  fs.appendFile(fd, `${message}\n`, (err) => {
+    if (err) {
+      writeOut(message);
+    }
+  });
 }
 
 function doEmitWarning(warning) {
-  return () => {
-    process.emit('warning', warning);
-  };
+  return () => process.emit('warning', warning);
 }
 
 function setupProcessWarnings() {
@@ -107,7 +73,11 @@ function setupProcessWarnings() {
       if (typeof warning.detail === 'string') {
         msg += `\n${warning.detail}`;
       }
-      output(msg);
+      const warningFile = lazyOption();
+      if (warningFile) {
+        return writeToFile(msg);
+      }
+      writeOut(msg);
     });
   }