doc,lib,src,test: make --experimental-report a nop

cjihrig · targos · commit 90d35adccd76 · 2020-04-28T16:14:48.000+02:00
This commit makes the --experimental-report CLI flag a no-op. PR-URL: #32242 Fixes: #26293 Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
diff --git a/doc/api/cli.md b/doc/api/cli.md
@@ -199,13 +199,6 @@ added: v10.0.0
 
 Enable experimental top-level `await` keyword support in REPL.
 
-### `--experimental-report`
-<!-- YAML
-added: v11.8.0
--->
-
-Enable experimental diagnostic report feature.
-
 ### `--experimental-specifier-resolution=mode`
 <!-- YAML
 added: v12.16.0
@@ -658,9 +651,9 @@ changes:
 
 Enables the report to be triggered on fatal errors (internal errors within
 the Node.js runtime such as out of memory) that lead to termination of the
-application, if `--experimental-report` is enabled. Useful to inspect various
-diagnostic data elements such as heap, stack, event loop state, resource
-consumption etc. to reason about the fatal error.
+application. Useful to inspect various diagnostic data elements such as heap,
+stack, event loop state, resource consumption etc. to reason about the fatal
+error.
 
 ### `--report-on-signal`
 <!-- YAML
@@ -673,8 +666,8 @@ changes:
 -->
 
 Enables report to be generated upon receiving the specified (or predefined)
-signal to the running Node.js process, if `--experimental-report` is enabled.
-The signal to trigger the report is specified through `--report-signal`.
+signal to the running Node.js process. The signal to trigger the report is
+specified through `--report-signal`.
 
 ### `--report-signal=signal`
 <!-- YAML
@@ -699,9 +692,9 @@ changes:
                  `--report-uncaught-exception`
 -->
 
-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.
+Enables report to be generated on uncaught exceptions. Useful when inspecting
+the JavaScript stack in conjunction with native stack and other runtime
+environment data.
 
 ### `--throw-deprecation`
 <!-- YAML
@@ -1121,7 +1114,6 @@ Node.js options that are allowed are:
 * `--experimental-modules`
 * `--experimental-policy`
 * `--experimental-repl-await`
-* `--experimental-report`
 * `--experimental-specifier-resolution`
 * `--experimental-vm-modules`
 * `--experimental-wasi-unstable-preview1`
diff --git a/doc/api/report.md b/doc/api/report.md
@@ -32,7 +32,6 @@ is provided below for reference.
     "cwd": "/home/nodeuser/project/node",
     "commandLine": [
       "/home/nodeuser/project/node/out/Release/node",
-      "--experimental-report",
       "--report-uncaught-exception",
       "/home/nodeuser/project/node/test/report/test-exception.js",
       "child"
@@ -393,14 +392,10 @@ is provided below for reference.
 ## Usage
 
 ```bash
-node --experimental-report --report-uncaught-exception \
-  --report-on-signal --report-on-fatalerror app.js
+node --report-uncaught-exception --report-on-signal \
+--report-on-fatalerror app.js
 ```
 
-* `--experimental-report` Enables the diagnostic report feature.
- In the absence of this flag, use of all other related options will result in
- an error.
-
 * `--report-uncaught-exception` Enables report to be generated on
 un-caught exceptions. Useful when inspecting JavaScript stack in conjunction
 with native stack and other runtime environment data.
@@ -570,7 +565,7 @@ Configuration on module initialization is also available via
 environment variables:
 
 ```bash
-NODE_OPTIONS="--experimental-report --report-uncaught-exception \
+NODE_OPTIONS="--report-uncaught-exception \
   --report-on-fatalerror --report-on-signal \
   --report-signal=SIGUSR2  --report-filename=./report.json \
   --report-directory=/home/nodeuser"
diff --git a/doc/node.1 b/doc/node.1
@@ -138,11 +138,6 @@ keyword support in REPL.
 .It Fl -experimental-specifier-resolution
 Select extension resolution algorithm for ES Modules; either 'explicit' (default) or 'node'
 .
-.It Fl -experimental-report
-Enable experimental
-.Sy diagnostic report
-feature.
-.
 .It Fl -experimental-vm-modules
 Enable experimental ES module support in VM module.
 .
@@ -308,16 +303,16 @@ will be written.
 .It Fl -report-on-fatalerror
 Enables the
 .Sy diagnostic report
-to be triggered on fatal errors (internal errors within the Node.js runtime such as out of memory) that leads to termination of the application, if
-.Sy --experimental-report
-is enabled. Useful to inspect various diagnostic data elements such as heap, stack, event loop state, resource consumption etc. to reason about the fatal error.
+to be triggered on fatal errors (internal errors within the Node.js runtime such
+as out of memory) that leads to termination of the application. Useful to
+inspect various diagnostic data elements such as heap, stack, event loop state,
+resource consumption etc. to reason about the fatal error.
 .
 .It Fl -report-on-signal
 Enables
 .Sy diagnostic report
-to be generated upon receiving the specified (or predefined) signal to the running Node.js process, if
-.Sy --experimental-report
-is enabled. Default signal is SIGUSR2.
+to be generated upon receiving the specified (or predefined) signal to the
+running Node.js process. Default signal is SIGUSR2.
 .
 .It Fl -report-signal
 Sets or resets the signal for
@@ -327,9 +322,8 @@ generation (not supported on Windows). Default signal is SIGUSR2.
 .It Fl -report-uncaught-exception
 Enables
 .Sy diagnostic report
-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.
+to be generated on un-caught exceptions. Useful when inspecting JavaScript
+stack in conjunction with native stack and other runtime environment data.
 .
 .It Fl -throw-deprecation
 Throw errors for deprecations.
diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -163,16 +163,11 @@ function setupStacktracePrinterOnSigint() {
 }
 
 function initializeReport() {
-  if (!getOptionValue('--experimental-report')) {
-    return;
-  }
   const { report } = require('internal/process/report');
-  const { emitExperimentalWarning } = require('internal/util');
   ObjectDefineProperty(process, 'report', {
     enumerable: false,
     configurable: true,
     get() {
-      emitExperimentalWarning('report');
       return report;
     }
   });
@@ -187,10 +182,6 @@ function setupDebugEnv() {
 
 // This has to be called after initializeReport() is called
 function initializeReportSignalHandlers() {
-  if (!getOptionValue('--experimental-report')) {
-    return;
-  }
-
   const { addSignalHandler } = require('internal/process/report');
 
   addSignalHandler();
diff --git a/src/node_options.cc b/src/node_options.cc
@@ -76,47 +76,6 @@ void PerProcessOptions::CheckOptions(std::vector<std::string>* errors) {
 
 void PerIsolateOptions::CheckOptions(std::vector<std::string>* errors) {
   per_env->CheckOptions(errors);
-
-  if (per_env->experimental_report) {
-    // Assign the report_signal default value here. Once the
-    // --experimental-report flag is dropped, move this initialization to
-    // node_options.h, where report_signal is declared.
-    if (report_signal.empty())
-      report_signal = "SIGUSR2";
-    return;
-  }
-
-  if (!report_directory.empty()) {
-    errors->push_back("--report-directory option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (!report_filename.empty()) {
-    errors->push_back("--report-filename option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (!report_signal.empty()) {
-    errors->push_back("--report-signal option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (report_on_fatalerror) {
-    errors->push_back(
-        "--report-on-fatalerror option is valid only when "
-        "--experimental-report is set");
-  }
-
-  if (report_on_signal) {
-    errors->push_back("--report-on-signal option is valid only when "
-                      "--experimental-report is set");
-  }
-
-  if (report_uncaught_exception) {
-    errors->push_back(
-        "--report-uncaught-exception option is valid only when "
-        "--experimental-report is set");
-  }
 }
 
 void EnvironmentOptions::CheckOptions(std::vector<std::string>* errors) {
@@ -395,10 +354,7 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
             &EnvironmentOptions::experimental_vm_modules,
             kAllowedInEnvironment);
   AddOption("--experimental-worker", "", NoOp{}, kAllowedInEnvironment);
-  AddOption("--experimental-report",
-            "enable report generation",
-            &EnvironmentOptions::experimental_report,
-            kAllowedInEnvironment);
+  AddOption("--experimental-report", "", NoOp{}, kAllowedInEnvironment);
   AddOption("--experimental-wasi-unstable-preview1",
             "experimental WASI support",
             &EnvironmentOptions::experimental_wasi,
diff --git a/src/node_options.h b/src/node_options.h
@@ -152,7 +152,6 @@ class EnvironmentOptions : public Options {
 
   bool syntax_check_only = false;
   bool has_eval_string = false;
-  bool experimental_report = false;
   bool experimental_wasi = false;
   std::string eval_string;
   bool print_eval = false;
@@ -189,7 +188,7 @@ class PerIsolateOptions : public Options {
   bool report_uncaught_exception = false;
   bool report_on_signal = false;
   bool report_on_fatalerror = false;
-  std::string report_signal;
+  std::string report_signal = "SIGUSR2";
   std::string report_filename;
   std::string report_directory;
   inline EnvironmentOptions* get_per_env_options();
diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-report
 'use strict';
 const common = require('../../common');
 const assert = require('assert');
diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js
@@ -22,6 +22,7 @@ const expectedModules = new Set([
   'Internal Binding native_module',
   'Internal Binding options',
   'Internal Binding process_methods',
+  'Internal Binding report',
   'Internal Binding string_decoder',
   'Internal Binding task_queue',
   'Internal Binding timers',
@@ -54,6 +55,7 @@ const expectedModules = new Set([
   'NativeModule internal/process/execution',
   'NativeModule internal/process/per_thread',
   'NativeModule internal/process/promises',
+  'NativeModule internal/process/report',
   'NativeModule internal/process/signal',
   'NativeModule internal/process/task_queues',
   'NativeModule internal/process/warning',
diff --git a/test/parallel/test-process-env-allowed-flags-are-documented.js b/test/parallel/test-process-env-allowed-flags-are-documented.js
@@ -84,6 +84,7 @@ const undocumented = difference(process.allowedNodeEnvironmentFlags,
 // Remove intentionally undocumented options.
 assert(undocumented.delete('--debug-arraybuffer-allocations'));
 assert(undocumented.delete('--es-module-specifier-resolution'));
+assert(undocumented.delete('--experimental-report'));
 assert(undocumented.delete('--experimental-worker'));
 assert(undocumented.delete('--no-node-snapshot'));
 assert(undocumented.delete('--loader'));
diff --git a/test/report/test-report-config.js b/test/report/test-report-config.js
@@ -1,12 +1,8 @@
-// Flags: --experimental-report --report-on-fatalerror --report-on-signal --report-uncaught-exception
+// Flags: --report-on-fatalerror --report-on-signal --report-uncaught-exception
 'use strict';
 const common = require('../common');
 const assert = require('assert');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
-
 // Verify that process.report.directory behaves properly.
 assert.strictEqual(process.report.directory, '');
 process.report.directory = __dirname;
diff --git a/test/report/test-report-fatal-error.js b/test/report/test-report-fatal-error.js
@@ -21,8 +21,7 @@ if (process.argv[2] === 'child') {
   const tmpdir = require('../common/tmpdir');
   tmpdir.refresh();
   const spawn = require('child_process').spawn;
-  const args = ['--experimental-report',
-                '--report-on-fatalerror',
+  const args = ['--report-on-fatalerror',
                 '--max-old-space-size=20',
                 __filename,
                 'child'];
diff --git a/test/report/test-report-getreport.js b/test/report/test-report-getreport.js
@@ -1,13 +1,8 @@
-// Flags: --experimental-report
 'use strict';
-const common = require('../common');
+require('../common');
 const assert = require('assert');
 const helper = require('../common/report');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
-
 {
   // Test with no arguments.
   helper.validateContent(process.report.getReport());
diff --git a/test/report/test-report-signal.js b/test/report/test-report-signal.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-report --report-on-signal
+// Flags: --report-on-signal
 'use strict';
 // Test producing a report via signal.
 const common = require('../common');
@@ -13,9 +13,6 @@ const assert = require('assert');
 const helper = require('../common/report');
 const tmpdir = require('../common/tmpdir');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
 tmpdir.refresh();
 process.report.directory = tmpdir.path;
 
diff --git a/test/report/test-report-uncaught-exception.js b/test/report/test-report-uncaught-exception.js
@@ -1,4 +1,4 @@
-// Flags: --experimental-report --report-uncaught-exception
+// Flags: --report-uncaught-exception
 'use strict';
 // Test producing a report on uncaught exception.
 const common = require('../common');
@@ -7,9 +7,6 @@ const helper = require('../common/report');
 const tmpdir = require('../common/tmpdir');
 const error = new Error('test error');
 
-common.expectWarning('ExperimentalWarning',
-                     'report is an experimental feature. This feature could ' +
-                     'change at any time');
 tmpdir.refresh();
 process.report.directory = tmpdir.path;
 
diff --git a/test/report/test-report-uv-handles.js b/test/report/test-report-uv-handles.js
@@ -77,24 +77,20 @@ if (process.argv[2] === 'child') {
   const tmpdir = require('../common/tmpdir');
   tmpdir.refresh();
   const options = { encoding: 'utf8', silent: true, cwd: tmpdir.path };
-  const child = fork('--experimental-report', [__filename, 'child'], options);
+  const child = fork(__filename, ['child'], options);
   let child_data;
   child.on('message', (data) => { child_data = data; });
   let stderr = '';
   child.stderr.on('data', (chunk) => { stderr += chunk; });
   let stdout = '';
-  const std_msg = 'Found messages in stderr unexpectedly: ';
   const report_msg = 'Report files were written: unexpectedly';
   child.stdout.on('data', (chunk) => { stdout += chunk; });
   child.on('exit', common.mustCall((code, signal) => {
     assert.deepStrictEqual(code, 0, 'Process exited unexpectedly with code: ' +
                            `${code}`);
     assert.deepStrictEqual(signal, null, 'Process should have exited cleanly,' +
                             ` but did not: ${signal}`);
-    assert.ok(stderr.match(
-      '(node:.*) ExperimentalWarning: report is an experimental' +
-      ' feature. This feature could change at any time'),
-              std_msg);
+    assert.strictEqual(stderr.trim(), '');
 
     const reports = helper.findReports(child.pid, tmpdir.path);
     assert.deepStrictEqual(reports, [], report_msg, reports);
diff --git a/test/report/test-report-worker.js b/test/report/test-report-worker.js
@@ -1,4 +1,3 @@
-// Flags: --experimental-report
 'use strict';
 const common = require('../common');
 const assert = require('assert');
diff --git a/test/report/test-report-writereport.js b/test/report/test-report-writereport.js

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,3 @@`
`1`		`-// Flags: --experimental-report`
`2`	`1`	`'use strict';`
`3`	`2`	`const common = require('../../common');`
`4`	`3`	`const assert = require('assert');`