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

domain: support promises #12489

Closed
wants to merge 10 commits into from
Closed

Conversation

addaleax
Copy link
Member

@addaleax addaleax commented Apr 18, 2017

Not entirely convinced this should happen, but @misterdjules brought it up and I thought one might as well give it a try. ¯\_(ツ)_/¯ Take a look, and let me know what you think.

Aside: It seems like a V8 design bug that Isolate::SetPromiseHook()/PromiseHook() don’t take a void* data opaque, does anybody agree? I’d probably like to change that in V8 then. (Also, same question for Isolate::SetPromiseRejectCallback()… am I missing something? This can’t be intentional, right?)

/cc @nodejs/diagnostics and fyi @ORESoftware

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included (somewhat? I guess?)
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

domain

@addaleax addaleax added domain Issues and PRs related to the domain subsystem. dont-land-on-v4.x promises Issues and PRs related to ECMAScript promises. v8 engine Issues and PRs related to the V8 dependency. labels Apr 18, 2017
@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Apr 18, 2017
@addaleax addaleax added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 18, 2017
Copy link
Member

@benjamingr benjamingr left a comment

I'm not sure this should happen either - but it solves a problem for a few users.

Also pinging @caitp @littledan about the promise hooks so they can take a look.

Copy link
Member

@jasnell jasnell left a comment

Not entirely convinced on this either but the change generally looks ok.

src/node.cc Outdated
if (enter_v.As<Function>()->Call(context, domain, 0, nullptr)
.IsEmpty()) {
FatalError("node::PromiseHook",
"domain enter callback threw, please report this");
Copy link
Member

@jasnell jasnell Apr 18, 2017

Choose a reason for hiding this comment

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

report to whom? If this would be considered a Node.js core bug, then the error message should be more descriptive.

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

This is copied verbatim from MakeCallback ¯\_(ツ)_/¯ I’ve updated this to be a bit more descriptive anyway.

Copy link
Member

@jasnell jasnell Apr 18, 2017

Choose a reason for hiding this comment

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

I thought it looked familiar :-) We should likely fix it over there also :-)

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

Probably, but I’m not eager to touch that code until #11883 lands. After that I’d be up for all kinds of refactoring… aside, I don’t think there has been a bug report for this crash site in forever, so it probably doesn’t really matter. :)

src/node.cc Outdated
domain->Get(context, env->exit_string()).ToLocalChecked();
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(context, domain, 0, nullptr)
.IsEmpty()) {
Copy link
Member

@jasnell jasnell Apr 18, 2017

Choose a reason for hiding this comment

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

Minor nit, feel free to ignore but the indenting on this line is odd to me

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

It’s odd to me too, I’ll gladly take suggestions. (Maybe just splitting the statement into multiple ones is the best way…)

Copy link
Member

@gsathya gsathya Apr 18, 2017

Choose a reason for hiding this comment

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

if you restructure this to return early, then indentation becomes better.

void PromiseHook(..) {
  if (kResolve) { return; }
  if (kInit) {
    ... set value on promise...
    return;
  }

  .. code to get domain ..
  Local<Value> lookup_string;
  if (kBefore) {
    lookup_string = env->enter_string();  
  } else { 
    lookup_string = env->exit_string();
  }
  
  Local<Value> fun = domain->Get(context, lookup_string).ToLocalChecked();
  if(!fun->IsFunction()) { return; }
  if(fun.As<Function>()->Call(context, domain, 0, nullptr).IsEmpty()) {
   ....
  } 
}

@ofrobots
Copy link
Contributor

ofrobots commented Apr 18, 2017

Adding @gsathya for PromiseHooks. /cc @matthewloring

src/node.cc Outdated
if (type == PromiseHookType::kInit && env->in_domain()) {
promise->Set(context,
env->domain_string(),
env->domain_array()->Get(0)).FromJust();
Copy link
Member

@TimothyGu TimothyGu Apr 18, 2017

Choose a reason for hiding this comment

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

I feel we should hide the domain property with a private symbol...

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

Again, this is very much in line with what MakeCallback does/uses, and it makes it clear that this actually messes with the Promise object itself (I would want that to be visible). But if anybody feels strongly I can change this as well. ¯\_(ツ)_/¯

Copy link
Member

@jasnell jasnell Apr 18, 2017

Choose a reason for hiding this comment

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

I think in this case, exposing the domain would be the right thing to do for sake of consistency

src/node.cc Outdated
domain->Get(context, env->exit_string()).ToLocalChecked();
if (exit_v->IsFunction()) {
if (exit_v.As<Function>()->Call(context, domain, 0, nullptr)
.IsEmpty()) {
Copy link
Member

@gsathya gsathya Apr 18, 2017

Choose a reason for hiding this comment

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

if you restructure this to return early, then indentation becomes better.

void PromiseHook(..) {
  if (kResolve) { return; }
  if (kInit) {
    ... set value on promise...
    return;
  }

  .. code to get domain ..
  Local<Value> lookup_string;
  if (kBefore) {
    lookup_string = env->enter_string();  
  } else { 
    lookup_string = env->exit_string();
  }
  
  Local<Value> fun = domain->Get(context, lookup_string).ToLocalChecked();
  if(!fun->IsFunction()) { return; }
  if(fun.As<Function>()->Call(context, domain, 0, nullptr).IsEmpty()) {
   ....
  } 
}

src/node.cc Outdated
Environment* env = Environment::GetCurrent(Isolate::GetCurrent());
Local<Context> context = env->context();

if (type == PromiseHookType::kInit && env->in_domain()) {
Copy link
Member

@gsathya gsathya Apr 18, 2017

Choose a reason for hiding this comment

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

Again, I don't have much context, but in_domain() check seems important, are you sure you don't need to do it for the other cases?

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

I’m pretty sure, yes. This is based on existing code for similar circumstances (plain JS calls coming from the void), and in_domain() will automatically be true between the enter and exit callbacks.

src/node.cc Outdated
@@ -1110,6 +1111,57 @@ bool ShouldAbortOnUncaughtException(Isolate* isolate) {
}


void PromiseHook(PromiseHookType type,
Local<Promise> promise,
Local<Value> parent) {
Copy link
Member

@gsathya gsathya Apr 18, 2017

Choose a reason for hiding this comment

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

Don't have much context about domains but, AFAICS, the promise nor the parent is passed to the domain callback. Is that ok?

Copy link
Member Author

@addaleax addaleax Apr 18, 2017

Choose a reason for hiding this comment

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

The domain enter/exit callbacks don’t do much, they basically just push or pop an object to an array.

@gsathya
Copy link
Member

gsathya commented Apr 18, 2017

Aside: It seems like a V8 design bug that Isolate::SetPromiseHook()/PromiseHook() don’t take a void* data opaque, does anybody agree? I’d probably like to change that in V8 then. (Also, same question for Isolate::SetPromiseRejectCallback()… am I missing something? This can’t be intentional, right?)

I didn't see the need for this when designing the API. Please file a V8 bug with a use case. I can take a look.

@matthewloring
Copy link

matthewloring commented Apr 18, 2017

It would be good to coordinate this work with the async hooks work here: #11883. Node can only register one set of promise hook callbacks (by design) so the callbacks will have to handle emitting the right async hooks lifecycle events in addition to propagating domains (if desired).

I have been prototyping promise hook integration for async hooks on top of #11883. I have experimented with refactoring AsyncWrap::MakeCallback into "before callback invocation" and "after callback invocation" helpers since they already have the correct domain propagation and emit the correct async hook events. Once refactored, these functions could then be called from the kBefore and kAfter promise hook callbacks. This approach won't be able to use the existing AsyncHooks::ExecScope mechanism but I think we can work around this. What do you think about this design?

@addaleax
Copy link
Member Author

addaleax commented Apr 18, 2017

I didn't see the need for this when designing the API. Please file a V8 bug with a use case. I can take a look.

Well, it forces consumers to use global state, or Isolate::GetCurrent() or similar to access any interesting data, which I would consider an anti-pattern (and, as I have recently learned, Isolate::GetCurrent() is something V8 would generally like to migrate away from), which is why it’s good practice to provide an opaque pointer to plain callbacks in C++ for those reasons. I’m happy to submit a V8 patch myself.

What do you think about this design?

@matthewloring Sounds good, I’d be curious to see what you have come up with.

@gsathya
Copy link
Member

gsathya commented Apr 18, 2017

Well, it forces consumers to use global state, or Isolate::GetCurrent() or similar to access any interesting data, which I would consider an anti-pattern (and, as I have recently learned, Isolate::GetCurrent() is something V8 would generally like to migrate away from), which is why it’s good practice to provide an opaque pointer to plain callbacks in C++ for those reasons.

There's only one possible promisehook that can be used so global state isn't the worst, but the embedder is free to create better encapsulation if required. Also, you can get the isolate from the promise object directly, no need for Isolate::GetCurrent(). Anyways, this is better discussed in a V8 bug.

Copy link
Member

@bnoordhuis bnoordhuis left a comment

LGTM modulo comments.

src/node.cc Outdated
void PromiseHook(PromiseHookType type,
Local<Promise> promise,
Local<Value> parent) {
Environment* env = Environment::GetCurrent(Isolate::GetCurrent());
Copy link
Member

@bnoordhuis bnoordhuis Apr 18, 2017

Choose a reason for hiding this comment

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

I think it should be safe (and a little more efficient) to call Environment::GetCurrent(promise->CreationContext()).

(It must be safe because some of the operations in this function will error without a HandleScope. I think the promise hook implicitly runs inside one.)

src/node.cc Outdated
if (type == PromiseHookType::kInit && env->in_domain()) {
promise->Set(context,
env->domain_string(),
env->domain_array()->Get(0)).FromJust();
Copy link
Member

@bnoordhuis bnoordhuis Apr 18, 2017

Choose a reason for hiding this comment

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

Get(context, 0).ToLocalChecked()

Promise.resolve().then(common.mustCall(() => {
assert.strictEqual(process.domain, d);
}));
}));
Copy link
Member

@bnoordhuis bnoordhuis Apr 18, 2017

Choose a reason for hiding this comment

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

It would be good to check that domains + promises + vm.runInNewContext() also does the right thing.

@chrisdickinson
Copy link
Contributor

chrisdickinson commented Apr 19, 2017

FWIW, I'm generally in favor of this change. Some (including myself!) have built continuation-local-storage-on-the-cheap on top of domains, & end up having to strictly enforce domain-compliant promise library use. This gives us a good road forward to move onto native promises, and from there onto asynchooks.

@benjamingr benjamingr mentioned this pull request Apr 19, 2017
4 tasks
@addaleax
Copy link
Member Author

addaleax commented Apr 19, 2017

@bnoordhuis I should have addressed your comments

CI: https://ci.nodejs.org/job/node-test-commit/9232/

@gsathya
Copy link
Member

gsathya commented Apr 27, 2017

Would it make sense/be possible to call the kResolve callback when the promise actually gets resolve?

Not sure I follow. Can you point to the exact step in the spec?

If not, I’m not sure how useful kResolve is given that it doesn’t provide much information about what’s actually happening with the promise (or am I mistaken)?

The only real use of the resolve hook is to provide information about who is triggering the promise resolving functions (to stitch together better stack traces).

var res;
new Promise(r => res = r ).then(console.log)
function foo() { res(1); }
foo();

with the resolve hook, you can figure out that foo() triggered the promise resolving function.

@addaleax
Copy link
Member Author

addaleax commented Apr 27, 2017

@gsathya My intuition would be that kResolve would make more sense as acting like an extra step after [[PromiseState]] is set to "fulfilled" or "rejected" in FulfillPromise and RejectPromise, respectively (rather than at the beginning of those functions, if I understand the V8 header comment correctly).

That should give the same stack traces etc., but carrying the additional information of the actual new Promise state.

@gsathya
Copy link
Member

gsathya commented Apr 27, 2017

That should give the same stack traces etc., but carrying the additional information of the actual new Promise state.

var res;
var x = new Promise(r => res = r);
function foo() { res(new Promise(r => setTimeout(r))); };
foo();

x won't be resolved until later by which time we lose the stack.

@MylesBorins
Copy link
Member

MylesBorins commented May 15, 2017

@addaleax would you consider backporting dca0815 to v6.x

addaleax added a commit to addaleax/node that referenced this pull request May 18, 2017
Ref: nodejs#12442
PR-URL: nodejs#12489
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax added a commit to addaleax/node that referenced this pull request May 18, 2017
Ref: nodejs#12442
PR-URL: nodejs#12489
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request May 11, 2017
jasnell added a commit to jasnell/node that referenced this pull request May 29, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](nodejs@4a7233c178)]
    [nodejs#12892](nodejs#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](nodejs@d2d32ea5a2)]
    [nodejs#11968](nodejs#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](nodejs@7eb1b4658e)]
    [nodejs#12141](nodejs#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](nodejs@beca3244e2)]
    [nodejs#10236](nodejs#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](nodejs@97a77288ce)]
    [nodejs#12348](nodejs#12348),
    [[`d75fdd96aa`](nodejs@d75fdd96aa)]
    [nodejs#10423](nodejs#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](nodejs@627ecee9ed)]
    [nodejs#10653](nodejs#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](nodejs@f18e08d820)]
    [nodejs#9744](nodejs#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](nodejs@3c3b36af0f)]
    [nodejs#12936](nodejs#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](nodejs@60d1aac8d2)]
    [nodejs#12784](nodejs#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](nodejs@84dabe8373)]
    [nodejs#12489](nodejs#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](nodejs@7a55e34ef4)]
    [nodejs#10467](nodejs#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](nodejs@3c2a9361ff)]
    [nodejs#9683](nodejs#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](nodejs@90403dd1d0)]
    [nodejs#11567](nodejs#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](nodejs@d3480776c7)]
    [nodejs#11259](nodejs#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](nodejs@fb71ba4921)]
    [nodejs#11355](nodejs#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](nodejs@3e6f1032a4)]
    [nodejs#10805](nodejs#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](nodejs@5de3cf099c)]
    [nodejs#10116](nodejs#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](nodejs@84a23391f6)]
    [nodejs#12113](nodejs#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](nodejs@56e881d0b0)]
    [nodejs#11975](nodejs#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](nodejs@03e89b3ff2)]
    [nodejs#10116](nodejs#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](nodejs@dd20e68b0f)]
    [nodejs#12725](nodejs#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](nodejs@3f27f02da0)]
    [nodejs#11599](nodejs#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (nodejs@ec7cbaf266)]
    [nodejs#12995](nodejs#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](nodejs@a16b570f8c)]
    [nodejs#11968](nodejs#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](nodejs@010f864426)]
    [nodejs#12949](nodejs#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](nodejs@a5f91ab230)]
    [nodejs#11689](nodejs#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](nodejs@8a7db9d4b5)]
    [nodejs#12087](nodejs#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](nodejs@b6e1d22fa6)]
    [nodejs#12925](nodejs#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](nodejs@07c7f198db)]
    [nodejs#12828](nodejs#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](nodejs@348cc80a3c)]
    [nodejs#5923](nodejs#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](nodejs@a2ae08999b)]
    [nodejs#11349](nodejs#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](nodejs@d523eb9c40)]
    [nodejs#11447](nodejs#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](nodejs@d080ead0f9)]
    [nodejs#12710](nodejs#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](nodejs@5bfd13b81e)]
    [nodejs#9726](nodejs#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](nodejs@455e6f1dd8)]
    [nodejs#11708](nodejs#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](nodejs@aab0d202f8)]
    [nodejs#11624](nodejs#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](nodejs@99da8e8e02)]
    [nodejs#12442](nodejs#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](nodejs@91383e47fd)]
    [nodejs#12001](nodejs#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](nodejs@b514bd231e)]
    [nodejs#11391](nodejs#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](4a7233c178)]
    [#12892](#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](d2d32ea5a2)]
    [#11968](#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](7eb1b4658e)]
    [#12141](#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](beca3244e2)]
    [#10236](#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](97a77288ce)]
    [#12348](#12348),
    [[`d75fdd96aa`](d75fdd96aa)]
    [#10423](#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](627ecee9ed)]
    [#10653](#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](f18e08d820)]
    [#9744](#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](3c3b36af0f)]
    [#12936](#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](60d1aac8d2)]
    [#12784](#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](84dabe8373)]
    [#12489](#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](7a55e34ef4)]
    [#10467](#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](3c2a9361ff)]
    [#9683](#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](90403dd1d0)]
    [#11567](#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](d3480776c7)]
    [#11259](#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](fb71ba4921)]
    [#11355](#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](3e6f1032a4)]
    [#10805](#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](5de3cf099c)]
    [#10116](#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](84a23391f6)]
    [#12113](#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](56e881d0b0)]
    [#11975](#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](03e89b3ff2)]
    [#10116](#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](dd20e68b0f)]
    [#12725](#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](3f27f02da0)]
    [#11599](#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (ec7cbaf266)]
    [#12995](#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](a16b570f8c)]
    [#11968](#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](010f864426)]
    [#12949](#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](a5f91ab230)]
    [#11689](#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](8a7db9d4b5)]
    [#12087](#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](b6e1d22fa6)]
    [#12925](#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](07c7f198db)]
    [#12828](#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](348cc80a3c)]
    [#5923](#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](a2ae08999b)]
    [#11349](#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](d523eb9c40)]
    [#11447](#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](d080ead0f9)]
    [#12710](#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](5bfd13b81e)]
    [#9726](#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](455e6f1dd8)]
    [#11708](#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](aab0d202f8)]
    [#11624](#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](99da8e8e02)]
    [#12442](#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](91383e47fd)]
    [#12001](#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](b514bd231e)]
    [#11391](#11391).
jasnell added a commit that referenced this pull request May 30, 2017
* **Async Hooks**
  * The `async_hooks` module has landed in core
    [[`4a7233c178`](4a7233c178)]
    [#12892](#12892).

* **Buffer**
  * Using the `--pending-deprecation` flag will cause Node.js to emit a
    deprecation warning when using `new Buffer(num)` or `Buffer(num)`.
    [[`d2d32ea5a2`](d2d32ea5a2)]
    [#11968](#11968).
  * `new Buffer(num)` and `Buffer(num)` will zero-fill new `Buffer` instances
    [[`7eb1b4658e`](7eb1b4658e)]
    [#12141](#12141).
  * Many `Buffer` methods now accept `Uint8Array` as input
    [[`beca3244e2`](beca3244e2)]
    [#10236](#10236).

* **Child Process**
  * Argument and kill signal validations have been improved
    [[`97a77288ce`](97a77288ce)]
    [#12348](#12348),
    [[`d75fdd96aa`](d75fdd96aa)]
    [#10423](#10423).
  * Child Process methods accept `Uint8Array` as input
    [[`627ecee9ed`](627ecee9ed)]
    [#10653](#10653).

* **Console**
  * Error events emitted when using `console` methods are now supressed.
    [[`f18e08d820`](f18e08d820)]
    [#9744](#9744).

* **Dependencies**
  * The npm client has been updated to 5.0.0
    [[`3c3b36af0f`](3c3b36af0f)]
    [#12936](#12936).
  * V8 has been updated to 5.8 with forward ABI stability to 6.0
    [[`60d1aac8d2`](60d1aac8d2)]
    [#12784](#12784).

* **Domains**
  * Native `Promise` instances are now `Domain` aware
    [[`84dabe8373`](84dabe8373)]
    [#12489](#12489).

* **Errors**
  * We have started assigning static error codes to errors generated by Node.js.
    This has been done through multiple commits and is still a work in
    progress.

* **File System**
  * The utility class `fs.SyncWriteStream` has been deprecated
    [[`7a55e34ef4`](7a55e34ef4)]
    [#10467](#10467).
  * The deprecated `fs.read()` string interface has been removed
    [[`3c2a9361ff`](3c2a9361ff)]
    [#9683](#9683).

* **HTTP**
  * Improved support for userland implemented Agents
    [[`90403dd1d0`](90403dd1d0)]
    [#11567](#11567).
  * Outgoing Cookie headers are concatenated into a single string
    [[`d3480776c7`](d3480776c7)]
    [#11259](#11259).
  * The `httpResponse.writeHeader()` method has been deprecated
    [[`fb71ba4921`](fb71ba4921)]
    [#11355](#11355).
  * New methods for accessing HTTP headers have been added to `OutgoingMessage`
    [[`3e6f1032a4`](3e6f1032a4)]
    [#10805](#10805).

* **Lib**
  * All deprecation messages have been assigned static identifiers
    [[`5de3cf099c`](5de3cf099c)]
    [#10116](#10116).
  * The legacy `linkedlist` module has been removed
    [[`84a23391f6`](84a23391f6)]
    [#12113](#12113).

* **N-API**
  * Experimental support for the new N-API API has been added
    [[`56e881d0b0`](56e881d0b0)]
    [#11975](#11975).

* **Process**
  * Process warning output can be redirected to a file using the
    `--redirect-warnings` command-line argument
    [[`03e89b3ff2`](03e89b3ff2)]
    [#10116](#10116).
  * Process warnings may now include additional detail
    [[`dd20e68b0f`](dd20e68b0f)]
    [#12725](#12725).

* **REPL**
  * REPL magic mode has been deprecated
    [[`3f27f02da0`](3f27f02da0)]
    [#11599](#11599).

* **Src**
  * `NODE_MODULE_VERSION` has been updated to 57
    (ec7cbaf266)]
    [#12995](#12995).
  * Add `--pending-deprecation` command-line argument and
    `NODE_PENDING_DEPRECATION` environment variable
    [[`a16b570f8c`](a16b570f8c)]
    [#11968](#11968).
  * The `--debug` command-line argument has been deprecated. Note that
    using `--debug` will enable the *new* Inspector-based debug protocol
    as the legacy Debugger protocol previously used by Node.js has been
    removed. [[`010f864426`](010f864426)]
    [#12949](#12949).
  * Throw when the `-c` and `-e` command-line arguments are used at the same
    time [[`a5f91ab230`](a5f91ab230)]
    [#11689](#11689).
  * Throw when the `--use-bundled-ca` and `--use-openssl-ca` command-line
    arguments are used at the same time.
    [[`8a7db9d4b5`](8a7db9d4b5)]
    [#12087](#12087).

* **Stream**
  * `Stream` now supports `destroy()` and `_destroy()` APIs
    [[`b6e1d22fa6`](b6e1d22fa6)]
    [#12925](#12925).
  * `Stream` now supports the `_final()` API
    [[`07c7f198db`](07c7f198db)]
    [#12828](#12828).

* **TLS**
  * The `rejectUnauthorized` option now defaults to `true`
    [[`348cc80a3c`](348cc80a3c)]
    [#5923](#5923).
  * The `tls.createSecurePair()` API now emits a runtime deprecation
    [[`a2ae08999b`](a2ae08999b)]
    [#11349](#11349).
  * A runtime deprecation will now be emitted when `dhparam` is less than
    2048 bits [[`d523eb9c40`](d523eb9c40)]
    [#11447](#11447).

* **URL**
  * The WHATWG URL implementation is now a fully-supported Node.js API
    [[`d080ead0f9`](d080ead0f9)]
    [#12710](#12710).

* **Util**
  * `Symbol` keys are now displayed by default when using `util.inspect()`
    [[`5bfd13b81e`](5bfd13b81e)]
    [#9726](#9726).
  * `toJSON` errors will be thrown when formatting `%j`
    [[`455e6f1dd8`](455e6f1dd8)]
    [#11708](#11708).
  * Convert `inspect.styles` and `inspect.colors` to prototype-less objects
    [[`aab0d202f8`](aab0d202f8)]
    [#11624](#11624).
  * The new `util.promisify()` API has been added
    [[`99da8e8e02`](99da8e8e02)]
    [#12442](#12442).

* **Zlib**
  * Support `Uint8Array` in Zlib convenience methods
    [[`91383e47fd`](91383e47fd)]
    [#12001](#12001).
  * Zlib errors now use `RangeError` and `TypeError` consistently
    [[`b514bd231e`](b514bd231e)]
    [#11391](#11391).
gibfahn pushed a commit to gibfahn/node that referenced this pull request Jun 17, 2017
Ref: nodejs#12442
PR-URL: nodejs#12489
Backport-PR-URL: nodejs#13103
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
gibfahn pushed a commit that referenced this pull request Jun 20, 2017
Ref: #12442
PR-URL: #12489
Backport-PR-URL: #13103
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jul 11, 2017
Ref: #12442
PR-URL: #12489
Backport-PR-URL: #13103
Reviewed-By: Matthew Loring <mattloring@google.com>
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jul 18, 2017
@ORESoftware
Copy link
Contributor

ORESoftware commented Sep 20, 2017

Hey all, I am wondering if anyone on this thread can help answer this question regarding Promises + Domains, thanks

nodejs/help#831

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. domain Issues and PRs related to the domain subsystem. notable-change PRs with changes that should be highlighted in changelogs. promises Issues and PRs related to ECMAScript promises. semver-minor PRs that contain new features and should be released in the next minor version. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet