From 3439c955abaeab503a4adf563ac5ed940665fe25 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 11 Dec 2018 19:30:14 +0100 Subject: [PATCH] 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 Reviewed-By: James M Snell --- lib/internal/process/warning.js | 98 ++++++++++++--------------------- 1 file changed, 34 insertions(+), 64 deletions(-) diff --git a/lib/internal/process/warning.js b/lib/internal/process/warning.js index 9416f15383b088..4d606b938949cf 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); }); }