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

The default CallJS method of ThreadSafeFunction ignores user exceptions #41377

Closed
mmomtchev opened this issue Jan 2, 2022 · 4 comments
Closed
Labels
node-api Issues and PRs related to the Node-API.

Comments

@mmomtchev
Copy link
Contributor

Version

All

Platform

All

Subsystem

node-api

What steps will reproduce the bug?

Throw a JS exception from an async callback called through ThreadSafeFunction as described here: nodejs/node-addon-api#669

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

Node exits cleanly reporting an unhandled exception: nodejs/node-addon-api#1119

What do you see instead?

Node continues as if nothing happened

Additional information

The exceptions are ignored here:

if (status != napi_ok && status != napi_pending_exception) {

@Trott Trott added the node-api Issues and PRs related to the Node-API. label Jan 2, 2022
@mhdawson
Copy link
Member

@mmomtchev I think this is related to #36510 and we are working on landing a fix.

@mhdawson
Copy link
Member

mhdawson commented Jun 3, 2022

@mmomtchev this should now be resolved since #36510 landed. Can you please check and let us know?

@mmomtchev
Copy link
Contributor Author

No, the unit test doesn't use the builtin CallJS which is used only when a ThreadSafeFunction is created without a custom callback - which is possible only by directly using Node-API without node-addon-api. CallJS hasn't changed and the unit test from the PR uses napi_call_function without creating a ThreadSafeFunction.

But it does implement everything needed - the only thing missing is to attach the new code in node_napi_env__::CallbackIntoModule to the builtin CallJS in node_api.cc.

@mmomtchev
Copy link
Contributor Author

In fact works, the exception being handled at

if (!last_exception.IsEmpty()) {

(I am sorry but a Debug build takes 4 hours on my machine + 10 minutes to load it in gdb)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
node-api Issues and PRs related to the Node-API.
Projects
None yet
Development

No branches or pull requests

3 participants