Skip to content

Commit

Permalink
report: remove verbose setting
Browse files Browse the repository at this point in the history
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: #26195
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
  • Loading branch information
cjihrig authored and addaleax committed Feb 21, 2019
1 parent bbbe77f commit fcf1b13
Show file tree
Hide file tree
Showing 9 changed files with 3 additions and 124 deletions.
8 changes: 0 additions & 8 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,13 +126,6 @@ Enables report to be generated on un-caught exceptions, if
`--experimental-report` is enabled. Useful when inspecting JavaScript stack in
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`
<!-- YAML
added: v6.0.0
Expand Down Expand Up @@ -672,7 +665,6 @@ Node.js options that are allowed are:
- `--diagnostic-report-on-signal`
- `--diagnostic-report-signal`
- `--diagnostic-report-uncaught-exception`
- `--diagnostic-report-verbose`
- `--enable-fips`
- `--experimental-modules`
- `--experimental-repl-await`
Expand Down
5 changes: 0 additions & 5 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1703,8 +1703,6 @@ added: v11.8.0
* `filename` {string} Name of the file where the report is written.
* `path` {string} Directory where the report is written.
**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
is reconfigured to generate reports based on `options`. Several usage examples
Expand All @@ -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 the associated standard streams. URLs are not supported.
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.
Expand Down
11 changes: 2 additions & 9 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -401,9 +401,6 @@ written.
* `--diagnostic-report-signal` Sets or resets the signal for report generation
(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:

```js
Expand Down Expand Up @@ -495,8 +492,7 @@ process.report.setOptions({
events: ['exception', 'fatalerror', 'signal'],
signal: 'SIGUSR2',
filename: 'myreport.json',
path: '/home/nodeuser',
verbose: true
path: '/home/nodeuser'
});
```

Expand All @@ -519,9 +515,6 @@ timestamp, PID and sequence number.
URLs are not supported. Defaults to the current working directory of the
Node.js process.

`verbose` specifies whether to print additional verbose messages
pertinent to the report generation. Defaults to `false`.

```js
// Trigger report only on uncaught exceptions.
process.report.setOptions({ events: ['exception'] });
Expand All @@ -541,7 +534,7 @@ environment variables:
NODE_OPTIONS="--experimental-report --diagnostic-report-uncaught-exception \
--diagnostic-report-on-fatalerror --diagnostic-report-on-signal \
--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
Expand Down
5 changes: 0 additions & 5 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,6 @@ to be generated on un-caught exceptions, if
.Sy --experimental-report
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
Enable FIPS-compliant crypto at startup.
Requires Node.js to be built with
Expand Down
10 changes: 1 addition & 9 deletions lib/internal/process/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ let config = {
events: [],
signal: 'SIGUSR2',
filename: '',
path: '',
verbose: false
path: ''
};
const report = {
setOptions(options) {
Expand Down Expand Up @@ -58,13 +57,6 @@ const report = {
else
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')
newConfig.signal = convertToValidSignal(options.signal);
else if (options.signal === undefined)
Expand Down
10 changes: 0 additions & 10 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,6 @@ void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
"--diagnostic-report-uncaught-exception option is valid only when "
"--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
}

Expand Down Expand Up @@ -339,11 +334,6 @@ PerIsolateOptionsParser::PerIsolateOptionsParser() {
" (default: current working directory of Node.js process)",
&PerIsolateOptions::report_directory,
kAllowedInEnvironment);
AddOption("--diagnostic-report-verbose",
"verbose option for report generation(true|false)."
" (default: false)",
&PerIsolateOptions::report_verbose,
kAllowedInEnvironment);
#endif // NODE_REPORT

Insert(&EnvironmentOptionsParser::instance,
Expand Down
1 change: 0 additions & 1 deletion src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ class PerIsolateOptions : public Options {
std::string report_signal;
std::string report_filename;
std::string report_directory;
bool report_verbose;
#endif // NODE_REPORT
inline EnvironmentOptions* get_per_env_options();
void CheckOptions(std::vector<std::string>* errors) override;
Expand Down
34 changes: 0 additions & 34 deletions src/node_report_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,17 +167,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {

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(context).FromJust();

if (sync) {
static const std::string e = "exception";
static const std::string s = "signal";
Expand All @@ -202,7 +191,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {
options->report_filename = *filestr;
CHECK_NOT_NULL(*pathstr);
options->report_directory = *pathstr;
options->report_verbose = verb;
} else {
int i = 0;
if (options->report_uncaught_exception &&
Expand Down Expand Up @@ -242,12 +230,6 @@ void SyncConfig(const FunctionCallbackInfo<Value>& info) {
.ToLocal(&path_value))
return;
if (!obj->Set(context, pathkey, path_value).FromJust()) return;

if (!obj->Set(context,
verbosekey,
Boolean::New(env->isolate(), options->report_verbose))
.FromJust())
return;
}
}

Expand All @@ -261,22 +243,6 @@ static void Initialize(Local<Object> exports,
env->SetMethod(exports, "onUnCaughtException", OnUncaughtException);
env->SetMethod(exports, "onUserSignal", OnUserSignal);
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
Expand Down
43 changes: 0 additions & 43 deletions test/node-report/test-diagnostic-report-verbose.js

This file was deleted.

0 comments on commit fcf1b13

Please sign in to comment.