[v14.x-backport] node-api: rtn pending excep on napi_new_instance #39516
Conversation
github-actions
bot
added
c++
legendecas
changed the title
addaleax
left a comment
addaleax
left a comment
There was a problem hiding this comment.
So … this looks good, but:
When I reviewed the original PR, the reason I approved that this was a bugfix that aligned this call site with other call sites.
However, this prompted me to take another look at the source, it seems that CHECK_MAYBE_EMPTY most often uses napi_generic_failure – which seems pretty wrong? An empty maybe by definition indicates a pending exception.
|
@addaleax previously we consider this returning status change as a possible ABI breaking change. However, the original PR does resolve a real problem, which we considered as a bug of node core. I believe it is time to re-evaluate if such a change is breaking anymore. If it is not been categorized as an ABI breaking change, we should apply the change on all other similar call sites. /cc @nodejs/node-api |
|
@legendecas Yeah, I think this is either an all-or-nothing situation.
But the same goes for the other call sites…? |
richardlau
force-pushed
the
v14.x-staging
branch
from
073941c to
0061e5d
Compare
|
Can you please rebase? |
BethGriggs
force-pushed
the
v14.x-staging
branch
from
78bb65e to
73e6781
Compare
|
ping @legendecas |
legendecas
force-pushed
the
backport-to-14/38798
branch
from
f89cf6a to
b55b2f9
Compare
|
@targos updated! :) |
MylesBorins
force-pushed
the
v14.x-staging
branch
from
b3f51ee to
327838d
Compare
When there are any JavaScript exceptions pending, `napi_pending_exception` should be returned. PR-URL: nodejs#38798 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
targos
force-pushed
the
backport-to-14/38798
branch
from
b55b2f9 to
e999999
Compare
When there are any JavaScript exceptions pending, `napi_pending_exception` should be returned. PR-URL: #38798 Backport-PR-URL: #39516 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
|
Landed in a74032a |
5 participants
When there are any JavaScript exceptions pending,
napi_pending_exceptionshould be returned.PR-URL: #38798
Reviewed-By: Anna Henningsen [email protected]
Reviewed-By: Michael Dawson [email protected]