Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add test-fs-read-buffer-to-string-fail as pass and flaky to all… #14472

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@ added: v6.0.0

Print stack traces for process warnings (including deprecations).

### `--redirect-warnings=file`
<!-- YAML
added: REPLACEME
-->

Write process warnings to the given file instead of printing to stderr. The
file will be created if it does not exist, and will be appended to if it does.
If an error occurs while attempting to write the warning to the file, the
warning will be written to stderr instead.

### `--trace-sync-io`
<!-- YAML
added: v2.1.0
Expand Down Expand Up @@ -382,6 +392,17 @@ Note: Be aware that unless the child environment is explicitly set, this
evironment variable will be inherited by any child processes, and if they use
OpenSSL, it may cause them to trust the same CAs as node.

### `NODE_REDIRECT_WARNINGS=file`
<!-- YAML
added: REPLACEME
-->

When set, process warnings will be emitted to the given file instead of
printing to stderr. The file will be created if it does not exist, and will be
appended to if it does. If an error occurs while attempting to write the
warning to the file, the warning will be written to stderr instead. This is
equivalent to using the `--redirect-warnings=file` command-line flag.

[emit_warning]: process.html#process_process_emitwarning_warning_name_ctor
[Buffer]: buffer.html#buffer_buffer
[debugger]: debugger.html
Expand Down
10 changes: 10 additions & 0 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,10 @@ Silence all process warnings (including deprecations).
.BR \-\-trace\-warnings
Print stack traces for process warnings (including deprecations).

.TP
.BR \-\-redirect\-warnings=\fIfile\fR
Write process warnings to the given file instead of printing to stderr.

.TP
.BR \-\-trace\-sync\-io
Print a stack trace whenever synchronous I/O is detected after the first turn
Expand Down Expand Up @@ -255,6 +259,12 @@ containing trusted certificates.
If \fB\-\-use\-openssl\-ca\fR is enabled, this overrides and sets OpenSSL's
file containing trusted certificates.

.TP
.BR NODE_REDIRECT_WARNINGS=\fIfile\fR
Write process warnings to the given file instead of printing to stderr.
(equivalent to using the \-\-redirect\-warnings=\fIfile\fR command-line
argument).

.SH BUGS
Bugs are tracked in GitHub Issues:
.ur https://github.com/nodejs/node/issues
Expand Down
75 changes: 73 additions & 2 deletions lib/internal/process/warning.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,81 @@ const traceWarnings = process.traceProcessWarnings;
const noDeprecation = process.noDeprecation;
const traceDeprecation = process.traceDeprecation;
const throwDeprecation = process.throwDeprecation;
const config = process.binding('config');
const prefix = `(${process.release.name}:${process.pid}) `;

exports.setup = setupProcessWarnings;

var fs;
var cachedFd;
var acquiringFd = false;
function nop() {}

function lazyFs() {
if (!fs)
fs = require('fs');
return fs;
}

function writeOut(message) {
if (console && typeof console.error === 'function')
return console.error(message);
process._rawDebug(message);
}

function onClose(fd) {
return function() {
lazyFs().close(fd, nop);
};
}

function onOpen(cb) {
return function(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 function(err, fd) {
if (err)
return writeOut(message);
lazyFs().appendFile(fd, `${message}\n`, nop);
};
}

function acquireFd(cb) {
if (cachedFd === undefined && !acquiringFd) {
acquiringFd = true;
lazyFs().open(config.warningFile, 'a', onOpen(cb));
} else if (cachedFd !== undefined && !acquiringFd) {
cb(null, cachedFd);
} else {
process.once('_node_warning_fd_acquired', cb);
}
}

function output(message) {
if (typeof config.warningFile === 'string') {
acquireFd(onAcquired(message));
return;
}
writeOut(message);
}

function doEmitWarning(warning) {
return function() {
process.emit('warning', warning);
};
}

function setupProcessWarnings() {
if (!process.noProcessWarnings && process.env.NODE_NO_WARNINGS !== '1') {
process.on('warning', (warning) => {
Expand All @@ -21,7 +92,7 @@ function setupProcessWarnings() {
var toString = warning.toString;
if (typeof toString !== 'function')
toString = Error.prototype.toString;
console.error(`${prefix}${toString.apply(warning)}`);
output(`${prefix}${toString.apply(warning)}`);
}
});
}
Expand All @@ -44,6 +115,6 @@ function setupProcessWarnings() {
if (throwDeprecation && warning.name === 'DeprecationWarning')
throw warning;
else
process.nextTick(() => process.emit('warning', warning));
process.nextTick(doEmitWarning(warning));
};
}
12 changes: 12 additions & 0 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ bool trace_warnings = false;
// that is used by lib/module.js
bool config_preserve_symlinks = false;

// Set in node.cc by ParseArgs when --redirect-warnings= is used.
const char* config_warning_file;

// process-relative uptime base, initialized at start-up
static double prog_start_time;
static bool debugger_running;
Expand Down Expand Up @@ -3683,6 +3686,8 @@ static void PrintHelp() {
"function is used\n"
" --no-warnings silence all process warnings\n"
" --trace-warnings show stack traces on process warnings\n"
" --redirect-warnings=path\n"
" write warnings to path instead of stderr\n"
" --trace-sync-io show stack trace when use of sync IO\n"
" is detected after the first tick\n"
" --track-heap-objects track heap object allocations for heap "
Expand Down Expand Up @@ -3747,6 +3752,7 @@ static void PrintHelp() {
" prefixed to the module search path\n"
"NODE_REPL_HISTORY path to the persistent REPL history file\n"
"OPENSSL_CONF load OpenSSL configuration from file\n"
"NODE_REDIRECT_WARNINGS write warnings to path instead of stderr\n"
"\n"
"Documentation can be found at https://nodejs.org/\n");
}
Expand Down Expand Up @@ -3848,6 +3854,8 @@ static void ParseArgs(int* argc,
no_process_warnings = true;
} else if (strcmp(arg, "--trace-warnings") == 0) {
trace_warnings = true;
} else if (strncmp(arg, "--redirect-warnings=", 20) == 0) {
config_warning_file = arg + 20;
} else if (strcmp(arg, "--trace-deprecation") == 0) {
trace_deprecation = true;
} else if (strcmp(arg, "--trace-sync-io") == 0) {
Expand Down Expand Up @@ -4383,6 +4391,10 @@ void Init(int* argc,
if (openssl_config.empty())
SafeGetenv("OPENSSL_CONF", &openssl_config);

if (auto redirect_warnings = secure_getenv("NODE_REDIRECT_WARNINGS")) {
config_warning_file = redirect_warnings;
}

// Parse a few arguments which are specific to Node.
int v8_argc;
const char** v8_argv;
Expand Down
10 changes: 10 additions & 0 deletions src/node_config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ using v8::Context;
using v8::Local;
using v8::Object;
using v8::ReadOnly;
using v8::String;
using v8::Value;

// The config binding is used to provide an internal view of compile or runtime
Expand Down Expand Up @@ -44,6 +45,15 @@ void InitConfig(Local<Object> target,

if (config_preserve_symlinks)
READONLY_BOOLEAN_PROPERTY("preserveSymlinks");

if (config_warning_file != nullptr) {
Local<String> name = OneByteString(env->isolate(), "warningFile");
Local<String> value = String::NewFromUtf8(env->isolate(),
config_warning_file,
v8::NewStringType::kNormal)
.ToLocalChecked();
target->DefineOwnProperty(env->context(), name, value).FromJust();
}
} // InitConfig

} // namespace node
Expand Down
5 changes: 5 additions & 0 deletions src/node_internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,11 @@ extern std::string openssl_config;
// that is used by lib/module.js
extern bool config_preserve_symlinks;

// Set in node.cc by ParseArgs when --redirect-warnings= is used.
// Used to redirect warning output to a file rather than sending
// it to stderr.
extern const char* config_warning_file;

// Forward declaration
class Environment;

Expand Down
1 change: 1 addition & 0 deletions test/parallel/parallel.status
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ prefix parallel
# sample-test : PASS,FLAKY

[true] # This section applies to all platforms
test-fs-read-buffer-tostring-fail : PASS,FLAKY

[$system==win32]

Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-process-redirect-warnings-env.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';

// Tests the NODE_REDIRECT_WARNINGS environment variable by spawning
// a new child node process that emits a warning into a temporary
// warnings file. Once the process completes, the warning file is
// opened and the contents are validated

const common = require('../common');
const fs = require('fs');
const fork = require('child_process').fork;
const path = require('path');
const assert = require('assert');

common.refreshTmpDir();

const warnmod = require.resolve(common.fixturesDir + '/warnings.js');
const warnpath = path.join(common.tmpDir, 'warnings.txt');

fork(warnmod, {env: {NODE_REDIRECT_WARNINGS: warnpath}})
.on('exit', common.mustCall(() => {
fs.readFile(warnpath, 'utf8', common.mustCall((err, data) => {
assert.ifError(err);
assert(/\(node:\d+\) Warning: a bad practice warning/.test(data));
}));
}));
25 changes: 25 additions & 0 deletions test/parallel/test-process-redirect-warnings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';

// Tests the --redirect-warnings command line flag by spawning
// a new child node process that emits a warning into a temporary
// warnings file. Once the process completes, the warning file is
// opened and the contents are validated

const common = require('../common');
const fs = require('fs');
const fork = require('child_process').fork;
const path = require('path');
const assert = require('assert');

common.refreshTmpDir();

const warnmod = require.resolve(common.fixturesDir + '/warnings.js');
const warnpath = path.join(common.tmpDir, 'warnings.txt');

fork(warnmod, {execArgv: [`--redirect-warnings=${warnpath}`]})
.on('exit', common.mustCall(() => {
fs.readFile(warnpath, 'utf8', common.mustCall((err, data) => {
assert.ifError(err);
assert(/\(node:\d+\) Warning: a bad practice warning/.test(data));
}));
}));