From 84027f83a596c8595d479cc5f6d43c1544db1065 Mon Sep 17 00:00:00 2001 From: Wyatt Preul Date: Thu, 25 Feb 2016 16:15:10 -0600 Subject: [PATCH] process: add 'warning' event --- doc/api/process.markdown | 166 ++++++++++++++++++++ lib/events.js | 14 +- lib/internal/util.js | 9 +- src/node.cc | 20 +++ src/node.js | 36 +++++ test/fixtures/warnings.js | 15 ++ test/parallel/test-global-console-exists.js | 25 +-- test/parallel/test-process-emitwarning.js | 30 ++++ test/sequential/test-deprecation-flags.js | 33 ++-- test/sequential/test-process-warnings.js | 39 +++++ 10 files changed, 353 insertions(+), 34 deletions(-) create mode 100644 test/fixtures/warnings.js create mode 100644 test/parallel/test-process-emitwarning.js create mode 100644 test/sequential/test-process-warnings.js diff --git a/doc/api/process.markdown b/doc/api/process.markdown index ca2971328025c5..2b08de93e6e44a 100644 --- a/doc/api/process.markdown +++ b/doc/api/process.markdown @@ -183,6 +183,84 @@ this, you can either attach a dummy `.catch(() => { })` handler to `resource.loaded`, preventing the `'unhandledRejection'` event from being emitted, or you can use the [`'rejectionHandled'`][] event. +## Event: 'warning' + +Emitted whenever Node.js emits a process warning. + +A process warning is similar to errors in that they describe exceptional +conditions that are being brought to the user's attention. However, they are not, +however, part of the normal Node.js and JavaScript error handling flow. Node.js +can emit warnings whenever it detects: (a) the use of deprecated APIs, (b) bad +coding practices that could lead to sub-optimal application performance or bugs. + +The event handler for `'warning'` events is called with a single `warning` +argument whose value is an `Error` object. There are three key properties that +describe the warning: + +* `name` - The name of the warning (currently either `DeprecationWarning`, + `BadPracticeWarning`). +* `message` - A system-provided description of the warning. +* `stack` - A stack trace to the location in the code where the warning was + issued. + +```js +process.on('warning', (warning) => { + console.warn(warning.name); // Print the warning name + console.warn(warning.message); // Print the warning message + console.warn(warning.stack); // Print the stack trace +}); +``` + +By default, Node.js will print process warnings to `stderr`. The `--no-warnings` +command-line option can be used to suppress the default console output but the +`'warning'` event will still be emitted by the `process` object. + +The following example illustrates the default deprecation warning that is +printed to `stderr` when a deprecated API is used: + +``` +$ node +> util.debug('foo'); +DEBUG: foo +undefined +> DeprecationWarning: (node:94742) util.debug is deprecated. Use +console.error instead. +``` + +In contrast, the following example turns off the default warning output and adds +a custom handler to the `'warning'` event: + +``` +$ node --no-warnings +> var p = process.on('warning', (warning) => console.warn('DEPRECATED API!')); +> util.debug('foo'); +DEBUG: foo +undefined +> DEPRECATED API! +``` + +The `--trace-warnings` command-line option can be used to have the default +console output for warnings include the full stack trace of the warning. + +### Warning Types + +Node.js can produce two types of warnings: + +* `BadPracticeWarning` - a warning indicating that parts of the Node.js API are + being used in a manner that is not recommended or has known issues. +* `DeprecationWarning` - a warning indicating that a deprecated feature is + being used. + +### Emitting Custom Warnings + +The `process.emitWarning(message, name)` method can be used issue custom or +application specific warnings. + +```js +// Emit a warning using an Error object... +process.emitWarning('Warning! Something happened!', 'CustomWarning'); +``` + ## Exit Codes Node.js will normally exit with a `0` status code when no more async @@ -395,6 +473,94 @@ Identical to the parent process's [`ChildProcess.disconnect()`][]. If Node.js was not spawned with an IPC channel, `process.disconnect()` will be undefined. +## process.emitWarning(warning[, name]) + +The `process.emitWarning(warning[, name])` method can be used to emit custom +or application specific process warnings. These can be listened for by adding +a handler to the [`process.emit('warning')`][process_warning] event. + +* `warning` {String|Error} The warning to issue +* `name` {String} the name representing the warning. Defaults to 'Warning' + +The `warning` argument can be either a string or an Error object. If a `name` +is supplied, it will be set on the resulting Error object, overwriting any +existing `name` property on the Error object. + +```js +// Emit a warning using a string... +process.emitWarning('Warning! Something happened!'); + // Emits: Warning: (node:56338) Warning! Something happened! +``` + +In the previous example, an `Error` object is generated and passed +through to the [`process.emit('warning')`][process_warning] event. + +```js +process.on('warning', (warning) => { + console.warn(warning.name); + console.warn(warning.message); + console.warn(warning.stack); +}); +``` + +If `warning` is passed as an `Error` object, it will be passed through to the +`process.emit('warning')` event handler. If a name is also passed in, then it +will be used to set the `name` property on the Error if one doesn't already +exist: + +```js +// Emit a warning using an Error object... + +const myWarning = new Error('Warning! Something happened!'); +myWarning.name = 'CustomWarning'; + +process.emitWarning(myWarning); + // Emits: CustomWarning: (node:56338) Warning! Something Happened! +``` + +```js +// Emit a warning using an Error object and name... + +const myWarning = new Error('Warning! Something happened!'); + +process.emitWarning(myWarning, 'CustomWarning'); + // Emits: CustomWarning: (node:56338) Warning! Something Happened! +``` + +```js +// Emit a warning using a string + +process.emitWarning('Warning! Something happened!', 'CustomWarning'); + // Emits: CustomWarning: (node:56338) Warning! Something Happened! +``` + + +A `TypeError` is thrown if `warning` is anything other than a string, or `Error` +object. + +Note that while process warnings use `Error` objects, the process warning +mechanism is **not** a replacement for normal error handling mechanisms. + +### Avoiding duplicate warnings + +As a best practice, warnings should be emitted only once per process. To do +so, it is recommended to place the `emitWarning()` behind a simple boolean +flag as illustrated in the example below: + +``` +var warned = false; +function emitMyWarning() { + if (!warned) { + process.emitWarning('Only warn once!'); + warned = true; + } +} +emitMyWarning(); + // Emits: Warning: (node: 56339) Only warn once! +emitMyWarning(); + // Emits nothing +``` + ## process.env An object containing the user environment. See environ(7). diff --git a/lib/events.js b/lib/events.js index 5860254654d8f1..21181aca68e4f2 100644 --- a/lib/events.js +++ b/lib/events.js @@ -1,6 +1,5 @@ 'use strict'; -var internalUtil; var domain; function EventEmitter() { @@ -246,14 +245,11 @@ EventEmitter.prototype.addListener = function addListener(type, listener) { m = $getMaxListeners(this); if (m && m > 0 && existing.length > m) { existing.warned = true; - if (!internalUtil) - internalUtil = require('internal/util'); - - internalUtil.error('warning: possible EventEmitter memory ' + - 'leak detected. %d %s listeners added. ' + - 'Use emitter.setMaxListeners() to increase limit.', - existing.length, type); - console.trace(); + + const warning = `warning: possible EventEmitter memory ` + + `leak detected. ${existing.length} ${type} listeners added. ` + + `Use emitter.setMaxListeners() to increase limit.`; + process.emitWarning(warning, 'BadPracticeWarning'); } } } diff --git a/lib/internal/util.js b/lib/internal/util.js index 21aafff21824b3..f71b42dbb79d43 100644 --- a/lib/internal/util.js +++ b/lib/internal/util.js @@ -44,10 +44,11 @@ exports._printDeprecationMessage = function(msg, warned) { if (process.throwDeprecation) throw new Error(msg); - else if (process.traceDeprecation) - console.trace(msg.startsWith(prefix) ? msg.replace(prefix, '') : msg); - else - console.error(msg); + + if (process.traceDeprecation) + msg = msg.startsWith(prefix) ? msg.replace(prefix, '') : msg; + + process.emitWarning(msg || 'deprecated', 'DeprecationWarning'); return true; }; diff --git a/src/node.cc b/src/node.cc index c0e8dbcd44db21..b37e1305f64ce6 100644 --- a/src/node.cc +++ b/src/node.cc @@ -161,6 +161,10 @@ static const char* icu_data_dir = nullptr; // used by C++ modules as well bool no_deprecation = false; +// true if process warnings should be suppressed +bool no_process_warnings = false; +bool trace_warnings = false; + #if HAVE_OPENSSL && NODE_FIPS_MODE // used by crypto module bool enable_fips_crypto = false; @@ -3027,6 +3031,16 @@ void SetupProcessObject(Environment* env, READONLY_PROPERTY(process, "noDeprecation", True(env->isolate())); } + // --no-warnings + if (no_process_warnings) { + READONLY_PROPERTY(process, "noProcessWarnings", True(env->isolate())); + } + + // --trace-warnings + if (trace_warnings) { + READONLY_PROPERTY(process, "traceProcessWarnings", True(env->isolate())); + } + // --throw-deprecation if (throw_deprecation) { READONLY_PROPERTY(process, "throwDeprecation", True(env->isolate())); @@ -3277,9 +3291,11 @@ static void PrintHelp() { " does not appear to be a terminal\n" " -r, --require module to preload (option can be repeated)\n" " --no-deprecation silence deprecation warnings\n" + " --no-warnings silence all process warnings\n" " --throw-deprecation throw an exception anytime a deprecated " "function is used\n" " --trace-deprecation show stack traces on deprecations\n" + " --trace-warnings show stack traces on process warnings\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 " @@ -3415,6 +3431,10 @@ static void ParseArgs(int* argc, force_repl = true; } else if (strcmp(arg, "--no-deprecation") == 0) { no_deprecation = true; + } else if (strcmp(arg, "--no-warnings") == 0) { + no_process_warnings = true; + } else if (strcmp(arg, "--trace-warnings") == 0) { + trace_warnings = true; } else if (strcmp(arg, "--trace-deprecation") == 0) { trace_deprecation = true; } else if (strcmp(arg, "--trace-sync-io") == 0) { diff --git a/src/node.js b/src/node.js index a0f5aec86db245..32d1d5da413719 100644 --- a/src/node.js +++ b/src/node.js @@ -22,6 +22,42 @@ EventEmitter.call(process); + // Default process warning handler.. + process.on('warning', (warning) => { + // By default, print the short warning message if --no-warnings is not + // set. If --trace-warnings is set, then the stack trace for the warning + // is shown. This takes backwards compatibility with --trace-deprecation + // into consideration also. + if (process.noProcessWarnings) return; + const trace = process.traceProcessWarnings || + (warning.name === 'DeprecationWarning' && + process.traceDeprecation); + if (trace) + console.warn(warning.stack); + else + console.warn(`${warning.name}: ${warning.message}`); + }); + + // process.emitWarning(error[, name]) + // process.emitWarning(str[, name]) + process.emitWarning = function(warning, name) { + if (!(warning instanceof Error) && typeof warning !== 'string') + throw new TypeError(`'warning' must be an Error object or string`); + + if (name && typeof name !== 'string') + throw new TypeError(`'name' must be a string`); + + if (typeof warning === 'string') { + warning = new Error(warning); + } + + warning.name = name || (warning.name === 'Error' + ? 'Warning' + : warning.name); + + process.nextTick(() => process.emit('warning', warning)); + }; + let eeWarned = false; Object.defineProperty(process, 'EventEmitter', { get() { diff --git a/test/fixtures/warnings.js b/test/fixtures/warnings.js new file mode 100644 index 00000000000000..9b22719d660ec3 --- /dev/null +++ b/test/fixtures/warnings.js @@ -0,0 +1,15 @@ +'use strict'; + +const securityWarning = new Error('a security warning'); +securityWarning.name = 'SecurityWarning'; +process.emit('warning', securityWarning); + + +const badPracticeWarning = new Error('a bad practice warning'); +badPracticeWarning.name = 'BadPracticeWarning'; +process.emit('warning', badPracticeWarning); + + +const deprecationWarning = new Error('a deprecation warning'); +deprecationWarning.name = 'DeprecationWarning'; +process.emit('warning', deprecationWarning); diff --git a/test/parallel/test-global-console-exists.js b/test/parallel/test-global-console-exists.js index 1a13ffec29c23b..fce7137f8648dc 100644 --- a/test/parallel/test-global-console-exists.js +++ b/test/parallel/test-global-console-exists.js @@ -5,17 +5,28 @@ 'use strict'; const assert = require('assert'); +const common = require('../common'); const EventEmitter = require('events'); const leak_warning = /EventEmitter memory leak detected\. 2 hello listeners/; var write_calls = 0; -process.stderr.write = function(data) { + +process.on('warning', (warning) => { + // This will be called after the default internal + // process warning handler is called. The default + // process warning writes to the console, which will + // invoke the monkeypatched process.stderr.write + // below. + assert.strictEqual(write_calls, 1); + EventEmitter.defaultMaxListeners = old_default; + // when we get here, we should be done +}); + +process.stderr.write = (data) => { if (write_calls === 0) assert.ok(data.match(leak_warning)); - else if (write_calls === 1) - assert.ok(data.match(/Trace/)); else - assert.ok(false, 'stderr.write should be called only twice'); + common.fail('stderr.write should be called only once'); write_calls++; }; @@ -27,10 +38,6 @@ const e = new EventEmitter(); e.on('hello', function() {}); e.on('hello', function() {}); -// TODO: figure out how to validate console. Currently, +// TODO: Figure out how to validate console. Currently, // there is no obvious way of validating that console // exists here exactly when it should. - -assert.equal(write_calls, 2); - -EventEmitter.defaultMaxListeners = old_default; diff --git a/test/parallel/test-process-emitwarning.js b/test/parallel/test-process-emitwarning.js new file mode 100644 index 00000000000000..2684f84b7ac106 --- /dev/null +++ b/test/parallel/test-process-emitwarning.js @@ -0,0 +1,30 @@ +// Flags: --no-warnings +// The flag suppresses stderr output but the warning event will still emit +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const util = require('util'); + +process.on('warning', common.mustCall((warning) => { + assert(warning); + assert(/^(Warning|CustomWarning)/.test(warning.name)); + assert(warning.message, 'A Warning'); +}, 5)); + +process.emitWarning('A Warning'); +process.emitWarning('A Warning', 'CustomWarning'); + +process.emitWarning(new Error('A Warning')); +process.emitWarning(new Error('A Warning'), 'CustomWarning'); + +function CustomWarning() { + this.name = 'CustomWarning'; + this.message = 'A Warning'; + Error.captureStackTrace(this, CustomWarning); +} +util.inherits(CustomWarning, Error); +process.emitWarning(new CustomWarning()); + +// TypeError is thrown on invalid output +assert.throws(() => process.emitWarning(1), TypeError); diff --git a/test/sequential/test-deprecation-flags.js b/test/sequential/test-deprecation-flags.js index 1b9521001e9eb0..1f729fba6de874 100644 --- a/test/sequential/test-deprecation-flags.js +++ b/test/sequential/test-deprecation-flags.js @@ -1,16 +1,17 @@ 'use strict'; require('../common'); -var assert = require('assert'); -var execFile = require('child_process').execFile; -var depmod = require.resolve('../fixtures/deprecated.js'); -var node = process.execPath; +const assert = require('assert'); +const execFile = require('child_process').execFile; +const depmod = require.resolve('../fixtures/deprecated.js'); +const node = process.execPath; -var depUserland = +const depUserland = require.resolve('../fixtures/deprecated-userland-function.js'); -var normal = [depmod]; -var noDep = ['--no-deprecation', depmod]; -var traceDep = ['--trace-deprecation', depmod]; +const normal = [depmod]; +const noDep = ['--no-deprecation', depmod]; +const traceDep = ['--trace-deprecation', depmod]; +const noWarn = ['--no-warnings', depmod]; // same effect as --no-deprecation execFile(node, normal, function(er, stdout, stderr) { console.error('normal: show deprecation warning'); @@ -28,15 +29,23 @@ execFile(node, noDep, function(er, stdout, stderr) { console.log('silent ok'); }); +execFile(node, noWarn, function(er, stdout, stderr) { + console.error('--no-warnings: silence warnings'); + assert.equal(er, null); + assert.equal(stdout, ''); + assert.equal(stderr, 'DEBUG: This is deprecated\n'); + console.log('silent ok'); +}); + execFile(node, traceDep, function(er, stdout, stderr) { console.error('--trace-deprecation: show stack'); assert.equal(er, null); assert.equal(stdout, ''); var stack = stderr.trim().split('\n'); // just check the top and bottom. - assert.equal(stack[0], - 'Trace: util.debug is deprecated. Use console.error instead.'); - assert.equal(stack.pop(), 'DEBUG: This is deprecated'); + assert.equal(stack[1], 'DeprecationWarning: util.debug is deprecated. ' + + 'Use console.error instead.'); + assert.equal(stack[0], 'DEBUG: This is deprecated'); console.log('trace ok'); }); @@ -44,6 +53,6 @@ execFile(node, [depUserland], function(er, stdout, stderr) { console.error('normal: testing deprecated userland function'); assert.equal(er, null); assert.equal(stdout, ''); - assert.equal(0, stderr.indexOf('deprecatedFunction is deprecated.')); + assert(/deprecatedFunction is deprecated/.test(stderr)); console.error('normal: ok'); }); diff --git a/test/sequential/test-process-warnings.js b/test/sequential/test-process-warnings.js new file mode 100644 index 00000000000000..3193cbcb97b53e --- /dev/null +++ b/test/sequential/test-process-warnings.js @@ -0,0 +1,39 @@ +// Flags: --expose-internals +'use strict'; + +require('../common'); +const assert = require('assert'); +const execFile = require('child_process').execFile; +const warnmod = require.resolve('../fixtures/warnings.js'); +const node = process.execPath; + +const normal = ['--expose-internals', warnmod]; +const noWarn = ['--expose-internals', '--no-warnings', warnmod]; +const traceWarn = ['--expose-internals', '--trace-warnings', warnmod]; + +execFile(node, normal, function(er, stdout, stderr) { + assert.equal(er, null); + assert.equal(stdout, ''); + assert(/SecurityWarning: a security warning/.test(stderr)); + assert(/BadPracticeWarning: a bad practice warning/.test(stderr)); + assert(/DeprecationWarning: a deprecation warning/.test(stderr)); +}); + +execFile(node, noWarn, function(er, stdout, stderr) { + assert.equal(er, null); + assert.equal(stdout, ''); + assert(!/SecurityWarning: a security warning/.test(stderr)); + assert(!/BadPracticeWarning: a bad practice warning/.test(stderr)); + assert(!/DeprecationWarning: a deprecation warning/.test(stderr)); +}); + +execFile(node, traceWarn, function(er, stdout, stderr) { + assert.equal(er, null); + assert.equal(stdout, ''); + assert(/SecurityWarning: a security warning/.test(stderr)); + assert(/BadPracticeWarning: a bad practice warning/.test(stderr)); + assert(/DeprecationWarning: a deprecation warning/.test(stderr)); + assert(/at Object\.\\s\(.+warnings.js:3:25\)/.test(stderr)); + assert(/at Object\.\\s\(.+warnings.js:8:28\)/.test(stderr)); + // DeprecationWarning will only show a trace if --trace-deprecation is on +});