report: refactor configuration management

This commit removes process.report.setOptions(). Instead of using complex configuration synchronization between C++ and JS, this commit introduces individual getters and setters. PR-URL: #26414 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Wyatt Preul <[email protected]>
nodejs · Mar 5, 2019 · 30ee277 · 30ee277
1 parent c78788a
commit 30ee277
Show file tree

Hide file tree

Showing 11 changed files with 390 additions and 277 deletions.
diff --git a/doc/api/process.md b/doc/api/process.md
@@ -1669,6 +1669,21 @@ added: v11.8.0
 reports for the current process. Additional documentation is available in the
 [report documentation][].
 
+## process.report.directory
+<!-- YAML
+added: REPLACEME
+-->
+
+* {string}
+
+Directory where the report is written. The default value is the empty string,
+indicating that reports are written to the current working directory of the
+Node.js process.
+
+```js
+console.log(`Report directory is ${process.report.directory}`);
+```
+
 ### process.report.getReport([err])
 <!-- YAML
 added: v11.8.0
@@ -1687,43 +1702,75 @@ console.log(data);
 
 Additional documentation is available in the [report documentation][].
 
-### process.report.setOptions([options]);
+## process.report.filename
 <!-- YAML
-added: v11.8.0
+added: REPLACEME
 -->
 
-* `options` {Object}
-  * `events` {string[]}
-    * `signal`: Generate a report in response to a signal raised on the process.
-    * `exception`: Generate a report on unhandled exceptions.
-    * `fatalerror`: Generate a report on internal fault
-      (such as out of memory errors or native assertions).
-  * `signal` {string} Sets or resets the signal for report generation
-    (not supported on Windows). **Default:** `'SIGUSR2'`.
-  * `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.
+* {string}
 
-Configures the diagnostic reporting behavior. Upon invocation, the runtime
-is reconfigured to generate reports based on `options`. Several usage examples
-are shown below.
+Filename where the report is written. If set to the empty string, the output
+filename will be comprised of a timestamp, PID, and sequence number. The default
+value is the empty string.
 
 ```js
-// Trigger a report on uncaught exceptions or fatal errors.
-process.report.setOptions({ events: ['exception', 'fatalerror'] });
+console.log(`Report filename is ${process.report.filename}`);
+```
+
+## process.report.reportOnFatalError
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean}
 
-// Change the default path and filename of the report.
-process.report.setOptions({ filename: 'foo.json', path: '/home' });
+If `true`, a diagnostic report is generated on fatal errors, such as out of
+memory errors or failed C++ assertions.
 
-// Produce the report onto stdout, when generated. Special meaning is attached
-// 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' });
+```js
+console.log(`Report on fatal error: ${process.report.reportOnFatalError}`);
 ```
 
-Signal based report generation is not supported on Windows.
+## process.report.reportOnSignal
+<!-- YAML
+added: REPLACEME
+-->
 
-Additional documentation is available in the [report documentation][].
+* {boolean}
+
+If `true`, a diagnostic report is generated when the process receives the
+signal specified by `process.report.signal`.
+
+```js
+console.log(`Report on signal: ${process.report.reportOnSignal}`);
+```
+
+## process.report.reportOnUncaughtException
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean}
+
+If `true`, a diagnostic report is generated on uncaught exception.
+
+```js
+console.log(`Report on exception: ${process.report.reportOnUncaughtException}`);
+```
+
+## process.report.signal
+<!-- YAML
+added: REPLACEME
+-->
+
+* {string}
+
+The signal used to trigger the creation of a diagnostic report. Defaults to
+`SIGUSR2`.
+
+```js
+console.log(`Report signal: ${process.report.signal}`);
+```
 
 ### process.report.triggerReport([filename][, err])
 <!-- YAML
@@ -1732,7 +1779,7 @@ added: v11.8.0
 
 * `filename` {string} Name of the file where the report is written. This
   should be a relative path, that will be appended to the directory specified in
-  `process.report.setOptions`, or the current working directory of the Node.js
+  `process.report.directory`, or the current working directory of the Node.js
   process, if unspecified.
 * `err` {Error} A custom error used for reporting the JavaScript stack.
 

diff --git a/doc/api/report.md b/doc/api/report.md
@@ -484,21 +484,17 @@ times for the same Node.js process.
 
 ## Configuration
 
-Additional runtime configuration that influences the report generation
-constraints are available using `setOptions()` API.
+Additional runtime configuration of report generation is available via
+the following properties of `process.report`:
 
-```js
-process.report.setOptions({
-  events: ['exception', 'fatalerror', 'signal'],
-  signal: 'SIGUSR2',
-  filename: 'myreport.json',
-  path: '/home/nodeuser'
-});
-```
+`reportOnFatalError` triggers diagnostic reporting on fatal errors when `true`.
+Defaults to `false`.
+
+`reportOnSignal` triggers diagnostic reporting on signal when `true`. This is
+not supported on Windows. Defaults to `false`.
 
-The `events` array contains one or more of the report triggering options.
-The only valid entries are `'exception'`, `'fatalerror'` and `'signal'`.
-By default, a report is not produced on any of these events.
+`reportOnUncaughtException` triggers diagnostic reporting on uncaught exception
+when `true`. Defaults to `false`.
 
 `signal` specifies the POSIX signal identifier that will be used
 to intercept external triggers for report generation. Defaults to
@@ -507,24 +503,30 @@ to intercept external triggers for report generation. Defaults to
 `filename` specifies the name of the output file in the file system.
 Special meaning is attached to `stdout` and `stderr`. Usage of these
 will result in report being written to the associated standard streams.
-In such cases when standard streams are used, value in `'path'` is ignored.
+In cases where standard streams are used, the value in `'directory'` is ignored.
 URLs are not supported. Defaults to a composite filename that contains
 timestamp, PID and sequence number.
 
-`path` specifies the filesystem directory where the report will be written to.
+`directory` specifies the filesystem directory where the report will be written.
 URLs are not supported. Defaults to the current working directory of the
 Node.js process.
 
 ```js
 // Trigger report only on uncaught exceptions.
-process.report.setOptions({ events: ['exception'] });
+process.report.reportOnFatalError = false;
+process.report.reportOnSignal = false;
+process.report.reportOnUncaughtException = true;
 
 // Trigger report for both internal errors as well as external signal.
-process.report.setOptions({ events: ['fatalerror', 'signal'] });
+process.report.reportOnFatalError = true;
+process.report.reportOnSignal = true;
+process.report.reportOnUncaughtException = false;
 
 // Change the default signal to `SIGQUIT` and enable it.
-process.report.setOptions(
-  { events: ['signal'], signal: 'SIGQUIT' });
+process.report.reportOnFatalError = false;
+process.report.reportOnUncaughtException = false;
+process.report.reportOnSignal = true;
+process.report.signal = 'SIGQUIT';
 ```
 
 Configuration on module initialization is also available via

diff --git a/lib/internal/bootstrap/pre_execution.js b/lib/internal/bootstrap/pre_execution.js
@@ -40,14 +40,8 @@ function initializeReport() {
   if (!getOptionValue('--experimental-report')) {
     return;
   }
-  const {
-    config,
-    report,
-    syncConfig
-  } = require('internal/process/report');
+  const { report } = require('internal/process/report');
   process.report = report;
-  // Download the CLI / ENV config into JS land.
-  syncConfig(config, false);
 }
 
 function setupSignalHandlers() {
@@ -68,13 +62,10 @@ function initializeReportSignalHandlers() {
   if (!getOptionValue('--experimental-report')) {
     return;
   }
-  const {
-    config,
-    handleSignal
-  } = require('internal/process/report');
-  if (config.events.includes('signal')) {
-    process.on(config.signal, handleSignal);
-  }
+
+  const { addSignalHandler } = require('internal/process/report');
+
+  addSignalHandler();
 }
 
 function setupTraceCategoryState() {

diff --git a/lib/internal/process/execution.js b/lib/internal/process/execution.js
@@ -109,18 +109,11 @@ function createFatalException() {
     if (er == null || er.domain == null) {
       try {
         const report = internalBinding('report');
-        if (report != null &&
-            require('internal/options')
-              .getOptionValue('--experimental-report')) {
-          const config = {};
-          report.syncConfig(config, false);
-          if (Array.isArray(config.events) &&
-              config.events.includes('exception')) {
-            report.triggerReport(er ? er.message : 'Exception',
-                                 'Exception',
-                                 null,
-                                 er ? er.stack : undefined);
-          }
+        if (report != null && report.shouldReportOnUncaughtException()) {
+          report.triggerReport(er ? er.message : 'Exception',
+                               'Exception',
+                               null,
+                               er ? er.stack : undefined);
         }
       } catch {}  // Ignore the exception. Diagnostic reporting is unavailable.
     }