Before this commit the EventEmitter methods were anonymous functions.
V8 tries to infer names for anonymous functions based on the execution
context but it frequently gets it wrong and when that happens, the
stack trace is usually confusing and unhelpful. This commit names all
methods so V8 can fall back to the method.name property.
The above gotcha applies to all anonymous functions but is exacerbated
for EventEmitter methods because those are invoked with a plenitude of
different receivers.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
Previously if you cached process.nextTick and then require('domain')
subsequent nextTick() calls would not be caught because enqueued
functions were taking the wrong path. This keeps nextTick to a single
function reference and changes the implementation details after domain
has been required.
Try embedding the ` ... ^` lines inside the `SyntaxError` (or any other
native error) object before giving up and printing them to the stderr.
fix#6920fix#1310
Passing a filename is still supported in place of certain options
arguments, for backward-compatibility, but timeout and display-errors
are not translated since those were undocumented.
Also managed to eliminate an extra stack trace line by not calling
through the `createScript` export.
Added a few message tests to show how `displayErrors` works.
As documented in #3042 and in [1], the existing vm implementation has
many problems. All of these are solved by @brianmcd's [contextify][2]
package. This commit uses contextify as a conceptual base and its code
core to overhaul the vm module and fix its many edge cases and caveats.
Functionally, this fixes#3042. In particular:
- A context is now indistinguishable from the object it is based on
(the "sandbox"). A context is simply a sandbox that has been marked
by the vm module, via `vm.createContext`, with special internal
information that allows scripts to be run inside of it.
- Consequently, items added to the context from anywhere are
immediately visible to all code that can access that context, both
inside and outside the virtual machine.
This commit also smooths over the API very slightly:
- Parameter defaults are now uniformly triggered via `undefined`, per
ES6 semantics and previous discussion at [3].
- Several undocumented and problematic features have been removed, e.g.
the conflation of `vm.Script` with `vm` itself, and the fact that
`Script` instances also had all static `vm` methods. The API is now
exactly as documented (although arguably the existence of the
`vm.Script` export is not yet documented, just the `Script` class
itself).
In terms of implementation, this replaces node_script.cc with
node_contextify.cc, which is derived originally from [4] (see [5]) but
has since undergone extensive modifications and iterations to expose
the most useful C++ API and use the coding conventions and utilities of
Node core.
The bindings exposed by `process.binding('contextify')`
(node_contextify.cc) replace those formerly exposed by
`process.binding('evals')` (node_script.cc). They are:
- ContextifyScript(code, [filename]), with methods:
- runInThisContext()
- runInContext(sandbox, [timeout])
- makeContext(sandbox)
From this, the vm.js file builds the entire documented vm module API.
node.js and module.js were modified to use this new native binding, or
the vm module itself where possible. This introduces an extra line or
two into the stack traces of module compilation (and thus into most
stack traces), explaining the changed tests.
The tests were also updated slightly, with all vm-related simple tests
consolidated as test/simple/test-vm-* (some of them were formerly
test/simple/test-script-*). At the same time they switched from
`common.debug` to `console.error` and were updated to use
`assert.throws` instead of rolling their own error-testing methods.
New tests were also added, of course, demonstrating the new
capabilities and fixes.
[1]: http://nodejs.org/docs/v0.10.16/api/vm.html#vm_caveats
[2]: https://github.com/brianmcd/contextify
[3]: https://github.com/joyent/node/issues/5323#issuecomment-20250726
[4]: bf123f3ef9/src/contextify.cc
[5]: https://gist.github.com/domenic/6068120
Removes the check for maxTickDepth for non-domain callbacks. So a user
can starve I/O by setting a recursive nextTick.
The domain case is more complex and will be addressed in another commit.
All compile time warnings about using deprecated APIs have been
suppressed by updating node's API. Though there are still many function
calls that can accept Isolate, and still need to be updated.
node_isolate had to be added as an extern variable in node.h and
node_object_wrap.h
Also a couple small fixes for Error handling.
Before v8 3.16.6 the error stack message was lazily written when it was
needed, which allowed you to change the message after instantiation.
Then the stack would be written with the new message the first time it
was accessed. Though that has changed. Now it creates the stack message
on instantiation. So setting a different message afterwards won't be
displayed.
This is not a complete fix for the problem. Getting error without any
message isn't very useful.
It's cleaner to only load domain ticker logic when the domains are being
used. This makes execution slightly quicker in both cases, and simpler
from the spinner since there is no need to check if the latest callback
requires use of domains.
Not necessary, since we can handle the error properly on the first tick
now, even if there are event listeners, etc.
Additionally, this removes the unnecessary "_needTickCallback" from
startup, since Module.loadMain() will kick off a nextTick callback right
after it runs the main module.
Fix#4856
* Callbacks from spinner now calls its own function, separate from the
tickCallback logic
* MakeCallback will call a domain specific function if a domain is
detected
* _tickCallback assumes no domains, until nextTick receives a callback
with a domain. After that _tickCallback is overridden with the domain
specific implementation.
* _needTickCallback runs in startup() instead of nextTick (isaacs)
* Fix bug in _fatalException where exit would be called twice (isaacs)
* Process.domain has a default value of null
* Manually track nextTickQueue.length (will be useful later)
* Update tests to reflect internal api changes
This reverts commit 0109a9f90a.
Also included: Port all the changes to process._makeCallback into the
C++ version. Immediate nextTick, etc.
This yields a slight boost in several benchmarks. V8 is optimizing and
deoptimizing process._makeCallback repeatedly.
So instead of:
node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
You will now see:
path/to/foo.js:1
throw new Error('bar');
^
This is a sub-set of isaacs patch here:
https://github.com/joyent/node/issues/3235
The difference is that this patch purely adresses the exception output,
but does not try to make any behavior changes / improvements.
This patch now reports the proper throw call site for exceptions
triggered within process.nextTick. So instead of this:
node.js:201
throw e; // process.nextTick error, or 'error' event on first tick
^
You will now see:
mydir/myscript.js:15
throw new Error('My Error');
^
From my testing this patch causes no performance regressions, but does
greatly simplify processing the nextTickQueue.
V8 3.4.12.1 changed exception log format.
3.14.10:
node.js:189
throw e; // process.nextTick error, or 'error' event on first tick
^
3.4.12.1:
node.js:189
throw e; // process.nextTick error, or 'error' event on first tick
^
The caret was moved.
Ben Noordhuis
Timothy J Fontaine
Fedor Indutny
Vladimir Kurchatkin
Domenic Denicola
Trevor Norris
isaacs
Bert Belder
Oleg Efimov
Felix Geisendörfer
Andreas Madsen
Ryan Dahl
koichik
Colton Baker
Daniel Ennis