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

napi_create_int32 does not return napi_pending_exception #356

Closed
GiedriusM opened this issue Dec 6, 2018 · 11 comments
Closed

napi_create_int32 does not return napi_pending_exception #356

GiedriusM opened this issue Dec 6, 2018 · 11 comments

Comments

@GiedriusM
Copy link

I noticed that napi_create_int32 (and other "Functions to convert from C types to N-API") do not return napi_pending_exception when one is actually present, however napi_create_buffer (and other "Object Creation Functions") do.

Is this inconsistency intended? According to this old post it should not (napi_create_number is checked as well as napi_create_buffer). Could not find any newer information, though.

Got really confused when the code started failing in the middle or variable creation :/

@mhdawson
Copy link
Member

I believe it started as an effort to allow you to call functions which would be needed to do minimal cleanup in the face of a pending exception. This may have been extended to cases were the methods cannot throw an exception themselves.

Do you have some sample code that shows where the exception was generated but then only later you have it returned?

@GiedriusM
Copy link
Author

GiedriusM commented Dec 19, 2018

Just to be clear, I do not consider this as a bug, but more of inconsistent behavior, which was weird to debug.
I encountered this when doing node.js wrappers for a 3rd party C library. The basic call stack was like this:

node_main -> 
  wrap_process() -> 
    lib_process ->
      // if certain condition is met, call:
      wrap_callback_A() ->
        napi_call_function ->
          node_callback_A (which throws)
      
      // normally at this stage we should return to Node.js context, but
      // library does not know anything about pending exception
      // or node.js for that matter, and continues to execute
      
      // if some other condition is met, call:
      wrap_callback_B() ->
        napi_create_int32() // succeeds
        napi_create_buffer() // fails if wrap_callback_A was called and node_callback_A threw exception
        napi_call_function -> // not rea
          node_callback_B()
  • node_X represents node.js code/context.
  • wrap_X - the code I was writing
  • lib_X - 3rd party library functions

Technically, wrap_callback_B should check for pending exception and return early, but it didn't because, well, I did not expect it would be called with a pending exception. Instead I got invalid statuses somewhere in the middle of wrap_callback, when the napi_create_buffer failed.

@mhdawson
Copy link
Member

Thanks for the detail. I do see why the first call passing, but the second one not could be confusing.

I assume napi_call_function would have returned that there was a pending exception? My guess is that would have been the right place to check and return, but I'm not quite sure from what you've provided above.

@GiedriusM
Copy link
Author

Thanks for the detail. I do see why the first call passing, but the second one not could be confusing.

Exactly this.

I assume napi_call_function would have returned that there was a pending exception? My guess is that would have been the right place to check and return, but I'm not quite sure from what you've provided above.

Yes, napi_call_function (A), did return pending exception and wrap_callback_A() handled that (early return with error code). However, in my situation the 3rd party library despite/due-to the error, continues it's process loop and calls wrap_callback_B(), which could not complete due to the pending exception.
To give another shot at describing, this would more or less be the node.js side code:

const wrap = require('wrapper');

const w = new wrap.wrap();

w.on('eventA', () => throw new Error());
w.on('eventB', () => console.log('B'));

// At some point eventA and eventB are called in the same process loop
setInterval(() => w.process(), 100);

Anyway, getting back to the point. Is this discrepancy between napi_create_X functions a feature or a bug?

@mhdawson
Copy link
Member

mhdawson commented Jan 2, 2019

It is a "feature" that we have chosen that you can call some API functions even though an exception is pending. This was to allow for minimal cleanup to be done if possible. I will take a look to see if we properly document that.

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2019

I'm going to update the docs to reflect the current behavior as they don't explain the case where one is already pending when you call a method.

I won't document the specific methods yet as I'm not sure we want to commit to which ones can/can not be called.

mhdawson added a commit to mhdawson/io.js that referenced this issue Jan 4, 2019
Refs: nodejs/abi-stable-node#356

Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.
@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2019

Created nodejs/node#25339 to clarify behaviour in the docs

@GiedriusM
Copy link
Author

Alright, thanks.

Also do you have any suggestion if there's a way for a C function to force Node.js handle pending exception, without returning? Sort of recursive node.js core loop handing? In my scenario I can't control when wrap_callback_B is called (without reworking the 3rd party lib) and if there's a already a pending exception it can't continue and is forced to skip and event. I guess it's more of an application architecture issue, but still.

@mhdawson
Copy link
Member

mhdawson commented Jan 4, 2019

One thing I can think of would be to check for a pending exception and then calling
napi_fatal_error which will end up terminating the process. That is also the likely outcome of the pending exception percolating up as if you cannot resolve the exception on the native side then its unlikely the JavaScript code can either.

@GiedriusM
Copy link
Author

Alright, thank you for the help. As far as I'm concerned, this issue can be closed.

@mhdawson
Copy link
Member

mhdawson commented Jan 8, 2019

@GiedriusM thanks for your help/discussion. Closing for now.

@mhdawson mhdawson closed this as completed Jan 8, 2019
danbev pushed a commit to nodejs/node that referenced this issue Jan 9, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: #25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
addaleax pushed a commit to nodejs/node that referenced this issue Jan 14, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: #25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BridgeAR pushed a commit to BridgeAR/node that referenced this issue Jan 16, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: nodejs#25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit to nodejs/node that referenced this issue Apr 28, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: #25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
BethGriggs pushed a commit to nodejs/node that referenced this issue May 10, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: #25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue May 16, 2019
Document current behaviour where some methods can be called
when an exception is pending, while others cannot and explain
the behaviour.

PR-URL: #25339
Refs: nodejs/abi-stable-node#356
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants