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: #26195 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]>
nodejs · Feb 28, 2019 · 8a40468 · 8a40468
1 parent 0e89d7a
commit 8a40468
Show file tree

Hide file tree

Showing 9 changed files with 3 additions and 124 deletions.
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -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
@@ -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`

diff --git a/doc/api/process.md b/doc/api/process.md
@@ -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
@@ -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.

diff --git a/doc/api/report.md b/doc/api/report.md
@@ -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
@@ -495,8 +492,7 @@ process.report.setOptions({
   events: ['exception', 'fatalerror', 'signal'],
   signal: 'SIGUSR2',
   filename: 'myreport.json',
-  path: '/home/nodeuser',
-  verbose: true
+  path: '/home/nodeuser'
 });
 ```
 
@@ -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'] });
@@ -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

diff --git a/doc/node.1 b/doc/node.1
@@ -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

diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js
@@ -25,8 +25,7 @@ let config = {
   events: [],
   signal: 'SIGUSR2',
   filename: '',
-  path: '',
-  verbose: false
+  path: ''
 };
 const report = {
   setOptions(options) {
@@ -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)

diff --git a/src/node_options.cc b/src/node_options.cc
@@ -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
 }
 
@@ -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,

diff --git a/src/node_options.h b/src/node_options.h
@@ -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;

diff --git a/src/node_report_module.cc b/src/node_report_module.cc
@@ -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";
@@ -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 &&
@@ -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;
   }
 }
 
@@ -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

diff --git a/test/node-report/test-diagnostic-report-verbose.js b/test/node-report/test-diagnostic-report-verbose.js