GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — promises: more robust stringification by TimothyGu · Pull Request #13784 · nodejs/node

Conversation

@TimothyGu

@TimothyGu TimothyGu commented Jun 19, 2017

Copy link
Copy Markdown
Member

Fixes: #13771

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

promises

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bnoordhuis

bnoordhuis commented Jun 19, 2017

Copy link
Copy Markdown
Member

Swallowing exceptions seems like an - wait for it - exceptionally bad idea to me. The test case in #13771 works the way I would expect it to.
cjihrig reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mscdex mscdex added promises Issues and PRs related to ECMAScript promises. process Issues and PRs related to the process subsystem. labels Jun 19, 2017
@jasnell

jasnell commented Jun 19, 2017

Copy link
Copy Markdown
Member

I'm not too keen on this either. I'd almost prefer to go in the other direction and just make it:

const warning = new Error('Unhandled promise rejection ' +
                          `(rejection id: ${uid}): ${reason}`);
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TimothyGu

TimothyGu commented Jun 20, 2017

Copy link
Copy Markdown
Member Author

@bnoordhuis While I would generally agree with that sentiment, this case is a special one, since the thrown error is uncatchable.

@jasnell That would mean reverting #11640.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Jun 20, 2017

Copy link
Copy Markdown
Member

All the more reason to let it bubble up. If .toString() throws an exception, something is probably deeply wrong with the application. Moving on as if nothing happened is just bad.

The more I think about it, the more I'm -1.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
benjamingr
benjamingr reviewed Jun 20, 2017
View reviewed changes
Comment thread test/parallel/test-promises-unhandled-unstringifyable-rejections.js Outdated

@benjamingr benjamingr Jun 20, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I don't think we should test this part, the below is good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax and thefourtheye reacted with thumbs up emoji
All reactions
  • 👍 2 reactions
@benjamingr

benjamingr commented Jun 20, 2017

Copy link
Copy Markdown
Member

All the more reason to let it bubble up. If .toString() throws an exception, something is probably deeply wrong with the application. Moving on as if nothing happened is just bad.

The issue isn't that toString threw an exception - is that Node using your code to give you a warning invoked toString which threw an error.

If no node side promise hooks were involved - the code wouldn't throw and the server wouldn't crash. Here, adding instrumentation can crash the server which is not a great idea.
TimothyGu and addaleax reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@benjamingr

benjamingr commented Jun 20, 2017

Copy link
Copy Markdown
Member

I would prefer to have something like <toString() threw exception ${ex}> with an error instance that preserves both the original and new error.

That said, what we should do is align with browser behavior (I don't think this is good enough reason to diverge) - did anyone check what chrome's hooks do?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TimothyGu

TimothyGu commented Jun 20, 2017

Copy link
Copy Markdown
Member Author

@benjamingr

I would prefer to have something like <toString() threw exception ${ex}>

Here, you are coercing ex to a string again… and so you see the challenge.

did anyone check what chrome's hooks do?

Chrome's Dev Tools offers to inspect the object and doesn't invoke any methods to serialize it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax
addaleax approved these changes Jun 21, 2017
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM, I’m good with this change for the reasons @benjamingr mentioned

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@benjamingr

benjamingr commented Jun 21, 2017

Copy link
Copy Markdown
Member

Here, you are coercing ex to a string again… and so you see the challenge.

@TimothyGu I'm sorry if it wasn't clear from my comment, but I'm +1 on these changes and the pull request.

It is unreasonable to swallow exceptions when the user causes them but it is unacceptable to crash the process for something the user didn't do for our internal tooling
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bnoordhuis
bnoordhuis previously requested changes Jun 22, 2017
View reviewed changes

@bnoordhuis bnoordhuis left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Making my -1 explicit so it's not overlooked.

Swallowing arbitrary exceptions is tantamount to process.on('uncaughtException', () => {}), it leaves the process in an undefined state.

It's bad enough when users do it. It has no place in core.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax addaleax added the ctc-review label Jun 22, 2017
@addaleax

addaleax commented Jun 22, 2017

Copy link
Copy Markdown
Member

@nodejs/ctc
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mcollina
mcollina reviewed Jun 22, 2017
View reviewed changes

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I am -0 on this matter, as swallowing the exception does not give me the reason for that exception and that stacktrace. So it would total to 2 exceptions that should have crashed my process and didn't.

However this does make the situation somewhat better, but I'd prefer to have node 9 enforcing that deprecation, which would lead to this code being removed anyway.

In case anyone is interested, I wrote this: https://github.com/mcollina/make-promises-safe

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/internal/process/promises.js Outdated

@mcollina mcollina Jun 22, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

can you please include the message of that error?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@addaleax addaleax Jun 22, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@mcollina Keep in mind that err can be any JS value, and practically any operation we do with it can lead to yet another exception.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mcollina mcollina Jun 22, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm ok for crashing badly in that case. It's always better to crash in those situations.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@joyeecheung joyeecheung Jun 24, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If even the error thrown can not be stringified safely then IMO it's reasonable to crash with what we've got. Not including the error message makes it more confusing than what it is now if you have no idea whose toString() is throwing...

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Fishrock123 Fishrock123 Aug 14, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

What can a promise be rejected with that can't either be stringified in some way or util/inspect-ed?

I suppose if toString() throws we could either inspect the object or just <Object>.prototype.toString()?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@addaleax addaleax Aug 14, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@Fishrock123 A promise can be rejected with any JS value, and inspecting that is not foolproof.

Also, since we have [Symbol.toStringTag], Object.prototype.toString() may throw as well.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@thefourtheye thefourtheye Aug 14, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This message, as it is, will not be clear to the users. Maybe something like, "stringifying promise rejection reason failed" would be better?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
addaleax and mcollina reacted with confused emoji
All reactions
  • 😕 2 reactions

@addaleax addaleax Aug 14, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

@thefourtheye The final error message already contains the information that it’s about promise rejection reasons, so that would be duplicating a lot. Also, I think V8 reports the same literal string in similar scenarios.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@benjamingr

benjamingr commented Jun 22, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

I want to point out that this is not swallowing an error, this is Node's instrumentation causing an error and then swallowing that internal error.

If we didn't have the promise hooks in place the error would not occur in the first place. It just happens to be an error. If no exception was thrown would that help with your objection @bnoordhuis ? (We can check explicitly for what error is)

  • Check if it's an object.
  • If it's not - String it and return.
  • Check if it is an Error object.
  • If it is, assume it has a stack and print it and its stack.
  • If it's not, go over the keys and print the keys (does util.inspect do that?). This is what chrome does.

However, I think that's way more complicated than checking if it threw an exception and catching that - it's an exception we're throwing and we're catching and I'm having a really hard time coming with a realistic case where I'd prefer to crash the user's server for something they didn't do.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mcollina

mcollina commented Jun 22, 2017

Copy link
Copy Markdown
Member

However, I think that's way more complicated than checking if it threw an exception and catching that - it's an exception we're throwing and we're catching and I'm having a really hard time coming with a realistic case where I'd prefer to crash the user's server for something they didn't do.

We are not throwing that exception, a user custom toString() method is. IMHO we should not call toString() at all. util.inspect might be a good idea.

I would note that we are already swallowing an error and not crashing. It's the full point of 'unhandledRejection' and this deprecation. It's always better to crash vs swallowing/not handling an exception, as swallowing can make file descriptors and other native resources leak.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TimothyGu

TimothyGu commented Jun 22, 2017

Copy link
Copy Markdown
Member Author

util.inspect might be a good idea.

Unfortunately util.inspect may crash also because it calls property getters that may throw. Heck, even Object.prototype.toString might throw if the Symbol.toStringTag property is implemented as a getter. Unlike Dev Tools, we are simply not in a position to print anything about the object without worrying about side effects. In this light, toString is the best we can do other than not trying to do anything about the object more than printing rejection value: object, which would be a great shame as most of promise rejection values are just plain Error objects.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mcollina

mcollina commented Jun 22, 2017

Copy link
Copy Markdown
Member

I like the proposal @benjamingr did in #13784 (comment). It's complex, but I can see that topic coming up more and more often.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Jun 24, 2017

Copy link
Copy Markdown
Member

If no exception was thrown would that help with your objection

If you mean inspecting the value in a way that no user code is invoked, making it impossible for exceptions to occur - yes, that's a good approach.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Trott Trott mentioned this pull request Jun 25, 2017
Node.js Foundation Core Technical Committee (CTC) Meeting 2017-06-28 nodejs/CTC#148
Closed
@Trott Trott mentioned this pull request Jul 2, 2017
Node.js Foundation Core Technical Committee (CTC) Meeting 2017-07-05 nodejs/CTC#153
Closed
@Trott Trott mentioned this pull request Jul 16, 2017
Node.js Foundation Core Technical Committee (CTC) Meeting 2017-07-19 nodejs/CTC#154
Closed
@TimothyGu TimothyGu mentioned this pull request Jul 17, 2017
repl: handle null thrown #14306
Closed
2 of 3 tasks
@Trott Trott mentioned this pull request Jul 31, 2017
Node.js Foundation Core Technical Committee (CTC) Meeting 2017-08-02 nodejs/CTC#160
Closed
@bnoordhuis bnoordhuis removed the ctc-review label Jul 31, 2017
@bnoordhuis

bnoordhuis commented Jul 31, 2017

Copy link
Copy Markdown
Member

I'm removing the ctc-review label, please add it back if you think that is a mistake.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax added the ctc-agenda label Aug 1, 2017
@addaleax

addaleax commented Aug 1, 2017

Copy link
Copy Markdown
Member

@bnoordhuis @nodejs/ctc I do think we should come to a conclusion on this, and I do think this is the right thing to do, so I’m adding it to the CTC agenda.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@hashseed

hashseed commented Dec 19, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

git blame says it was last touched in 2015, and if memory serves me correctly we had that long before that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins added the backport-requested-v6.x label Dec 19, 2017
@MylesBorins

MylesBorins commented Dec 19, 2017

Copy link
Copy Markdown
Contributor

@TimothyGu this will need to be manually backported if we are going to land on v6.x. Please lmk if you want to do this, as an alternative we can decide not to land
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bnoordhuis

bnoordhuis commented Dec 19, 2017

Copy link
Copy Markdown
Member

does anyone know which version of v8 v8::Value::ToDetailString()was introduced in?

I think it's been in V8 since its first public release. I know for a fact it was in node.js v0.0.1!
TimothyGu reacted with laugh emoji
All reactions
  • 😄 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@TimothyGu

TimothyGu commented Dec 19, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

This should definitely go into v6.x. But, I'll have limited time and availability over the break so it would be great if someone else could take care of this.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
TimothyGu added a commit to TimothyGu/node that referenced this pull request Dec 23, 2017
@TimothyGu
promises: more robust stringification
83261d2 
PR-URL: nodejs#13784
Fixes: nodejs#13771
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@TimothyGu TimothyGu mentioned this pull request Dec 23, 2017
[backport v6.x] promises: more robust stringification #17833
Closed
3 tasks done
@TimothyGu

TimothyGu commented Dec 23, 2017

Copy link
Copy Markdown
Member Author

Backported to v6.x in #17833.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MylesBorins pushed a commit that referenced this pull request Jan 18, 2018
@TimothyGu @MylesBorins
promises: more robust stringification
3e7f0fc 
Backport-PR-URL: #17833
PR-URL: #13784
Fixes: #13771
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@MylesBorins MylesBorins added backported-to-v6.x and removed backport-requested-v6.x labels Jan 18, 2018
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
v6.13.0 proposal #18342
Merged
MylesBorins pushed a commit that referenced this pull request Feb 11, 2018
@TimothyGu @MylesBorins
promises: more robust stringification
a5ecc23 
Backport-PR-URL: #17833
PR-URL: #13784
Fixes: #13771
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Feb 11, 2018
@MylesBorins
2018-01-13, Version 6.13.0 'Boron' (LTS)
b5bef9c 
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins pushed a commit that referenced this pull request Feb 12, 2018
@TimothyGu @MylesBorins
promises: more robust stringification
2d6e802 
Backport-PR-URL: #17833
PR-URL: #13784
Fixes: #13771
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Feb 12, 2018
@MylesBorins
2018-01-13, Version 6.13.0 'Boron' (LTS)
8317be1 
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins pushed a commit that referenced this pull request Feb 13, 2018
@TimothyGu @MylesBorins
promises: more robust stringification
c64a73b 
Backport-PR-URL: #17833
PR-URL: #13784
Fixes: #13771
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Rod Vagg <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
MylesBorins added a commit that referenced this pull request Feb 13, 2018
@MylesBorins
2018-01-13, Version 6.13.0 'Boron' (LTS)
0f7dcba 
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins added a commit that referenced this pull request Feb 13, 2018
@MylesBorins
2018-01-13, Version 6.13.0 'Boron' (LTS)
9441526 
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MylesBorins added a commit that referenced this pull request Feb 13, 2018
@MylesBorins
2018-01-13, Version 6.13.0 'Boron' (LTS)
60e612d 
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    #12678
* crypto:
  - expose ECDH class (Bryan English)
    #8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    #10209
  - warn on invalid authentication tag length (Tobias Nießen)
    #17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    #16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    #7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    #13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    #13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    #16386
* net:
  - return this from getConnections() (Sam Roberts)
    #13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    #13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    #14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    #16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    #12087
  - add process.ppid (cjihrig)
    #16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    #12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    #15179
* url:
  - WHATWG URL api support (James M Snell)
    #7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    #10308

PR-URL: #18342
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@MylesBorins @MayaLekova
2018-01-13, Version 6.13.0 'Boron' (LTS)
a527e80 
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.

Notable Changes:

* console:
  - added console.count() and console.clear() (James M Snell)
    nodejs#12678
* crypto:
  - expose ECDH class (Bryan English)
    nodejs#8188
  - added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
    nodejs#10209
  - warn on invalid authentication tag length (Tobias Nießen)
    nodejs#17566
* deps:
  - upgrade libuv to 1.16.1 (cjihrig)
    nodejs#16835
* dgram:
  - added socket.setMulticastInterface() (Will Young)
    nodejs#7855
* http:
  - add agent.keepSocketAlive and agent.reuseSocket as to allow
    overridable keep-alive behavior of `Agent` (Fedor Indutny)
    nodejs#13005
* lib:
  - return this from net.Socket.end() (Sam Roberts)
    nodejs#13481
* module:
  - add builtinModules api that provides list of all builtin modules in
    Node (Jon Moss)
    nodejs#16386
* net:
  - return this from getConnections() (Sam Roberts)
    nodejs#13553
* promises:
  - more robust stringification for unhandled rejections (Timothy Gu)
    nodejs#13784
* repl:
  - improve require() autocompletion (Alexey Orlenko)
    nodejs#14409
* src:
  - add openssl-system-ca-path configure option (Daniel Bevenius)
    nodejs#16790
  - add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
    nodejs#12087
  - add process.ppid (cjihrig)
    nodejs#16839
* tls:
  - accept `lookup` option for `tls.connect()` (Fedor Indutny)
    nodejs#12839
* tools, build:
  - a new macOS installer! (JP Wesselink)
    nodejs#15179
* url:
  - WHATWG URL api support (James M Snell)
    nodejs#7448
* util:
  - add %i and %f formatting specifiers (Roman Reiss)
    nodejs#10308

PR-URL: nodejs#18342
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
@joyeecheung joyeecheung joyeecheung left review comments
@mcollina mcollina mcollina approved these changes
@jasnell jasnell jasnell approved these changes
@benjamingr benjamingr benjamingr approved these changes
@cjihrig cjihrig cjihrig approved these changes
@BridgeAR BridgeAR BridgeAR approved these changes
@bnoordhuis bnoordhuis bnoordhuis left review comments
+4 more reviewers
@thefourtheye thefourtheye thefourtheye left review comments
@Fishrock123 Fishrock123 Fishrock123 left review comments
@rvagg rvagg rvagg approved these changes
@targos targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

process Issues and PRs related to the process subsystem. promises Issues and PRs related to ECMAScript promises.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Unhandled promise rejection with throwing toString crashes node

Uh oh!

There was an error while loading. Please reload this page.

17 participants

@TimothyGu @bnoordhuis @jasnell @benjamingr @addaleax @mcollina @rvagg @Fishrock123 @joyeecheung @BridgeAR @MylesBorins @hashseed @thefourtheye @targos @cjihrig @mscdex @mhdawson
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.