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

src: add errorProperties on process.report #28426

Closed
wants to merge 2 commits into from
Closed
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
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 @@ -433,7 +433,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()) {
himself65 marked this conversation as resolved.
Show resolved Hide resolved
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)) {
himself65 marked this conversation as resolved.
Show resolved Hide resolved
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