Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

report: refactor and rename setDiagnosticReportOptions() #25990

Merged
merged 2 commits into from
Feb 9, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 7 additions & 12 deletions doc/api/process.md
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ console.log(data);

Additional documentation is available in the [report documentation][].

### process.report.setDiagnosticReportOptions([options]);
### process.report.setOptions([options]);
<!-- YAML
added: v11.8.0
-->
Expand All @@ -1712,23 +1712,18 @@ are shown below.

```js
// Trigger a report on uncaught exceptions or fatal errors.
process.report.setDiagnosticReportOptions({
events: ['exception', 'fatalerror']
});
process.report.setOptions({ events: ['exception', 'fatalerror'] });

// Change the default path and filename of the report.
process.report.setDiagnosticReportOptions({
filename: 'foo.json',
path: '/home'
});
process.report.setOptions({ filename: 'foo.json', path: '/home' });

// 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.setDiagnosticReportOptions({ filename: 'stdout' });
process.report.setOptions({ filename: 'stdout' });

// Enable verbose option on report generation.
process.report.setDiagnosticReportOptions({ verbose: true });
process.report.setOptions({ verbose: true });
```

Signal based report generation is not supported on Windows.
Expand All @@ -1742,8 +1737,8 @@ 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.setDiagnosticReportOptions`, or the current working directory
of the Node.js process, if unspecified.
`process.report.setOptions`, or the current working directory of the Node.js
process, if unspecified.
* `err` {Error} A custom error used for reporting the JavsScript stack.

* Returns: {string} Returns the filename of the generated report.
Expand Down
10 changes: 5 additions & 5 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,10 @@ times for the same Node.js process.
## Configuration

Additional runtime configuration that influences the report generation
constraints are available using `setDiagnosticReportOptions()` API.
constraints are available using `setOptions()` API.

```js
process.report.setDiagnosticReportOptions({
process.report.setOptions({
events: ['exception', 'fatalerror', 'signal'],
signal: 'SIGUSR2',
filename: 'myreport.json',
Expand Down Expand Up @@ -481,13 +481,13 @@ pertinent to the report generation. Defaults to `false`.

```js
// Trigger report only on uncaught exceptions.
process.report.setDiagnosticReportOptions({ events: ['exception'] });
process.report.setOptions({ events: ['exception'] });

// Trigger report for both internal errors as well as external signal.
process.report.setDiagnosticReportOptions({ events: ['fatalerror', 'signal'] });
process.report.setOptions({ events: ['fatalerror', 'signal'] });

// Change the default signal to `SIGQUIT` and enable it.
process.report.setDiagnosticReportOptions(
process.report.setOptions(
{ events: ['signal'], signal: 'SIGQUIT' });
```

Expand Down
135 changes: 51 additions & 84 deletions lib/internal/process/report.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
'use strict';

const { emitExperimentalWarning } = require('internal/util');
const {
convertToValidSignal,
emitExperimentalWarning
} = require('internal/util');
const {
ERR_INVALID_ARG_TYPE,
ERR_SYNTHETIC
} = require('internal/errors').codes;

const REPORTEVENTS = 1;
const REPORTSIGNAL = 2;
const REPORTFILENAME = 3;
const REPORTPATH = 4;
const REPORTVERBOSE = 5;

// If report is enabled, extract the binding and
// wrap the APIs with thin layers, with some error checks.
// user options can come in from CLI / ENV / API.
Expand All @@ -33,70 +29,58 @@ let config = {
verbose: false
};
const report = {
setDiagnosticReportOptions(options) {
setOptions(options) {
emitExperimentalWarning('report');
// Reuse the null and undefined checks. Save
// space when dealing with large number of arguments.
const list = parseOptions(options);
const previousConfig = config;
const newConfig = {};

// Flush the stale entries from report, as
// we are refreshing it, items that the users did not
// touch may be hanging around stale otherwise.
config = {};
if (options === null || typeof options !== 'object')
options = {};

// The parseOption method returns an array that include
// the indices at which valid params are present.
list.forEach((i) => {
switch (i) {
case REPORTEVENTS:
if (Array.isArray(options.events))
config.events = options.events;
else
throw new ERR_INVALID_ARG_TYPE('events',
'Array',
options.events);
break;
case REPORTSIGNAL:
if (typeof options.signal !== 'string') {
throw new ERR_INVALID_ARG_TYPE('signal',
'String',
options.signal);
}
process.removeListener(config.signal, handleSignal);
if (config.events.includes('signal'))
process.on(options.signal, handleSignal);
config.signal = options.signal;
break;
case REPORTFILENAME:
if (typeof options.filename !== 'string') {
throw new ERR_INVALID_ARG_TYPE('filename',
'String',
options.filename);
}
config.filename = options.filename;
break;
case REPORTPATH:
if (typeof options.path !== 'string')
throw new ERR_INVALID_ARG_TYPE('path', 'String', options.path);
config.path = options.path;
break;
case REPORTVERBOSE:
if (typeof options.verbose !== 'string' &&
typeof options.verbose !== 'boolean') {
throw new ERR_INVALID_ARG_TYPE('verbose',
'Booelan | String' +
' (true|false|yes|no)',
options.verbose);
}
config.verbose = options.verbose;
break;
}
});
// Upload this new config to C++ land
nr.syncConfig(config, true);
},
if (Array.isArray(options.events))
newConfig.events = options.events.slice();
else if (options.events === undefined)
newConfig.events = [];
else
throw new ERR_INVALID_ARG_TYPE('events', 'Array', options.events);

if (typeof options.filename === 'string')
newConfig.filename = options.filename;
else if (options.filename === undefined)
newConfig.filename = '';
else
throw new ERR_INVALID_ARG_TYPE('filename', 'string', options.filename);

if (typeof options.path === 'string')
newConfig.path = options.path;
else if (options.path === undefined)
newConfig.path = '';
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)
newConfig.signal = 'SIGUSR2';
else
throw new ERR_INVALID_ARG_TYPE('signal', 'string', options.signal);

if (previousConfig.signal)
process.removeListener(previousConfig.signal, handleSignal);

if (newConfig.events.includes('signal'))
process.on(newConfig.signal, handleSignal);

config = newConfig;
nr.syncConfig(config, true);
},
triggerReport(file, err) {
emitExperimentalWarning('report');
if (err == null) {
Expand Down Expand Up @@ -133,23 +117,6 @@ function handleSignal(signo) {
nr.onUserSignal(signo);
}

function parseOptions(obj) {
const list = [];
if (obj == null)
return list;
if (obj.events != null)
list.push(REPORTEVENTS);
if (obj.signal != null)
list.push(REPORTSIGNAL);
if (obj.filename != null)
list.push(REPORTFILENAME);
if (obj.path != null)
list.push(REPORTPATH);
if (obj.verbose != null)
list.push(REPORTVERBOSE);
return list;
}

module.exports = {
config,
handleSignal,
Expand Down