From 74f5e30d692e9559ee2d7521c9feddf55f941cc7 Mon Sep 17 00:00:00 2001 From: legendecas Date: Mon, 24 May 2021 23:39:36 +0800 Subject: [PATCH] node-api: rtn pending excep on napi_new_instance When there are any JavaScript exceptions pending, `napi_pending_exception` should be returned. PR-URL: https://github.com/nodejs/node/pull/38798 Reviewed-By: Anna Henningsen Reviewed-By: Michael Dawson --- src/js_native_api_v8.cc | 2 +- test/js-native-api/test_exception/test.js | 47 +++++++++++++++++++ .../test_exception/test_exception.c | 31 ++++++++++++ 3 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index 1541f7eb36ea43..95f8212d870a72 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -2660,7 +2660,7 @@ napi_status napi_new_instance(napi_env env, auto maybe = ctor->NewInstance(context, argc, reinterpret_cast*>(const_cast(argv))); - CHECK_MAYBE_EMPTY(env, maybe, napi_generic_failure); + CHECK_MAYBE_EMPTY(env, maybe, napi_pending_exception); *result = v8impl::JsValueFromV8LocalValue(maybe.ToLocalChecked()); return GET_RETURN_STATUS(env); diff --git a/test/js-native-api/test_exception/test.js b/test/js-native-api/test_exception/test.js index ff11b702198b88..38e8fd1d6b6bdc 100644 --- a/test/js-native-api/test_exception/test.js +++ b/test/js-native-api/test_exception/test.js @@ -48,6 +48,34 @@ const test_exception = (function() { ` thrown, but ${returnedError} was passed`); } + +{ + const throwTheError = class { constructor() { throw theError; } }; + + // Test that the native side successfully captures the exception + let returnedError = test_exception.constructReturnException(throwTheError); + assert.strictEqual(returnedError, theError); + + // Test that the native side passes the exception through + assert.throws( + () => { test_exception.constructAllowException(throwTheError); }, + (err) => err === theError + ); + + // Test that the exception thrown above was marked as pending + // before it was handled on the JS side + const exception_pending = test_exception.wasPending(); + assert.strictEqual(exception_pending, true, + 'Exception not pending as expected,' + + ` .wasPending() returned ${exception_pending}`); + + // Test that the native side does not capture a non-existing exception + returnedError = test_exception.constructReturnException(common.mustCall()); + assert.strictEqual(returnedError, undefined, + 'Returned error should be undefined when no exception is' + + ` thrown, but ${returnedError} was passed`); +} + { // Test that no exception appears that was not thrown by us let caughtError; @@ -66,3 +94,22 @@ const test_exception = (function() { 'Exception state did not remain clear as expected,' + ` .wasPending() returned ${exception_pending}`); } + +{ + // Test that no exception appears that was not thrown by us + let caughtError; + try { + test_exception.constructAllowException(common.mustCall()); + } catch (anError) { + caughtError = anError; + } + assert.strictEqual(caughtError, undefined, + 'No exception originated on the native side, but' + + ` ${caughtError} was passed`); + + // Test that the exception state remains clear when no exception is thrown + const exception_pending = test_exception.wasPending(); + assert.strictEqual(exception_pending, false, + 'Exception state did not remain clear as expected,' + + ` .wasPending() returned ${exception_pending}`); +} diff --git a/test/js-native-api/test_exception/test_exception.c b/test/js-native-api/test_exception/test_exception.c index 791f5219fc61fe..844f4475ac4d4c 100644 --- a/test/js-native-api/test_exception/test_exception.c +++ b/test/js-native-api/test_exception/test_exception.c @@ -22,6 +22,22 @@ static napi_value returnException(napi_env env, napi_callback_info info) { return NULL; } +static napi_value constructReturnException(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + napi_value result; + napi_status status = napi_new_instance(env, args[0], 0, 0, &result); + if (status == napi_pending_exception) { + napi_value ex; + NODE_API_CALL(env, napi_get_and_clear_last_exception(env, &ex)); + return ex; + } + + return NULL; +} + static napi_value allowException(napi_env env, napi_callback_info info) { size_t argc = 1; napi_value args[1]; @@ -38,6 +54,19 @@ static napi_value allowException(napi_env env, napi_callback_info info) { return NULL; } +static napi_value constructAllowException(napi_env env, napi_callback_info info) { + size_t argc = 1; + napi_value args[1]; + NODE_API_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL)); + + napi_value result; + napi_new_instance(env, args[0], 0, 0, &result); + // Ignore status and check napi_is_exception_pending() instead. + + NODE_API_CALL(env, napi_is_exception_pending(env, &exceptionWasPending)); + return NULL; +} + static napi_value wasPending(napi_env env, napi_callback_info info) { napi_value result; NODE_API_CALL(env, napi_get_boolean(env, exceptionWasPending, &result)); @@ -64,6 +93,8 @@ napi_value Init(napi_env env, napi_value exports) { napi_property_descriptor descriptors[] = { DECLARE_NODE_API_PROPERTY("returnException", returnException), DECLARE_NODE_API_PROPERTY("allowException", allowException), + DECLARE_NODE_API_PROPERTY("constructReturnException", constructReturnException), + DECLARE_NODE_API_PROPERTY("constructAllowException", constructAllowException), DECLARE_NODE_API_PROPERTY("wasPending", wasPending), DECLARE_NODE_API_PROPERTY("createExternal", createExternal), };