From bff7a46f31f1da259071a1f4bd51aca726d5926e Mon Sep 17 00:00:00 2001 From: Christopher Hiller Date: Wed, 10 Jul 2019 13:27:44 -0700 Subject: [PATCH] report: modify getReport() to return an Object 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: https://github.com/nodejs/diagnostics/issues/315 PR-URL: https://github.com/nodejs/node/pull/28630 Reviewed-By: Anna Henningsen Reviewed-By: Richard Lau Reviewed-By: Colin Ihrig Reviewed-By: Jiawen Geng Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- doc/api/process.md | 13 +++++++++---- doc/api/report.md | 11 +++++++---- lib/internal/process/report.js | 3 ++- test/addons/worker-addon/test.js | 2 +- test/common/README.md | 13 +++++++------ test/common/report.js | 26 +++++++++++++++++--------- test/report/test-report-uv-handles.js | 2 +- 7 files changed, 44 insertions(+), 26 deletions(-) diff --git a/doc/api/process.md b/doc/api/process.md index 9bf2b7dc235df1..c5833e5bf44ad2 100644 --- a/doc/api/process.md +++ b/doc/api/process.md @@ -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][]. diff --git a/doc/api/report.md b/doc/api/report.md index 268caf64160a7d..08a0f602abda3a 100644 --- a/doc/api/report.md +++ b/doc/api/report.md @@ -463,12 +463,15 @@ 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 @@ -476,7 +479,7 @@ 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 @@ -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 diff --git a/lib/internal/process/report.js b/lib/internal/process/report.js index 251c32a109d55a..cc78aebbdaa358 100644 --- a/lib/internal/process/report.js +++ b/lib/internal/process/report.js @@ -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) { @@ -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(); diff --git a/test/addons/worker-addon/test.js b/test/addons/worker-addon/test.js index ef158e98b14469..85a5e2701609fd 100644 --- a/test/addons/worker-addon/test.js +++ b/test/addons/worker-addon/test.js @@ -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; diff --git a/test/common/README.md b/test/common/README.md index 2b8f852a3b74dd..0fd03ce767265a 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -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` [<string>] Diagnostic report file name to validate. +* `filepath` [<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` [<string>] Contents of a diagnostic report file. +* `report` [<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 diff --git a/test/common/report.js b/test/common/report.js index 7637e5ed956044..eda3ad4ae2471e 100644 --- a/test/common/report.js +++ b/test/common/report.js @@ -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 @@ -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', diff --git a/test/report/test-report-uv-handles.js b/test/report/test-report-uv-handles.js index 20619204325f41..4d9bb12b44ff03 100644 --- a/test/report/test-report-uv-handles.js +++ b/test/report/test-report-uv-handles.js @@ -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' });