util: use V8 C++ API for inspecting Promises#12254
Conversation
nodejs-github-bot
added
c++
lib/util.js
Outdated
There was a problem hiding this comment.
Also fyi @Fishrock123, this seems like a cleaner way to inspect Promise states than what the abort-on-rejected PR currently does…
There was a problem hiding this comment.
Errr, this allows you to get the actual result object e.g. an error without going through the Debug API? I was told that was not possible to do synchronously.
There was a problem hiding this comment.
@Fishrock123 this set of APIs is relatively new, having been added in V8 5.7 in v8/v8@2843258. That might be why.
joshgav
left a comment
joshgav
left a comment
There was a problem hiding this comment.
LGTM, but perhaps returning an object instead of an array would be better.
src/node_util.cc
Outdated
There was a problem hiding this comment.
might it be better to use Object and name the properties? might be easier if we want to extend with additional metadata, and would allow the JavaScript to be de-coupled from the order of items in the array.
There was a problem hiding this comment.
Besides the fact that this is simpler to implement, it is also the approach taken by the existing GetProxyDetails. Plus, I don't think it is very likely for promises to change its internal properties anytime soon.
PR-URL: nodejs#12254 Refs: nodejs#11875 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
TimothyGu
force-pushed
the
promise-inspect
branch
from
fa77241 to
a37273c
Compare
|
Landed in a37273c. |
|
cc @TimothyGu |
|
This depends on V8 5.7 iiuc, so I’m adding the dont-land labels here |
10 participants
Avoid using the deprecated debug mirror API.
Refs: #11875
Refs: #12243
CI: https://ci.nodejs.org/job/node-test-pull-request/7244/
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
url