Skip to content

Commit

Permalink
report: modify getReport() to return an Object
Browse files Browse the repository at this point in the history
It's likely that anyone using `process.report.getReport()` will be
processing the return value thereafter (e.g., filtering fields or
redacting secrets). This change eliminates boilerplate by calling
`JSON.parse()` on the return value.

Also modified the `validateContent()` and `validate()` test helpers in
`test/common/report.js` to be somewhat more obvious and helpful. Of
note, a report failing validation will now be easier (though still not
_easy_) to read when prepended to the stack trace.

- Refs: nodejs/diagnostics#315

PR-URL: #28630
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
boneskull committed Jul 12, 2019
1 parent 28e18cf commit bff7a46
Show file tree
Hide file tree
Showing 7 changed files with 44 additions and 26 deletions.
13 changes: 9 additions & 4 deletions doc/api/process.md
Expand Up @@ -1726,14 +1726,19 @@ added: v11.8.0
-->

* `err` {Error} A custom error used for reporting the JavaScript stack.
* Returns: {string}
* Returns: {Object}

Returns a JSON-formatted diagnostic report for the running process. The report's
JavaScript stack trace is taken from `err`, if present.
Returns a JavaScript Object representation of a diagnostic report for the
running process. The report's JavaScript stack trace is taken from `err`, if
present.

```js
const data = process.report.getReport();
console.log(data);
console.log(data.header.nodeJsVersion);

// Similar to process.report.writeReport()
const fs = require('fs');
fs.writeFileSync(util.inspect(data), 'my-report.log', 'utf8');
```

Additional documentation is available in the [report documentation][].
Expand Down
11 changes: 7 additions & 4 deletions doc/api/report.md
Expand Up @@ -463,20 +463,23 @@ try {
// Any other code
```

The content of the diagnostic report can be returned as a JSON-compatible object
The content of the diagnostic report can be returned as a JavaScript Object
via an API call from a JavaScript application:

```js
const report = process.report.getReport();
console.log(report);
console.log(typeof report === 'object'); // true

// Similar to process.report.writeReport() output
console.log(JSON.stringify(report, null, 2));
```

This function takes an optional additional argument `err` - an `Error` object
that will be used as the context for the JavaScript stack printed in the report.

```js
const report = process.report.getReport(new Error('custom error'));
console.log(report);
console.log(typeof report === 'object'); // true
```

The API versions are useful when inspecting the runtime state from within
Expand All @@ -498,7 +501,7 @@ Node.js report completed
>
```

When a report is triggered, start and end messages are issued to stderr
When a report is written, start and end messages are issued to stderr
and the filename of the report is returned to the caller. The default filename
includes the date, time, PID and a sequence number. The sequence number helps
in associating the report dump with the runtime state if generated multiple
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/process/report.js
Expand Up @@ -5,6 +5,7 @@ const {
} = require('internal/errors').codes;
const { validateSignalName, validateString } = require('internal/validators');
const nr = internalBinding('report');
const { JSON } = primordials;
const report = {
writeReport(file, err) {
if (typeof file === 'object' && file !== null) {
Expand All @@ -26,7 +27,7 @@ const report = {
else if (err === null || typeof err !== 'object')
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);

return nr.getReport(err.stack);
return JSON.parse(nr.getReport(err.stack));
},
get directory() {
return nr.getDirectory();
Expand Down
2 changes: 1 addition & 1 deletion test/addons/worker-addon/test.js
Expand Up @@ -31,7 +31,7 @@ switch (process.argv[2]) {
}

// Use process.report to figure out if we might be running under musl libc.
const glibc = JSON.parse(process.report.getReport()).header.glibcVersionRuntime;
const glibc = process.report.getReport().header.glibcVersionRuntime;
assert(typeof glibc === 'string' || glibc === undefined, glibc);

const libcMayBeMusl = common.isLinux && glibc === undefined;
Expand Down
13 changes: 7 additions & 6 deletions test/common/README.md
Expand Up @@ -859,19 +859,20 @@ functionality.
Returns an array of diagnotic report file names found in `dir`. The files should
have been generated by a process whose PID matches `pid`.

### validate(report)
### validate(filepath)

* `report` [&lt;string>] Diagnostic report file name to validate.
* `filepath` [&lt;string>] Diagnostic report filepath to validate.

Validates the schema of a diagnostic report file whose path is specified in
`report`. If the report fails validation, an exception is thrown.
`filepath`. If the report fails validation, an exception is thrown.

### validateContent(data)
### validateContent(report)

* `data` [&lt;string>] Contents of a diagnostic report file.
* `report` [&lt;Object|string>] JSON contents of a diagnostic report file, the
parsed Object thereof, or the result of `process.report.getReport()`.

Validates the schema of a diagnostic report whose content is specified in
`data`. If the report fails validation, an exception is thrown.
`report`. If the report fails validation, an exception is thrown.

## tick Module

Expand Down
26 changes: 17 additions & 9 deletions test/common/report.js
Expand Up @@ -4,6 +4,7 @@ const assert = require('assert');
const fs = require('fs');
const os = require('os');
const path = require('path');
const util = require('util');

function findReports(pid, dir) {
// Default filenames are of the form
Expand All @@ -21,24 +22,31 @@ function findReports(pid, dir) {
return results;
}

function validate(report) {
const data = fs.readFileSync(report, 'utf8');

validateContent(data);
function validate(filepath) {
validateContent(JSON.parse(fs.readFileSync(filepath, 'utf8')));
}

function validateContent(data) {
function validateContent(report) {
if (typeof report === 'string') {
try {
report = JSON.parse(report);
} catch {
throw new TypeError(
'validateContent() expects a JSON string or JavaScript Object');
}
}
try {
_validateContent(data);
_validateContent(report);
} catch (err) {
err.stack += `\n------\nFailing Report:\n${data}`;
try {
err.stack += util.format('\n------\nFailing Report:\n%O', report);
} catch {}
throw err;
}
}

function _validateContent(data) {
function _validateContent(report) {
const isWindows = process.platform === 'win32';
const report = JSON.parse(data);

// Verify that all sections are present as own properties of the report.
const sections = ['header', 'javascriptStack', 'nativeStack',
Expand Down
2 changes: 1 addition & 1 deletion test/report/test-report-uv-handles.js
Expand Up @@ -43,7 +43,7 @@ if (process.argv[2] === 'child') {
const server = http.createServer((req, res) => {
req.on('end', () => {
// Generate the report while the connection is active.
console.log(process.report.getReport());
console.log(JSON.stringify(process.report.getReport(), null, 2));
child_process.kill();

res.writeHead(200, { 'Content-Type': 'text/plain' });
Expand Down

0 comments on commit bff7a46

Please sign in to comment.