frida / frida-node

Frida Node.js bindings
280 stars 65 forks source link

Backtrace absent from thrown errors #31

Closed mvastola closed 5 years ago

mvastola commented 6 years ago

So I just learned this, but apparently, in nodejs, the code

async function run() {
  throw "Random error.";
}
function onError(err) {
  console.error(`[*] Fatal Error: ${err.message}`);
  console.trace(err);
}
run().catch(onError);

will produce a trace as follows:

[*] Fatal Error: undefined
Trace: Random error.
    at onError (./errtest.js:6:11)
    at process._tickCallback (internal/process/next_tick.js:68:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:745:11)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)

The problem being, the stack trace does not include the frame at which the error occurred (in this case, line 2). This makes it extremely difficult to debug complex apps.

Apparently, the solution is to, instead of:

throw "Random error.";

.. use:

throw new Error("Random error.");

This produces the following backtrace:

[*] Fatal Error: Random error.
Trace: Error: Random error.
    at run (./errtest.js:2:9)
    at Object.<anonymous> (./errtest.js:8:1)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)
    at onError (./errtest.js:6:11)
    at process._tickCallback (internal/process/next_tick.js:68:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:745:11)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)

.. which is infinitely more helpful.

Currently, this package uses the (v8 equivalent of the) first syntax.

Due to frida/frida#592, I currently cannot test this out, but it would seem that this is an easy fix: Given, say:

Nan::ThrowTypeError("Bad argument, expected Buffer");

.. instead use:

Nan::ThrowTypeError(new Nan::TypeError("Bad argument, expected Buffer"));

Could this change be made globally in this repo?

Nihilight commented 6 years ago

+1 Not sure if it's related, but I'm recieveing the following error, without a stack trace at all. For example: [0;31mUnhandled promise rejection TypeError: ...

oleavr commented 6 years ago

Great catch! Anyone interested in opening a PR?

Nihilight commented 6 years ago

I'm not sure adding the TypeError instead of the string would change anything. Look at part I emphasized from the documentation on Nan errors

Nan::ThrowTypeError()

Throw an TypeError object (a specialized v8::Value as above) in the current context. If a msg is provided, a new TypeError object will be created.

But searching for [0;31m brought me to try_handle_log_message (inject.vala and gadget.vala) Perhaps there is more data to print from the message?

oleavr commented 6 years ago

@Nihilight That logic handles the type: "log" messages sent when using console.log() and friends on the agent side. Language bindings typically handle those and hide them from the user's message callback. However for frida-inject and frida-gadget (in standalone mode) those need their own handling. Anyway, log messages don't carry any stack-trace, and this issue is about frida-node APIs throwing.

oleavr commented 5 years ago

Timed out. Will probably be addressed by an N-API rewrite at some point.