Skip to content

Commit

Permalink
src: add errorProperties on process.report
Browse files Browse the repository at this point in the history
PR-URL: #28426
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
himself65 authored and codebytere committed Jun 27, 2020
1 parent 0c32920 commit 9760c6c
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 38 deletions.
2 changes: 1 addition & 1 deletion lib/internal/process/execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ function createOnGlobalUncaughtException() {
report.writeReport(er ? er.message : 'Exception',
'Exception',
null,
er ? er.stack : undefined);
er ? er : {});
}
} catch {} // Ignore the exception. Diagnostic reporting is unavailable.
}
Expand Down
4 changes: 2 additions & 2 deletions lib/internal/process/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ const report = {
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);
}

return nr.writeReport('JavaScript API', 'API', file, err.stack);
return nr.writeReport('JavaScript API', 'API', file, err);
},
getReport(err) {
if (err === undefined)
err = new ERR_SYNTHETIC();
else if (err === null || typeof err !== 'object')
throw new ERR_INVALID_ARG_TYPE('err', 'Object', err);

return JSONParse(nr.getReport(err.stack));
return JSONParse(nr.getReport(err));
},
get directory() {
return nr.getDirectory();
Expand Down
2 changes: 1 addition & 1 deletion src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ void OnFatalError(const char* location, const char* message) {

if (report_on_fatalerror) {
report::TriggerNodeReport(
isolate, env, message, "FatalError", "", Local<String>());
isolate, env, message, "FatalError", "", Local<Object>());
}

fflush(stderr);
Expand Down
92 changes: 72 additions & 20 deletions src/node_report.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,18 @@ using node::Mutex;
using node::NativeSymbolDebuggingContext;
using node::TIME_TYPE;
using node::worker::Worker;
using v8::Array;
using v8::Context;
using v8::HeapSpaceStatistics;
using v8::HeapStatistics;
using v8::Isolate;
using v8::Local;
using v8::Number;
using v8::Object;
using v8::StackTrace;
using v8::String;
using v8::TryCatch;
using v8::Value;
using v8::V8;

namespace per_process = node::per_process;
Expand All @@ -53,13 +60,16 @@ static void WriteNodeReport(Isolate* isolate,
const char* trigger,
const std::string& filename,
std::ostream& out,
Local<String> stackstr,
Local<Object> error,
bool compact);
static void PrintVersionInformation(JSONWriter* writer);
static void PrintJavaScriptStack(JSONWriter* writer,
Isolate* isolate,
Local<String> stackstr,
const char* trigger);
static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<Object> error,
const char* trigger);
static void PrintJavaScriptErrorProperties(JSONWriter* writer,
Isolate* isolate,
Local<Object> error);
static void PrintNativeStack(JSONWriter* writer);
static void PrintResourceUsage(JSONWriter* writer);
static void PrintGCStatistics(JSONWriter* writer, Isolate* isolate);
Expand All @@ -76,7 +86,7 @@ std::string TriggerNodeReport(Isolate* isolate,
const char* message,
const char* trigger,
const std::string& name,
Local<String> stackstr) {
Local<Object> error) {
std::string filename;

// Determine the required report filename. In order of priority:
Expand Down Expand Up @@ -142,7 +152,7 @@ std::string TriggerNodeReport(Isolate* isolate,
compact = per_process::cli_options->report_compact;
}
WriteNodeReport(isolate, env, message, trigger, filename, *outstream,
stackstr, compact);
error, compact);

// Do not close stdout/stderr, only close files we opened.
if (outfile.is_open()) {
Expand All @@ -161,9 +171,9 @@ void GetNodeReport(Isolate* isolate,
Environment* env,
const char* message,
const char* trigger,
Local<String> stackstr,
Local<Object> error,
std::ostream& out) {
WriteNodeReport(isolate, env, message, trigger, "", out, stackstr, false);
WriteNodeReport(isolate, env, message, trigger, "", out, error, false);
}

// Internal function to coordinate and write the various
Expand All @@ -174,7 +184,7 @@ static void WriteNodeReport(Isolate* isolate,
const char* trigger,
const std::string& filename,
std::ostream& out,
Local<String> stackstr,
Local<Object> error,
bool compact) {
// Obtain the current time and the pid.
TIME_TYPE tm_struct;
Expand Down Expand Up @@ -259,8 +269,13 @@ static void WriteNodeReport(Isolate* isolate,
PrintVersionInformation(&writer);
writer.json_objectend();

// Report summary JavaScript stack backtrace
PrintJavaScriptStack(&writer, isolate, stackstr, trigger);
writer.json_objectstart("javascriptStack");
// Report summary JavaScript error stack backtrace
PrintJavaScriptErrorStack(&writer, isolate, error, trigger);

// Report summary JavaScript error properties backtrace
PrintJavaScriptErrorProperties(&writer, isolate, error);
writer.json_objectend(); // the end of 'javascriptStack'

// Report native stack backtrace
PrintNativeStack(&writer);
Expand Down Expand Up @@ -301,7 +316,7 @@ static void WriteNodeReport(Isolate* isolate,
env,
"Worker thread subreport",
trigger,
Local<String>(),
Local<Object>(),
os);

Mutex::ScopedLock lock(workers_mutex);
Expand Down Expand Up @@ -455,18 +470,56 @@ static void PrintNetworkInterfaceInfo(JSONWriter* writer) {
}
}

static void PrintJavaScriptErrorProperties(JSONWriter* writer,
Isolate* isolate,
Local<Object> error) {
writer->json_objectstart("errorProperties");
if (!error.IsEmpty()) {
TryCatch try_catch(isolate);
Local<Context> context = error->GetIsolate()->GetCurrentContext();
Local<Array> keys;
if (!error->GetOwnPropertyNames(context).ToLocal(&keys)) {
return writer->json_objectend(); // the end of 'errorProperties'
}
uint32_t keys_length = keys->Length();
for (uint32_t i = 0; i < keys_length; i++) {
Local<Value> key;
if (!keys->Get(context, i).ToLocal(&key) || !key->IsString()) {
continue;
}
Local<Value> value;
Local<String> value_string;
if (!error->Get(context, key).ToLocal(&value) ||
!value->ToString(context).ToLocal(&value_string)) {
continue;
}
String::Utf8Value k(isolate, key);
if (!strcmp(*k, "stack") || !strcmp(*k, "message")) continue;
String::Utf8Value v(isolate, value_string);
writer->json_keyvalue(std::string(*k, k.length()),
std::string(*v, v.length()));
}
}
writer->json_objectend(); // the end of 'errorProperties'
}

// Report the JavaScript stack.
static void PrintJavaScriptStack(JSONWriter* writer,
static void PrintJavaScriptErrorStack(JSONWriter* writer,
Isolate* isolate,
Local<String> stackstr,
Local<Object> error,
const char* trigger) {
writer->json_objectstart("javascriptStack");

std::string ss;
Local<Value> stackstr;
std::string ss = "";
TryCatch try_catch(isolate);
if ((!strcmp(trigger, "FatalError")) ||
(!strcmp(trigger, "Signal"))) {
ss = "No stack.\nUnavailable.\n";
} else {
} else if (!error.IsEmpty() &&
error
->Get(isolate->GetCurrentContext(),
node::FIXED_ONE_BYTE_STRING(isolate,
"stack"))
.ToLocal(&stackstr)) {
String::Utf8Value sv(isolate, stackstr);
ss = std::string(*sv, sv.length());
}
Expand All @@ -490,7 +543,6 @@ static void PrintJavaScriptStack(JSONWriter* writer,
}
writer->json_arrayend();
}
writer->json_objectend();
}

// Report a native stack backtrace
Expand Down
4 changes: 2 additions & 2 deletions src/node_report.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ std::string TriggerNodeReport(v8::Isolate* isolate,
const char* message,
const char* trigger,
const std::string& name,
v8::Local<v8::String> stackstr);
v8::Local<v8::Object> error);
void GetNodeReport(v8::Isolate* isolate,
node::Environment* env,
const char* message,
const char* trigger,
v8::Local<v8::String> stackstr,
v8::Local<v8::Object> error,
std::ostream& out);

// Function declarations - utility functions in src/node_report_utils.cc
Expand Down
18 changes: 14 additions & 4 deletions src/node_report_module.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,21 @@ void WriteReport(const FunctionCallbackInfo<Value>& info) {
Isolate* isolate = env->isolate();
HandleScope scope(isolate);
std::string filename;
Local<String> stackstr;
Local<Object> error;

CHECK_EQ(info.Length(), 4);
String::Utf8Value message(isolate, info[0].As<String>());
String::Utf8Value trigger(isolate, info[1].As<String>());
stackstr = info[3].As<String>();

if (info[2]->IsString())
filename = *String::Utf8Value(isolate, info[2]);
if (!info[3].IsEmpty() && info[3]->IsObject())
error = info[3].As<Object>();
else
error = Local<Object>();

filename = TriggerNodeReport(
isolate, env, *message, *trigger, filename, stackstr);
isolate, env, *message, *trigger, filename, error);
// Return value is the report filename
info.GetReturnValue().Set(
String::NewFromUtf8(isolate, filename.c_str(), v8::NewStringType::kNormal)
Expand All @@ -55,10 +58,17 @@ void GetReport(const FunctionCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
Isolate* isolate = env->isolate();
HandleScope scope(isolate);
Local<Object> error;
std::ostringstream out;

CHECK_EQ(info.Length(), 1);
if (!info[0].IsEmpty() && info[0]->IsObject())
error = info[0].As<Object>();
else
error = Local<Object>();

GetNodeReport(
isolate, env, "JavaScript API", __func__, info[0].As<String>(), out);
isolate, env, "JavaScript API", __func__, error, out);

// Return value is the contents of a report as a string.
info.GetReturnValue().Set(String::NewFromUtf8(isolate,
Expand Down
37 changes: 30 additions & 7 deletions test/common/report.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ function findReports(pid, dir) {
return results;
}

function validate(filepath) {
function validate(filepath, fields) {
const report = fs.readFileSync(filepath, 'utf8');
if (process.report.compact) {
const end = report.indexOf('\n');
assert.strictEqual(end, report.length - 1);
}
validateContent(JSON.parse(report));
validateContent(JSON.parse(report), fields);
}

function validateContent(report) {
function validateContent(report, fields = []) {
if (typeof report === 'string') {
try {
report = JSON.parse(report);
Expand All @@ -43,7 +43,7 @@ function validateContent(report) {
}
}
try {
_validateContent(report);
_validateContent(report, fields);
} catch (err) {
try {
err.stack += util.format('\n------\nFailing Report:\n%O', report);
Expand All @@ -52,7 +52,7 @@ function validateContent(report) {
}
}

function _validateContent(report) {
function _validateContent(report, fields = []) {
const isWindows = process.platform === 'win32';

// Verify that all sections are present as own properties of the report.
Expand All @@ -71,6 +71,26 @@ function _validateContent(report) {
assert(typeof report[section] === 'object' && report[section] !== null);
});

fields.forEach((field) => {
function checkLoop(actual, rest, expect) {
actual = actual[rest.shift()];
if (rest.length === 0 && actual !== undefined) {
assert.strictEqual(actual, expect);
} else {
assert(actual);
checkLoop(actual, rest, expect);
}
}
let actual, expect;
if (Array.isArray(field)) {
[actual, expect] = field;
} else {
actual = field;
expect = undefined;
}
checkLoop(report, actual.split('.'), expect);
});

// Verify the format of the header section.
const header = report.header;
const headerFields = ['event', 'trigger', 'filename', 'dumpEventTime',
Expand Down Expand Up @@ -144,7 +164,10 @@ function _validateContent(report) {
assert.strictEqual(header.host, os.hostname());

// Verify the format of the javascriptStack section.
checkForUnknownFields(report.javascriptStack, ['message', 'stack']);
checkForUnknownFields(report.javascriptStack,
['message', 'stack', 'errorProperties']);
assert.strictEqual(typeof report.javascriptStack.errorProperties,
'object');
assert.strictEqual(typeof report.javascriptStack.message, 'string');
if (report.javascriptStack.stack !== undefined) {
assert(Array.isArray(report.javascriptStack.stack));
Expand Down Expand Up @@ -262,7 +285,7 @@ function _validateContent(report) {

// Verify the format of the workers section.
assert(Array.isArray(report.workers));
report.workers.forEach(_validateContent);
report.workers.forEach((worker) => _validateContent(worker));
}

function checkForUnknownFields(actual, expected) {
Expand Down
7 changes: 7 additions & 0 deletions test/report/test-report-getreport.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,13 @@ const helper = require('../common/report');
assert.deepStrictEqual(helper.findReports(process.pid, process.cwd()), []);
}

{
const error = new Error();
error.foo = 'goo';
helper.validateContent(process.report.getReport(error),
[['javascriptStack.errorProperties.foo', 'goo']]);
}

// Test with an invalid error argument.
[null, 1, Symbol(), function() {}, 'foo'].forEach((error) => {
assert.throws(() => {
Expand Down
9 changes: 8 additions & 1 deletion test/report/test-report-writereport.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ process.report.directory = tmpdir.path;
function validate() {
const reports = helper.findReports(process.pid, tmpdir.path);
assert.strictEqual(reports.length, 1);
helper.validate(reports[0]);
helper.validate(reports[0], arguments[0]);
fs.unlinkSync(reports[0]);
return reports[0];
}
Expand All @@ -40,6 +40,13 @@ function validate() {
validate();
}

{
const error = new Error();
error.foo = 'goo';
process.report.writeReport(error);
validate([['javascriptStack.errorProperties.foo', 'goo']]);
}

{
// Test with a file argument.
const file = process.report.writeReport('custom-name-1.json');
Expand Down

0 comments on commit 9760c6c

Please sign in to comment.