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

Conversation

@rvagg
Copy link
Member

@rvagg rvagg commented Oct 15, 2013

Custom errors, subclasses of Error at least, should show up as errors and not empty objects with util.format() / console.log(). The objectToString() / Object.prototype.toString() call reaches in to the instance_class_name which I'm pretty sure we can't set in JS-land to make it return [object Error] hence the switch to instanceof.

Background: errors produced by errno's custom error utility produce '{}' with console.log().

indutny and others added 30 commits August 1, 2013 16:06
And, if present and non-empty, don't invoke `resumeSession` callback.

fix #5872
Build breakage accidentally introduced in 8e29ce9 during code cleanup.

HAVE_OPENSSL is always defined (as either 0 or 1) so use #if rather
than #ifdef.

Fixes #5979.
* Numeric values passed to alloc were converted to int32, not uint32
  before the range check, which allows wrap around on ToUint32. This
  would cause massive malloc calls and v8 fatal errors.
* dispose would not check if value was an Object, causing segfault if a
  Primitive was passed.
* kMaxLength was not enumerable.
Now that values are checks in JS, no need for them to be checked in C++.
There is no need for it and it's a tiny bit slower than the version of
PersistentToLocal() that takes a v8::Isolate* as its first argument.
Commit 78d9094 updated src/*.cc to use the version of PersistentToLocal
that takes a v8::Isolate* as its first argument. This commit removes
the non-isolate version.
When doing `FreeEmpty`, `NodeBIO` skips pre-allocated `head_` buffer.
However this might lead to double-freeing buffers since in `~NodeBIO()`
we're starting deallocation from `head_` buffer.
Update a few more `Local<T>::New(isolate, persistent)` call sites to
`PersistentToLocal(isolate, persistent)` - the latter has a fast path
for non-weak persistent references.
Code cleanup, replace a few uses of reinterpret_cast<T*>(void_ptr) with
static_cast<T*>(void_ptr).
It shouldn't ignore it!

There're two possibile cases, which should be handled properly:

1. Having a default `SNICallback` which is using contexts, added with
  `server.addContext(...)` routine
2. Having a custom `SNICallback`.

In first case we may want to opt-out setting `.onsniselect` method (and
thus save some CPU time), if there're no contexts added. But, if custom
`SNICallback` is used, `.onsniselect` should always be set, because
server contexts don't affect it.
This is [1] applied ahead of time. Summary:

    OpenBSD doesn't have <ucontext.h>. ucontext_t lives in <signal.h>
    and is a typedef for struct sigcontext. There is no uc_mcontext.

[1] https://codereview.chromium.org/21705003/
https.get('https://github.com/') should use port 443, not 80.
This builds and includes the mdb_v8.so in the binary of node so mdb
can be sure to always use the latest version
Ignore encoding *if* chunk is a buffer
Share ClientHelloParser code between `tls_wrap.cc` and `node_crypto.cc`.

fix #5959
Make ClientHelloParser handle SNI extension, and extend `_tls_wrap.js`
to support loading SNI Context from both hello, and resumed session.

fix #5967
The type of the expression `(uint16_t) server_names_len + 2` gets
implicitly widened to int. Change the type of server_names_len to
uint32_t to avoid the following warnings:

    ../../src/node_crypto_clienthello.cc:144: warning: comparison
    between signed and unsigned integer expressions
    ../../src/node_crypto_clienthello.cc:146: warning: comparison
    between signed and unsigned integer expressions
The handle object was checked against the wrong constructor template.
Put another way, it was unwrapped as the wrong StreamWrap type.
Don't create a superfluous Number object, just use the version of
v8::Object::Get() that takes an unsigned int. Convert the index to
unsigned int while we're here.
Conflicts:
	deps/v8/test/cctest/test-api.cc
	lib/events.js
	lib/http.js
`server.SNICallback` was initialized with `SNICallback.bind(this)`, and
therefore check `this.SNICallback === SNICallback` was always false, and
`_tls_wrap.js` always thought that it was a custom callback instead of
default one. Which in turn was causing clienthello parser to be enabled
regardless of presence of SNI contexts.
bnoordhuis and others added 23 commits September 25, 2013 19:44
Drop the ObjectWrap dependency in favor of an internal WeakObject class.

Let's us stop worrying about API and ABI compatibility when making
changes to the way node.js deals with weakly persistent handles
internally.
I haven't actually tested this code, but was reading it due to a
post that linked to the code here:

    http://dailyjs.com/2013/09/26/libuv/

As I was reading through the code, I noticed a path that can't
be reached.

I didn't strictly follow the contributing guide:

    https://github.com/joyent/node/wiki/Contributing

but the change seems safe.

Feel free to close this out. I'm not sure if it was just an oversight
or what.
Regression introduced by commit 9ef9a9d.
Commit 204228b moved a few slow tests to pummel but I forgot to update
the require() path in pummel/test-debugger-repl-break-in-module.  Mea
culpa.
Do a binary search for the maximum RLIMIT_NOFILE.  Works around the
low, low limits on certain high, high-priced devices from Cupertino, CA.
Inform V8's CPU profiler when we're idle.  The profiler is
sampling-based but not all samples are created equal; mark the wall
clock time spent in epoll_wait() and friends so profiling tools can
filter it out.  The samples still end up in v8.log but with state=IDLE
rather than state=EXTERNAL.
The previous commit adds a notifier that tells the V8 profiler when
node.js is idle, i.e. when it's about to start sleeping in the
platform's equivalent of epoll_wait().

This commit adds a heuristic that only starts the notifier when the
V8 profiler is started from the command line.
The previous commit changes the profiler idle notifier so that it only
gets started when a --prof or --prof_lazy argument is specified on the
command line.

This commit adds two internal methods to the process object that allows
one to start and stop the idle notifier programmatically.
Mea culpa, I didn't properly resolve a merge conflict in the last two
commits.  The resulting segmentation fault only happened on Linux and
only sometimes.

Fixes #6306.
Keep track of the reference count, don't make the wrapper object weak
when there are pending write requests.  Fixes a regression from c79d516.
This change makes several improvements to the ustack helper and MDB
support:

- ustack helper and MDB: add support for two-byte strings
  (necessary to print many filenames in stacktraces in 0.10 and later).
- ustack helper: fix position numbers, which were off by a factor of two
- ustack helper: fix frames with undefined Scripts (e.g., "RegExp")
- ustack helper: add stub frames
- MDB: add support for sliced strings
- MDB: sync up with changes from the illumos version of the module

Fixes #6309
Closes #6318
AssertionError.generatedMessage is now true when
AssertionError.message was generated from expected and actual

Fixes #5836, #6206
Because it's possible for the data within a Buffer instance to be
altered after instantiation, or in case a user attempts to do something
like the following:

Buffer.prototype.fill.call({}, 10, 0, 10);

It doesn't result in a segfault.
Added a NOLINT so that cpplint won't complain about some code.
Don't emit the 'disconnect' event until all workers have gone away.
Before this commit, the event was emitted when all open handles were
closed, which usually - but not always - amounts to the same thing.

Fixes #6346.
lib/util.js Outdated
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider && ( objectToString(e) === '[object Error]' || e instanceof Error ) to continue working with errors coming from another JavaScript contexts.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call (now I remember why this is so common in the browser...), fixed.

@bnoordhuis
Copy link
Member

I'm okay with the change but can you target it at master instead of v0.10?

@rvagg
Copy link
Member Author

rvagg commented Oct 15, 2013

see #6352

@rvagg rvagg closed this Oct 15, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.