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

Exceptions in N-API Init are not reported during require() call #19437

Closed
fluggo opened this issue Mar 19, 2018 · 3 comments
Closed

Exceptions in N-API Init are not reported during require() call #19437

fluggo opened this issue Mar 19, 2018 · 3 comments
Assignees

Comments

@fluggo
Copy link

fluggo commented Mar 19, 2018

When using N-API, if an exception is thrown during the module's Init, it is not reported during the require() call and might not be reported at all.

If you modify the N-API hello world sample code to throw an error during Init:

 napi_value Init(napi_env env, napi_value exports) {
   napi_status status;
   napi_property_descriptor desc = DECLARE_NAPI_METHOD("hello", Method);
   status = napi_define_properties(env, exports, 1, &desc);
   assert(status == napi_ok);
-  return exports;
+  napi_throw_error(env, "CODE", "Test error");
+  printf("Ran error code\n");
+  return nullptr;
 }

...the error itself is not thrown until the hello() method call:

root@a5d5c8becf83:/app/pure-napi# node hello.js
Ran error code
/app/pure-napi/hello.js:3
console.log(addon.hello()); // 'world'
                  ^

Error [CODE]: Test error
    at Object.Module._extensions..node (module.js:678:18)
    at Module.load (module.js:561:32)
    at tryModuleLoad (module.js:501:12)
    at Function.Module._load (module.js:493:3)
    at Module.require (module.js:593:17)
    at require (internal/module.js:11:18)
    at bindings (/app/pure-napi/node_modules/bindings/bindings.js:76:44)
    at Object.<anonymous> (/app/pure-napi/hello.js:1:94)
    at Module._compile (module.js:649:30)
    at Object.Module._extensions..js (module.js:660:10)

Further, if the method is not called, the error isn't reported at all.

@fluggo
Copy link
Author

fluggo commented Mar 20, 2018

Verified that it does work when not using N-API. In a pure V8-only addon:

void Init(Handle<Object> exports) {
  Isolate* isolate = Isolate::GetCurrent();
  exports->Set(String::NewFromUtf8(isolate, "hello"),
      FunctionTemplate::New(isolate, Method)->GetFunction());
  isolate->ThrowException(String::NewFromUtf8(isolate, "Test error"));
}

I get:

root@a5d5c8becf83:/app/pure-node# node hello.js

module.js:678
  return process.dlopen(module, path.toNamespacedPath(filename));
                 ^
Test error

@richardlau
Copy link
Member

cc @nodejs/n-api

@gabrielschulhof
Copy link
Contributor

There are actually several places where we should check for exceptions resulting from calls into the N-API addon.

gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Mar 23, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: nodejs#19437
targos pushed a commit that referenced this issue Apr 2, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: #19437
PR-URL: #19537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 12, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: nodejs#19437
PR-URL: nodejs#19537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
gabrielschulhof pushed a commit to gabrielschulhof/node that referenced this issue Apr 16, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: nodejs#19437
PR-URL: nodejs#19537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue Apr 16, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: #19437
Backport-PR-URL: #19447
PR-URL: #19537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
MylesBorins pushed a commit that referenced this issue May 1, 2018
Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: #19437
Backport-PR-URL: #19265
PR-URL: #19537
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
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

Successfully merging a pull request may close this issue.

3 participants