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

Diagnostics report: inspect the error object #28415

Closed
mmarchini opened this issue Jun 25, 2019 · 4 comments
Closed

Diagnostics report: inspect the error object #28415

mmarchini opened this issue Jun 25, 2019 · 4 comments
Labels
feature request Issues that request new features to be added to Node.js. report Issues and PRs related to process.report.

Comments

@mmarchini
Copy link
Contributor

mmarchini commented Jun 25, 2019

Is your feature request related to a problem? Please describe.
Now we inspect the error object on fatal errors, which is awesome. I started to play with node-report recently and this feature was not incorporated there yet. I think this would make a lot of sense since it would make the report more useful for users.

Describe the solution you'd like
Take the program below as an example:

// crash.js
function crash() { 
  let e = new Error('boom'); 
  e.val = 1; 
  throw e;
} 

crash();

Today it will output the following stack trace (see how it also prints the custom attribute we added):

$ ./node ./crash.js
/home/mmarchini/workspace/nodejs/node/crash.js:5
  throw e;
  ^

Error: boom
    at crash (/home/mmarchini/workspace/nodejs/node/crash.js:3:11)
    at Object.<anonymous> (/home/mmarchini/workspace/nodejs/node/crash.js:8:1)
    at Module._compile (internal/modules/cjs/loader.js:779:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:790:10)
    at Module.load (internal/modules/cjs/loader.js:642:32)
    at Function.Module._load (internal/modules/cjs/loader.js:555:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:842:10)
    at internal/main/run_main_module.js:17:11 {
  val: 1
}

The report for this error looks like this today:

  "javascriptStack": {
    "message": "[eval]:1",
    "stack": [
      "at crash (/home/mmarchini/workspace/nodejs/node/crash.js:3:11)",
      "at Object.<anonymous> (/home/mmarchini/workspace/nodejs/node/crash.js:8:1)",
      "at Module._compile (internal/modules/cjs/loader.js:779:30)",
      "at Object.Module._extensions..js (internal/modules/cjs/loader.js:790:10)",
      "at Module.load (internal/modules/cjs/loader.js:642:32)",
      "at Function.Module._load (internal/modules/cjs/loader.js:555:12)",
      "at Function.Module.runMain (internal/modules/cjs/loader.js:842:10)"
    ]
  },

It would be amazing if we had the properties as well, as shown below:

  "javascriptStack": {
    "message": "[eval]:1",
    "stack": [
      "at crash (/home/mmarchini/workspace/nodejs/node/crash.js:3:11)",
      "at Object.<anonymous> (/home/mmarchini/workspace/nodejs/node/crash.js:8:1)",
      "at Module._compile (internal/modules/cjs/loader.js:779:30)",
      "at Object.Module._extensions..js (internal/modules/cjs/loader.js:790:10)",
      "at Module.load (internal/modules/cjs/loader.js:642:32)",
      "at Function.Module._load (internal/modules/cjs/loader.js:555:12)",
      "at Function.Module.runMain (internal/modules/cjs/loader.js:842:10)"
    ],
    "errorProperties": {
      "val": 1
    }
  },

Describe alternatives you've considered
The only alternative as powerful as this would be core dumps, but the entry barrier for core dumps is way higher than node-report.

@richardlau richardlau added feature request Issues that request new features to be added to Node.js. report Issues and PRs related to process.report. labels Jun 25, 2019
@himself65
Copy link
Member

related code lines:

node/src/node_report.cc

Lines 352 to 388 in c42da5e

static void PrintJavaScriptStack(JSONWriter* writer,
Isolate* isolate,
Local<String> stackstr,
const char* trigger) {
writer->json_objectstart("javascriptStack");
std::string ss;
if ((!strcmp(trigger, "FatalError")) ||
(!strcmp(trigger, "Signal"))) {
ss = "No stack.\nUnavailable.\n";
} else {
String::Utf8Value sv(isolate, stackstr);
ss = std::string(*sv, sv.length());
}
int line = ss.find('\n');
if (line == -1) {
writer->json_keyvalue("message", ss);
writer->json_objectend();
} else {
std::string l = ss.substr(0, line);
writer->json_keyvalue("message", l);
writer->json_arraystart("stack");
ss = ss.substr(line + 1);
line = ss.find('\n');
while (line != -1) {
l = ss.substr(0, line);
l.erase(l.begin(), std::find_if(l.begin(), l.end(), [](int ch) {
return !std::iswspace(ch);
}));
writer->json_element(l);
ss = ss.substr(line + 1);
line = ss.find('\n');
}
}
writer->json_arrayend();
writer->json_objectend();
}

@himself65
Copy link
Member

I'm working on this.

@himself65
Copy link
Member

I think I've done this, see #28426

@himself65
Copy link
Member

landed in 870f0fc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. report Issues and PRs related to process.report.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants