Skip to content

Commit

Permalink
doc,lib,src,test: make --experimental-report a nop
Browse files Browse the repository at this point in the history
This commit makes the --experimental-report CLI flag a no-op.

PR-URL: nodejs#32242
Fixes: nodejs#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>
  • Loading branch information
cjihrig committed Mar 15, 2020
1 parent 4c64e7c commit 9d1a3b6
Show file tree
Hide file tree
Showing 17 changed files with 35 additions and 134 deletions.
24 changes: 8 additions & 16 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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: v13.4.0
Expand Down Expand Up @@ -635,9 +628,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
Expand All @@ -650,8 +643,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
Expand All @@ -676,9 +669,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
Expand Down Expand Up @@ -1098,7 +1091,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`
Expand Down
11 changes: 3 additions & 8 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -392,14 +391,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.
Expand Down Expand Up @@ -569,7 +564,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"
Expand Down
22 changes: 8 additions & 14 deletions doc/node.1
Original file line number Diff line number Diff line change
Expand Up @@ -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.
.
Expand Down Expand Up @@ -299,16 +294,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
Expand All @@ -318,9 +313,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.
Expand Down
9 changes: 0 additions & 9 deletions lib/internal/bootstrap/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
Expand All @@ -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();
Expand Down
46 changes: 1 addition & 45 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -360,10 +319,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,
Expand Down
3 changes: 1 addition & 2 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,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;
Expand Down Expand Up @@ -188,7 +187,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();
Expand Down
1 change: 0 additions & 1 deletion test/addons/worker-addon/test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-report
'use strict';
const common = require('../../common');
const assert = require('assert');
Expand Down
2 changes: 2 additions & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -64,6 +65,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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
Expand Down
6 changes: 1 addition & 5 deletions test/report/test-report-config.js
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
3 changes: 1 addition & 2 deletions test/report/test-report-fatal-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'];
Expand Down
7 changes: 1 addition & 6 deletions test/report/test-report-getreport.js
Original file line number Diff line number Diff line change
@@ -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());
Expand Down
5 changes: 1 addition & 4 deletions test/report/test-report-signal.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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;

Expand Down
5 changes: 1 addition & 4 deletions test/report/test-report-uncaught-exception.js
Original file line number Diff line number Diff line change
@@ -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');
Expand All @@ -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;

Expand Down
8 changes: 2 additions & 6 deletions test/report/test-report-uv-handles.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion test/report/test-report-worker.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
// Flags: --experimental-report
'use strict';
const common = require('../common');
const assert = require('assert');
Expand Down
Loading

0 comments on commit 9d1a3b6

Please sign in to comment.