From 823c988b482e609978869820758b7182f32e9de5 Mon Sep 17 00:00:00 2001
From: cjihrig <cjihrig@gmail.com>
Date: Mon, 18 Feb 2019 14:01:12 -0500
Subject: [PATCH] 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>
---
 doc/api/cli.md                                |  8 ----
 doc/api/process.md                            |  5 ---
 doc/api/report.md                             | 11 +----
 doc/node.1                                    |  5 ---
 lib/internal/process/report.js                | 10 +----
 src/node_options.cc                           | 10 -----
 src/node_options.h                            |  1 -
 src/node_report_module.cc                     | 34 ---------------
 .../test-diagnostic-report-verbose.js         | 43 -------------------
 9 files changed, 3 insertions(+), 124 deletions(-)
 delete mode 100644 test/node-report/test-diagnostic-report-verbose.js

diff --git a/doc/api/cli.md b/doc/api/cli.md
index 5e817eaa1da518..a4866e94d51e9c 100644
--- 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
@@ -688,7 +681,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
index b87223b9c41289..1a009bce546473 100644
--- 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
index 8d6d054c0a10b9..419fe33417345d 100644
--- 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
index 7d27bf59efe115..266a5a3f9d2f10 100644
--- 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
index 95972773b18ecf..87fad4c1958e5a 100644
--- 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
index e68487a2bf9a99..937298e0d833df 100644
--- a/src/node_options.cc
+++ b/src/node_options.cc
@@ -90,11 +90,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
 }
 
@@ -357,11 +352,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
index e9481e2de5da9e..fe2e1034c937aa 100644
--- a/src/node_options.h
+++ b/src/node_options.h
@@ -143,7 +143,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
index d13eb61c83820a..f56da94bac196d 100644
--- 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(env->isolate());
-
   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
deleted file mode 100644
index f59b07d62a9f07..00000000000000
--- a/test/node-report/test-diagnostic-report-verbose.js
+++ /dev/null
@@ -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}'`);
-    }
-  }));
-}