report: remove verbose setting

This commit removes the --diagnostic-report-verbose CLI option
and all associated logic. The flag is currently only used in one
place, and only reflects the settings at startup. Additionally,
Node tends to use the NODE_DEBUG mechanism for adding verbose
output.

PR-URL: https://github.com/nodejs/node/pull/26195
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
This commit is contained in:
cjihrig 2019-02-18 14:01:12 -05:00
parent 11a8a940d7
commit 823c988b48
No known key found for this signature in database
GPG Key ID: 7434390BDBE9B9C5
9 changed files with 3 additions and 124 deletions

View File

@ -126,13 +126,6 @@ Enables report to be generated on un-caught exceptions, if
`--experimental-report` is enabled. Useful when inspecting JavaScript stack in `--experimental-report` is enabled. Useful when inspecting JavaScript stack in
conjunction with native stack and other runtime environment data. conjunction with native stack and other runtime environment data.
### `--diagnostic-report-verbose`
<!-- YAML
added: v11.8.0
-->
Flag that enables additional information to be printed during report generation.
### `--enable-fips` ### `--enable-fips`
<!-- YAML <!-- YAML
added: v6.0.0 added: v6.0.0
@ -688,7 +681,6 @@ Node.js options that are allowed are:
- `--diagnostic-report-on-signal` - `--diagnostic-report-on-signal`
- `--diagnostic-report-signal` - `--diagnostic-report-signal`
- `--diagnostic-report-uncaught-exception` - `--diagnostic-report-uncaught-exception`
- `--diagnostic-report-verbose`
- `--enable-fips` - `--enable-fips`
- `--experimental-modules` - `--experimental-modules`
- `--experimental-repl-await` - `--experimental-repl-await`

View File

@ -1703,8 +1703,6 @@ added: v11.8.0
* `filename` {string} Name of the file where the report is written. * `filename` {string} Name of the file where the report is written.
* `path` {string} Directory where the report is written. * `path` {string} Directory where the report is written.
**Default:** the current working directory of the Node.js process. **Default:** the current working directory of the Node.js process.
* `verbose` {boolean} Flag that controls additional verbose information on
report generation. **Default:** `false`.
Configures the diagnostic reporting behavior. Upon invocation, the runtime Configures the diagnostic reporting behavior. Upon invocation, the runtime
is reconfigured to generate reports based on `options`. Several usage examples is reconfigured to generate reports based on `options`. Several usage examples
@ -1721,9 +1719,6 @@ process.report.setOptions({ filename: 'foo.json', path: '/home' });
// to `stdout` and `stderr`. Usage of these will result in report being written // to `stdout` and `stderr`. Usage of these will result in report being written
// to the associated standard streams. URLs are not supported. // to the associated standard streams. URLs are not supported.
process.report.setOptions({ filename: 'stdout' }); process.report.setOptions({ filename: 'stdout' });
// Enable verbose option on report generation.
process.report.setOptions({ verbose: true });
``` ```
Signal based report generation is not supported on Windows. Signal based report generation is not supported on Windows.

View File

@ -401,9 +401,6 @@ written.
* `--diagnostic-report-signal` Sets or resets the signal for report generation * `--diagnostic-report-signal` Sets or resets the signal for report generation
(not supported on Windows). Default signal is `SIGUSR2`. (not supported on Windows). Default signal is `SIGUSR2`.
* `--diagnostic-report-verbose` Flag that enables additional information to be
printed during report generation.
A report can also be triggered via an API call from a JavaScript application: A report can also be triggered via an API call from a JavaScript application:
```js ```js
@ -495,8 +492,7 @@ process.report.setOptions({
events: ['exception', 'fatalerror', 'signal'], events: ['exception', 'fatalerror', 'signal'],
signal: 'SIGUSR2', signal: 'SIGUSR2',
filename: 'myreport.json', filename: 'myreport.json',
path: '/home/nodeuser', path: '/home/nodeuser'
verbose: true
}); });
``` ```
@ -519,9 +515,6 @@ timestamp, PID and sequence number.
URLs are not supported. Defaults to the current working directory of the URLs are not supported. Defaults to the current working directory of the
Node.js process. Node.js process.
`verbose` specifies whether to print additional verbose messages
pertinent to the report generation. Defaults to `false`.
```js ```js
// Trigger report only on uncaught exceptions. // Trigger report only on uncaught exceptions.
process.report.setOptions({ events: ['exception'] }); process.report.setOptions({ events: ['exception'] });
@ -541,7 +534,7 @@ environment variables:
NODE_OPTIONS="--experimental-report --diagnostic-report-uncaught-exception \ NODE_OPTIONS="--experimental-report --diagnostic-report-uncaught-exception \
--diagnostic-report-on-fatalerror --diagnostic-report-on-signal \ --diagnostic-report-on-fatalerror --diagnostic-report-on-signal \
--diagnostic-report-signal=SIGUSR2 --diagnostic-report-filename=./report.json \ --diagnostic-report-signal=SIGUSR2 --diagnostic-report-filename=./report.json \
--diagnostic-report-directory=/home/nodeuser --diagnostic-report-verbose" --diagnostic-report-directory=/home/nodeuser"
``` ```
Specific API documentation can be found under Specific API documentation can be found under

View File

@ -114,11 +114,6 @@ to be generated on un-caught exceptions, if
.Sy --experimental-report .Sy --experimental-report
is enabled. Useful when inspecting JavaScript stack in conjunction with native stack and other runtime environment data. is enabled. Useful when inspecting JavaScript stack in conjunction with native stack and other runtime environment data.
. .
.It Fl -diagnostic-report-verbose
Flag that enables additional information to be printed during
.Sy diagnostic report
generation.
.
.It Fl -enable-fips .It Fl -enable-fips
Enable FIPS-compliant crypto at startup. Enable FIPS-compliant crypto at startup.
Requires Node.js to be built with Requires Node.js to be built with

View File

@ -25,8 +25,7 @@ let config = {
events: [], events: [],
signal: 'SIGUSR2', signal: 'SIGUSR2',
filename: '', filename: '',
path: '', path: ''
verbose: false
}; };
const report = { const report = {
setOptions(options) { setOptions(options) {
@ -58,13 +57,6 @@ const report = {
else else
throw new ERR_INVALID_ARG_TYPE('path', 'string', options.path); throw new ERR_INVALID_ARG_TYPE('path', 'string', options.path);
if (typeof options.verbose === 'boolean')
newConfig.verbose = options.verbose;
else if (options.verbose === undefined)
newConfig.verbose = false;
else
throw new ERR_INVALID_ARG_TYPE('verbose', 'boolean', options.verbose);
if (typeof options.signal === 'string') if (typeof options.signal === 'string')
newConfig.signal = convertToValidSignal(options.signal); newConfig.signal = convertToValidSignal(options.signal);
else if (options.signal === undefined) else if (options.signal === undefined)

View File

@ -90,11 +90,6 @@ void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
"--diagnostic-report-uncaught-exception option is valid only when " "--diagnostic-report-uncaught-exception option is valid only when "
"--experimental-report is set"); "--experimental-report is set");
} }
if (report_verbose) {
errors->push_back("--diagnostic-report-verbose option is valid only when "
"--experimental-report is set");
}
#endif // NODE_REPORT #endif // NODE_REPORT
} }
@ -357,11 +352,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() {
" (default: current working directory of Node.js process)", " (default: current working directory of Node.js process)",
&PerIsolateOptions::report_directory, &PerIsolateOptions::report_directory,
kAllowedInEnvironment); kAllowedInEnvironment);
AddOption("--diagnostic-report-verbose",
"verbose option for report generation(true|false)."
" (default: false)",
&PerIsolateOptions::report_verbose,
kAllowedInEnvironment);
#endif // NODE_REPORT #endif // NODE_REPORT
Insert(&EnvironmentOptionsParser::instance, Insert(&EnvironmentOptionsParser::instance,

View File

@ -143,7 +143,6 @@ class PerIsolateOptions : public Options {
std::string report_signal; std::string report_signal;
std::string report_filename; std::string report_filename;
std::string report_directory; std::string report_directory;
bool report_verbose;
#endif // NODE_REPORT #endif // NODE_REPORT
inline EnvironmentOptions* get_per_env_options(); inline EnvironmentOptions* get_per_env_options();
void CheckOptions(std::vector<std::string>* errors) override; void CheckOptions(std::vector<std::string>* errors) override;

View File

@ -167,17 +167,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {
Utf8Value pathstr(env->isolate(), path); Utf8Value pathstr(env->isolate(), path);
// Report verbosity
Local<String> verbosekey = FIXED_ONE_BYTE_STRING(env->isolate(), "verbose");
Local<Value> verbose_unchecked;
if (!obj->Get(context, verbosekey).ToLocal(&verbose_unchecked)) return;
Local<Boolean> verbose;
if (verbose_unchecked->IsUndefined() || verbose_unchecked->IsNull())
verbose_unchecked = Boolean::New(env->isolate(), "verbose");
verbose = verbose_unchecked.As<Boolean>();
bool verb = verbose->BooleanValue(env->isolate());
if (sync) { if (sync) {
static const std::string e = "exception"; static const std::string e = "exception";
static const std::string s = "signal"; static const std::string s = "signal";
@ -202,7 +191,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {
options->report_filename = *filestr; options->report_filename = *filestr;
CHECK_NOT_NULL(*pathstr); CHECK_NOT_NULL(*pathstr);
options->report_directory = *pathstr; options->report_directory = *pathstr;
options->report_verbose = verb;
} else { } else {
int i = 0; int i = 0;
if (options->report_uncaught_exception && if (options->report_uncaught_exception &&
@ -242,12 +230,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {
.ToLocal(&path_value)) .ToLocal(&path_value))
return; return;
if (!obj->Set(context, pathkey, path_value).FromJust()) return; if (!obj->Set(context, pathkey, path_value).FromJust()) return;
if (!obj->Set(context,
verbosekey,
Boolean::New(env->isolate(), options->report_verbose))
.FromJust())
return;
} }
} }
@ -261,22 +243,6 @@ static void Initialize(Local<Object> exports,
env->SetMethod(exports, "onUnCaughtException", OnUncaughtException); env->SetMethod(exports, "onUnCaughtException", OnUncaughtException);
env->SetMethod(exports, "onUserSignal", OnUserSignal); env->SetMethod(exports, "onUserSignal", OnUserSignal);
env->SetMethod(exports, "syncConfig", SyncConfig); env->SetMethod(exports, "syncConfig", SyncConfig);
// TODO(gireeshpunathil) if we are retaining this flag,
// insert more verbose information at vital control flow
// points. Right now, it is only this one.
if (options->report_verbose) {
std::cerr << "report: initialization complete, event flags:" << std::endl;
std::cerr << "report_uncaught_exception: "
<< options->report_uncaught_exception << std::endl;
std::cerr << "report_on_signal: " << options->report_on_signal << std::endl;
std::cerr << "report_on_fatalerror: " << options->report_on_fatalerror
<< std::endl;
std::cerr << "report_signal: " << options->report_signal << std::endl;
std::cerr << "report_filename: " << options->report_filename << std::endl;
std::cerr << "report_directory: " << options->report_directory << std::endl;
std::cerr << "report_verbose: " << options->report_verbose << std::endl;
}
} }
} // namespace report } // namespace report

View File

@ -1,43 +0,0 @@
'use strict';
// Tests --diagnostic-report-verbose option.
const common = require('../common');
common.skipIfReportDisabled();
if (process.argv[2] === 'child') {
// no-op
} else {
const helper = require('../common/report.js');
const spawn = require('child_process').spawn;
const assert = require('assert');
const tmpdir = require('../common/tmpdir');
tmpdir.refresh();
const expected = [ 'report: initialization complete, event flags:',
'report_uncaught_exception: 0',
'report_on_signal: 0',
'report_on_fatalerror: 0',
'report_signal:',
'report_filename:',
'report_directory:',
'report_verbose: 1' ];
const child = spawn(process.execPath,
['--experimental-report',
'--diagnostic-report-verbose',
__filename,
'child',
],
{ cwd: tmpdir.path });
let stderr;
child.stderr.on('data', (data) => stderr += data);
child.on('exit', common.mustCall((code) => {
const process_msg = 'Process exited unexpectedly';
assert.strictEqual(code, 0, process_msg + ':' + code);
const reports = helper.findReports(child.pid, tmpdir.path);
assert.strictEqual(reports.length, 0,
`Found unexpected report ${reports[0]}`);
for (const line of expected) {
assert.ok(stderr.includes(line), `'${line}' not found in '${stderr}'`);
}
}));
}