Skip to content

Commit

Permalink
process: improve --redirect-warnings handling
Browse files Browse the repository at this point in the history
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: nodejs#24965
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
BridgeAR authored and refack committed Jan 10, 2019
1 parent 31a05a8 commit 55c2523
Showing 1 changed file with 34 additions and 64 deletions.
98 changes: 34 additions & 64 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);
});
}

Expand Down

0 comments on commit 55c2523

Please sign in to comment.