Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

"Pause on uncaught exceptions" not pausing on "undefined is not a function" #7439

Closed
tgirardi opened this issue Apr 9, 2014 · 10 comments
Closed
Milestone

Comments

@tgirardi
Copy link

tgirardi commented Apr 9, 2014

Note: this issue was reproduced using node-inspector. I also posted it on node-inspector's issue tracker (node-inspector/node-inspector#344) and they suggested me to post it here also as it is not clear if this is a node issue or a node-inspector issue.

While using node-inspector and having "Pause on uncaught exception" option activated, the debugger won't stop at an exception of type "TypeError: undefined is not a function".

How to reproduce:

  1. Start node-inspector
  2. Create a code that will cause a "TypeError: undefined is not a function" exception.
  3. Start debugging the code
  4. Activate the "Pause on uncaught exceptions" option
  5. Run the whole code

Expected result:

The debugger pauses at the line where the exception is going to occur.

Actual result:

The debugger doesn't pause at the line where the exception occurs. The terminal shows that the exception occurred.

Tested under:

  • node v0.11.12 (also tested under v0.11.3, which is the version in which @bajtos' fix for uncaught exception was included)
  • node-inspector v0.7.3
  • mac os x v10.8.5
  • chrome v33.0.1750.152

Code used for my tests:

var undef;
undef();

Notes:

I tested with another type of uncaught exception and the debugger did pause. It was a "TypeError: Cannot read property 'prop' of undefined" exception caused by code: var undef; undef.prop;

@bajtos took a look at the issue and could also reproduce it, but he is short on time at the moment in order to investigate it further.

@indutny indutny added this to the v0.12 milestone Apr 24, 2014
@misterdjules
Copy link

@tgirardi I had some time to investigate this issue. Here's what I found out so far.

Node-inspector can't find the mapping between where the exception event occurred and any actual source file that was loaded. The reason is that, in the case of the call of an undefined function, the exception is thrown by a builtin CALL_NON_FUNCTION defined in v8/src/runtime.js. Thus, the script id of the event received by the debugger doesn't match any of the script loaded by node-inspector, and this code:

if (!source || source.hidden) {
this._debuggerClient.request('continue', { stepaction: 'out' });
return;
}

in BreakEventHandler.js makes node-inspector issue a continue command to the debugger, preventing node-inspector to pause the execution.

When accessing a property of an undefined object, the exception event that node-inspector receives from the debugger has the right script id and thus node-inspector can pause the debuggee.

It seems that V8 generates the code that handle function calls in memory (see code around EmitSlowCase in code-stubs-x64.cc). In this generated code, it checks if the function is defined, and if not it calculates the address of where the "CALL_NON_FUNCTION" code has also been generated in memory by the assembler:
__ GetBuiltinEntry(rdx, Builtins::CALL_NON_FUNCTION); Handle<Code> adaptor = isolate->builtins()->ArgumentsAdaptorTrampoline(); __ Jump(adaptor, RelocInfo::CODE_TARGET);
I don't know if the issue is because V8 handles calls to undefined functions differently now, or because of a change in node-inspector. I haven't had the time to look into that.

@indutny @trevnorris @tjfontaine Any idea?

@misterdjules
Copy link

Also /cc @bajtos

@trevnorris
Copy link

Not familiar with node-inspector's internals, but the following does work to properly halt execution on the tip of v0.12:

process.on('uncaughtException', function() {
  process._rawDebug('HERE');
  debugger;
});

var def;
def();

And run with:

$ ./node debugger catch-undef-fn.js

@misterdjules
Copy link

@trevnorris With your code example, node-inspector receives two events that could make it pause the debuggee:

  1. An exception event that represents this call to an undefined function:
var def;
def()

This event is associated with a source line that is in deps/v8/src/runtime.js. Because node-inspector can't find this source file in its source maps, it will not pause the debuggee. Instead, it sends a request to the debugger to continue the execution of the process.

  1. A break event generated by the debugger statement. This event is associated with the file that contains the example code. Node-inspector can find this source file in its source maps, and thus can pause the debuggee on that particular source line.

What I think is not working as expected by @tgirardi is the first step. Maybe the event generated at step one used to be associated with the actual source file of the call site, or maybe node-inspector changed how it behaves when the event's source file is not found in its source maps. It would be great to have some feedback about this from @bajtos.

@misterdjules
Copy link

@tgirardi I realize that I may have assumed too quickly that this feature used to work for previous versions of node-inspector/node. However, after testing several different combinations of node/node-inspector, I have yet to find one for which this feature works. Do you know if pausing the debuggee on a call to an undefined function has ever worked?

@3y3
Copy link

3y3 commented Aug 9, 2014

@trevnorris , @misterdjules , this issue is relative to #5713.
There @bajtos enables TryCatch verbose mode in some places to enable debugger notifications about uncaught errors.

TryCatch try_catch;
try_catch.setVerbose(true);

Node Inspector internals uses nothing special for handling uncaught errors, it only listens on exception event of v8 debugger protocol. I checked that "undefined is not a function" is not emitted (You can check it manually by logging in this place)

In other words I think that there is missed one or more try_catch.setVerbose(true);

@misterdjules
Copy link

@3y3 "undefined is not a function" is not emitted as is, but an exception event that comes from the call to the undefined function is sent by v8 and received by node-inspector. You should be able to see it by running node-inspector with the DEBUG='node-inspector:protocol:v8-debug' environment variable set.

When the call to the undefined function generates an exception, I was able to confirm using lldb that its "verbose" flag is set to true. So in my opinion, the issue doesn't come from the fact that some TryCatch blocks don't have the verbose flag set to true.

However, if you read the implementation of Isolate::DoThrow, it seems that the actual exception location is computed after the event is sent to the debugger:

// Notify debugger of exception.
  if (catchable_by_javascript) {
    debugger_->OnException(exception_handle, report_exception);
  }

  // Generate the message if required.
  if (report_exception || try_catch_needs_message) {
    MessageLocation potential_computed_location;
    if (location == NULL) {
      // If no location was specified we use a computed one instead.
      ComputeLocation(&potential_computed_location);
      location = &potential_computed_location;
    }

I will investigate further down this path when I have some time, but it'll be a slow process since I'm quite new to v8.

@3y3
Copy link

3y3 commented Aug 9, 2014

@misterdjules , ok, excuse me, I started Inspector with help of node-debug, therefor I missed all inspector log messages.
Now I see that the event was emitted. And this problem can be resolved in Node Inspector. For see the working version you can comment this
I'll continue discussion in node-inspector/node-inspector#344. For some reasons this will be complicated commit, I'll describe this in target issue.

This issue can be closed.

@misterdjules
Copy link

@3y3 No problem, thank you very much for helping solve this issue!

@bajtos
Copy link

bajtos commented Aug 11, 2014

@misterdjules Thank you very much for investigating this issue, your conclusions are correct. I submitted a patch for Node Inspector to fix the problem there (node-inspector/node-inspector#418).

@trevnorris You can close this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants