Node-addon-api: Async handles and Errors

[fivdi]

Async handles can be used to "wakeup" the event loop and get a callback called from another thread. However, if the callback throws an error it doesn't appear to be possible to handle the error and have it displayed to the user.

An example of such an async handle can be seen here. This async handle is initialized to call DispatchEvent when told to do so once per second. The callback is called by DispatchEvent. If the callback throws an error what needs to be done to display the error to the user?

[mhdawson]

Thanks for the test case, this will help investigate and come up with an solution.

[mhdawson]

Believe these are related: https://github.com/nodejs/node/issues/15371

[mhdawson]

Made some progress on the plane today. Likely needs 2 PRs. One to fix up escaping of handles when the handle is undefined and then one to add napi_throw_fatal. Managed to get the test case to throw the expected error but still have work to validate what I have makes sense and then generate tests.

@fivdi thanks for the test case it was very helpful.

[mhdawson]

This will add the function needed to get the exception to come out: https://github.com/nodejs/node/pull/19337, but there is still another fix needed for the right exception to come out in this test case. I'll work on the PR for that.

[mhdawson]

PR to address escape issue that was preventing the right exception from coming out.

https://github.com/nodejs/node/pull/19434

EDIT: Turns out this was incorrect, so ignore.

[mhdawson]

Once the 2 PRs above land we can look at the PR to add a method in node-addon-api to be able to through an exception and validate the test case in this issue passes.

[mhdawson]

https://github.com/nodejs/node-addon-api/pull/245 along with the earlier PR (nodejs/node#19337).

Will let you get the correct exception if you use:

napi_fatal_exception(env, e.Value());

Next step will be to add function to throw the fatal exception with node-addon-api itself along with a test.

[mhdawson]

@fivdi I think we'll need a test case added to the node-addon-api test suite which is pretty much what you had. Do you want to contribute a PR to add it to the test suite ?

[fivdi]

Do you want to contribute a PR to add it to the test suite ?

@mhdawson I'll give it a go. When would the test be needed?

[mhdawson]

There is no fixed time when it would be needed, but sooner than later would be better so that we have test coverage. You can writ it using napi_fatal_exception and then can change that to the node-add-api method when its available.

[mhdawson]

I just committed, https://github.com/nodejs/node-addon-api/pull/245. You should now be able to build the test case using napi_fatal_exception(env, e.Value()); and it should pass with the latest node-addon-api.

[mhdawson]

@fivdi do you think you'll still get to this or should I close the issue?

[fivdi]

@mhdawson please close the issue