Skip to content

Commit 0901dc0

Browse files
ammarahm-edammarahm-ed
committed
feat: improve error handling in the runtime
1 parent 3c3012c commit 0901dc0

23 files changed

Lines changed: 1587 additions & 762 deletions

‎test-app/runtime/src/main/cpp/napi/common/native_api_util.h‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
#include <dlfcn.h>
66
#include <sstream>
77

8+
#ifdef __ANDROID__
9+
#include <android/log.h>
10+
#endif
11+
812
#ifndef NAPI_PREAMBLE
913
#define NAPI_PREAMBLE napi_status status;
1014
#endif
@@ -80,28 +84,30 @@
8084
NAPI_ERROR_INFO \
8185
napi_throw_error(env, NULL, error_info->error_message);
8286

83-
#ifndef DEBUG
87+
#ifdef __ANDROID__
88+
#define NAPI_LOG_ERROR(status_val, expr_str) \
89+
__android_log_print(ANDROID_LOG_ERROR, "TNS.Native", \
90+
"Node-API returned error: %d\n %s\n ^\n at %s:%d", \
91+
(int)(status_val), (expr_str), __FILE__, __LINE__)
92+
#else
93+
#define NAPI_LOG_ERROR(status_val, expr_str) ((void)0)
94+
#endif
8495

96+
// NAPI_GUARD(expr) { ...on-error block... }
97+
// Assigns the result of `expr` to the in-scope `status`, logs a diagnostic
98+
// (status, expression, file:line) on failure so an invalid runtime state is
99+
// traceable, and runs the trailing block when the call did not return napi_ok.
100+
// napi_pending_exception is JS-level control flow (a callback threw), not an
101+
// invalid runtime state, so it is intentionally not logged — the caller's
102+
// exception handling deals with it.
85103
#define NAPI_GUARD(expr) \
86104
status = expr; \
87-
if (status != napi_ok) \
105+
if (status != napi_ok && status != napi_pending_exception) \
88106
{ \
89-
NAPI_ERROR_INFO \
90-
std::stringstream msg; \
91-
msg << "Node-API returned error: " << status << "\n " << #expr \
92-
<< "\n ^\n " \
93-
<< "at " << __FILE__ << ":" << __LINE__ << ""; \
107+
NAPI_LOG_ERROR(status, #expr); \
94108
} \
95109
if (status != napi_ok)
96110

97-
#else
98-
99-
#define NAPI_GUARD(expr) \
100-
status = expr; \
101-
if (status != napi_ok)
102-
103-
#endif
104-
105111
#define NAPI_FUNCTION(name) \
106112
napi_value JS_##name(napi_env env, napi_callback_info cbinfo)
107113

‎test-app/runtime/src/main/cpp/runtime/Runtime.cpp‎

Lines changed: 51 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -68,28 +68,31 @@ namespace {
6868
// microtask on every engine the napi runtime targets (V8, QuickJS, Hermes,
6969
// JSC). This preserves ordering with Promise microtasks and runs before timers.
7070
napi_value QueueMicrotaskCallback(napi_env env, napi_callback_info info) {
71+
napi_status status;
7172
size_t argc = 1;
7273
napi_value argv[1];
73-
napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr);
74+
NAPI_GUARD(napi_get_cb_info(env, info, &argc, argv, nullptr, nullptr)) {
75+
return nullptr;
76+
}
7477

7578
napi_valuetype type = napi_undefined;
7679
if (argc >= 1) {
77-
napi_typeof(env, argv[0], &type);
80+
NAPI_GUARD(napi_typeof(env, argv[0], &type)) {}
7881
}
7982
if (argc < 1 || type != napi_function) {
80-
napi_throw_type_error(env, nullptr,
81-
"queueMicrotask: callback must be a function");
83+
NAPI_GUARD(napi_throw_type_error(env, nullptr,
84+
"queueMicrotask: callback must be a function")) {}
8285
return nullptr;
8386
}
8487

8588
napi_value global, promiseCtor, resolveFn, resolved, thenFn;
86-
napi_get_global(env, &global);
87-
napi_get_named_property(env, global, "Promise", &promiseCtor);
88-
napi_get_named_property(env, promiseCtor, "resolve", &resolveFn);
89-
napi_call_function(env, promiseCtor, resolveFn, 0, nullptr, &resolved);
90-
napi_get_named_property(env, resolved, "then", &thenFn);
89+
NAPI_GUARD(napi_get_global(env, &global)) { return nullptr; }
90+
NAPI_GUARD(napi_get_named_property(env, global, "Promise", &promiseCtor)) { return nullptr; }
91+
NAPI_GUARD(napi_get_named_property(env, promiseCtor, "resolve", &resolveFn)) { return nullptr; }
92+
NAPI_GUARD(napi_call_function(env, promiseCtor, resolveFn, 0, nullptr, &resolved)) { return nullptr; }
93+
NAPI_GUARD(napi_get_named_property(env, resolved, "then", &thenFn)) { return nullptr; }
9194
napi_value thenArgs[1] = {argv[0]};
92-
napi_call_function(env, resolved, thenFn, 1, thenArgs, nullptr);
95+
NAPI_GUARD(napi_call_function(env, resolved, thenFn, 1, thenArgs, nullptr)) {}
9396

9497
return nullptr;
9598
}
@@ -188,8 +191,9 @@ Runtime::Init(JNIEnv *_env, jobject obj, int runtimeId, jstring filesPath, jstri
188191
}
189192

190193
napi_value Runtime::GlobalAccessorCallback(napi_env env, napi_callback_info info) {
194+
napi_status status;
191195
napi_value global;
192-
napi_get_global(env, &global);
196+
NAPI_GUARD(napi_get_global(env, &global)) { return nullptr; }
193197
return global;
194198
}
195199

@@ -222,15 +226,16 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
222226
v8::Isolate::Scope isolate_scope(env->isolate);
223227
v8::Context::Scope context_scope(env->context());
224228
#endif
225-
napi_open_handle_scope(env, &global_scope);
229+
napi_status status;
230+
NAPI_GUARD(napi_open_handle_scope(env, &global_scope)) {}
226231

227232
napi_handle_scope handleScope;
228-
napi_open_handle_scope(env, &handleScope);
233+
NAPI_GUARD(napi_open_handle_scope(env, &handleScope)) {}
229234

230235
env_to_runtime_cache.Insert(env, this);
231236

232237
napi_value global;
233-
napi_get_global(env, &global);
238+
NAPI_GUARD(napi_get_global(env, &global)) {}
234239

235240
// Newer JSC ships a native `WeakRef` global, so the old polyfill (which was
236241
// actually a strong reference and leaked) is no longer needed.
@@ -255,12 +260,12 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
255260
napi_util::napi_set_function(env, global, "__exit", CallbackHandlers::ExitMethodCallback);
256261

257262
napi_value rt_version;
258-
napi_create_string_utf8(env, NATIVE_SCRIPT_RUNTIME_VERSION, NAPI_AUTO_LENGTH, &rt_version);
259-
napi_set_named_property(env, global, "__runtimeVersion", rt_version);
263+
NAPI_GUARD(napi_create_string_utf8(env, NATIVE_SCRIPT_RUNTIME_VERSION, NAPI_AUTO_LENGTH, &rt_version)) {}
264+
NAPI_GUARD(napi_set_named_property(env, global, "__runtimeVersion", rt_version)) {}
260265

261266
napi_value engine;
262267
js_get_runtime_version(env, &engine);
263-
napi_set_named_property(env, global, "__engine", engine);
268+
NAPI_GUARD(napi_set_named_property(env, global, "__engine", engine)) {}
264269

265270

266271
napi_util::napi_set_function(env, global, "__time", CallbackHandlers::TimeCallback);
@@ -272,8 +277,9 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
272277
CallbackHandlers::RemoveFrameCallback);
273278
napi_util::napi_set_function(env, global, "__markingMode",
274279
[](napi_env _env, napi_callback_info) -> napi_value {
280+
napi_status status;
275281
napi_value mode;
276-
napi_create_int32(_env, 0, &mode);
282+
NAPI_GUARD(napi_create_int32(_env, 0, &mode)) { return nullptr; }
277283
return mode;
278284
});
279285

@@ -345,14 +351,14 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
345351
*/
346352
{
347353
napi_value worker;
348-
napi_define_class(env, "Worker", strlen("Worker"), CallbackHandlers::NewThreadCallback,
349-
nullptr, 0, nullptr, &worker);
354+
NAPI_GUARD(napi_define_class(env, "Worker", strlen("Worker"), CallbackHandlers::NewThreadCallback,
355+
nullptr, 0, nullptr, &worker)) {}
350356
napi_value prototype = napi_util::get_prototype(env, worker);
351357
napi_util::napi_set_function(env, prototype, "postMessage",
352358
CallbackHandlers::WorkerObjectPostMessageCallback, nullptr);
353359
napi_util::napi_set_function(env, prototype, "terminate",
354360
CallbackHandlers::WorkerObjectTerminateCallback, nullptr);
355-
napi_set_named_property(env, global, "Worker", worker);
361+
NAPI_GUARD(napi_set_named_property(env, global, "Worker", worker)) {}
356362
}
357363

358364
napi_util::define_property(env, global, "global", nullptr, GlobalAccessorCallback);
@@ -385,7 +391,7 @@ void Runtime::Init(JNIEnv *_env, jstring filesPath, jstring nativeLibsDir,
385391

386392
s_mainThreadInitialized = true;
387393

388-
napi_close_handle_scope(env, handleScope);
394+
NAPI_GUARD(napi_close_handle_scope(env, handleScope)) {}
389395

390396
DEBUG_WRITE("%s", "NativeScript Runtime Loaded!");
391397
}
@@ -443,6 +449,7 @@ std::string Runtime::ReadFileText(const std::string &filePath) {
443449
}
444450

445451
void Runtime::DestroyRuntime() {
452+
napi_status status;
446453
is_destroying = true;
447454
if (m_looperTasks != nullptr) {
448455
m_looperTasks->Terminate();
@@ -456,7 +463,7 @@ void Runtime::DestroyRuntime() {
456463
Console::onDisposeEnv(env);
457464
CallbackHandlers::RemoveEnvEntries(env);
458465
this->m_objectManager->OnDisposeEnv();
459-
napi_close_handle_scope(env, this->global_scope);
466+
NAPI_GUARD(napi_close_handle_scope(env, this->global_scope)) {}
460467
Runtime::thread_id_to_rt_cache.Remove(this->my_thread_id);
461468
id_to_runtime_cache.Remove(m_id);
462469
env_to_runtime_cache.Remove(env);
@@ -492,20 +499,21 @@ void Runtime::AdjustAmountOfExternalAllocatedMemory() {
492499

493500
bool Runtime::TryCallGC() {
494501
if (this->is_destroying) return true;
502+
napi_status status;
495503
napi_value global;
496-
napi_get_global(env, &global);
504+
NAPI_GUARD(napi_get_global(env, &global)) { return false; }
497505
if (!m_gcFunc) {
498506
napi_value gc;
499-
napi_get_named_property(env, global, "gc", &gc);
507+
NAPI_GUARD(napi_get_named_property(env, global, "gc", &gc)) { return false; }
500508
if (napi_util::is_null_or_undefined(env, gc)) return true;
501-
napi_create_reference(env, gc, 1, &m_gcFunc);
509+
NAPI_GUARD(napi_create_reference(env, gc, 1, &m_gcFunc)) { return false; }
502510
}
503511

504512
bool success = __sync_bool_compare_and_swap(&m_runGC, true, false);
505513

506514
if (success) {
507515
napi_value result;
508-
napi_call_function(env, global, napi_util::get_ref_value(env, m_gcFunc), 0, nullptr, &result);
516+
NAPI_GUARD(napi_call_function(env, global, napi_util::get_ref_value(env, m_gcFunc), 0, nullptr, &result)) {}
509517
}
510518

511519
return success;
@@ -515,11 +523,12 @@ void Runtime::RunModule(JNIEnv *_jEnv, jobject obj, jstring scriptFile) {
515523
JEnv jEnv(_jEnv);
516524
string filePath = ArgConverter::jstringToString(scriptFile);
517525
m_module.Load(env, filePath);
526+
napi_status status;
518527
bool pendingException;
519-
napi_is_exception_pending(env, &pendingException);
528+
NAPI_GUARD(napi_is_exception_pending(env, &pendingException)) { return; }
520529
if (pendingException) {
521530
napi_value error = nullptr;
522-
napi_get_and_clear_last_exception(env, &error);
531+
NAPI_GUARD(napi_get_and_clear_last_exception(env, &error)) {}
523532
throw NativeScriptException(env, error, string("Error running module at path: ") + filePath);
524533
}
525534
}
@@ -549,7 +558,7 @@ jobject Runtime::RunScript(JNIEnv *_env, jobject obj, jstring scriptFile) {
549558
auto src = ReadFileText(filename);
550559

551560
napi_value soureCode;
552-
napi_create_string_utf8(env, src.c_str(), src.length(), &soureCode);
561+
NAPI_GUARD(napi_create_string_utf8(env, src.c_str(), src.length(), &soureCode)) { return nullptr; }
553562

554563
napi_value result;
555564
DEBUG_WRITE("%s", filename.c_str());
@@ -560,7 +569,7 @@ jobject Runtime::RunScript(JNIEnv *_env, jobject obj, jstring scriptFile) {
560569
if (status != napi_ok || pendingException) {
561570
napi_value error = nullptr;
562571
if (pendingException) {
563-
napi_get_and_clear_last_exception(env, &error);
572+
NAPI_GUARD(napi_get_and_clear_last_exception(env, &error)) {}
564573
}
565574
if (error) {
566575
throw NativeScriptException(env, error, "Error running script " + filename);
@@ -691,13 +700,14 @@ Runtime::PassExceptionToJsNative(JNIEnv *jEnv, jobject obj, jthrowable exception
691700
jstring fullStackTrace, jstring jsStackTrace,
692701
jboolean isDiscarded, jboolean isPendingError) {
693702
napi_env napiEnv = env;
703+
napi_status status;
694704

695705
std::string errMsg = ArgConverter::jstringToString(message);
696706

697707
napi_value errObj;
698708
napi_value errMsgNapi;
699-
napi_create_string_utf8(napiEnv, errMsg.c_str(), NAPI_AUTO_LENGTH, &errMsgNapi);
700-
napi_create_error(napiEnv, nullptr, errMsgNapi, &errObj);
709+
NAPI_GUARD(napi_create_string_utf8(napiEnv, errMsg.c_str(), NAPI_AUTO_LENGTH, &errMsgNapi)) {}
710+
NAPI_GUARD(napi_create_error(napiEnv, nullptr, errMsgNapi, &errObj)) {}
701711

702712
// Create a new native exception js object
703713
jint javaObjectID = m_objectManager->GetOrCreateObjectId((jobject) exception);
@@ -708,21 +718,21 @@ Runtime::PassExceptionToJsNative(JNIEnv *jEnv, jobject obj, jthrowable exception
708718
// Create proxy object that wraps the java err
709719
nativeExceptionObject = m_objectManager->CreateJSWrapper(javaObjectID, className);
710720
if (nativeExceptionObject == nullptr) {
711-
napi_create_object(napiEnv, &nativeExceptionObject);
721+
NAPI_GUARD(napi_create_object(napiEnv, &nativeExceptionObject)) {}
712722
}
713723
}
714724

715725
// Create a JS error object
716726
napi_value fullStackTraceNapi;
717-
napi_create_string_utf8(napiEnv, ArgConverter::jstringToString(fullStackTrace).c_str(),
718-
NAPI_AUTO_LENGTH, &fullStackTraceNapi);
719-
napi_set_named_property(napiEnv, errObj, "nativeException", nativeExceptionObject);
720-
napi_set_named_property(napiEnv, errObj, "stackTrace", fullStackTraceNapi);
727+
NAPI_GUARD(napi_create_string_utf8(napiEnv, ArgConverter::jstringToString(fullStackTrace).c_str(),
728+
NAPI_AUTO_LENGTH, &fullStackTraceNapi)) {}
729+
NAPI_GUARD(napi_set_named_property(napiEnv, errObj, "nativeException", nativeExceptionObject)) {}
730+
NAPI_GUARD(napi_set_named_property(napiEnv, errObj, "stackTrace", fullStackTraceNapi)) {}
721731
if (jsStackTrace != nullptr) {
722732
napi_value jsStackTraceNapi;
723-
napi_create_string_utf8(napiEnv, ArgConverter::jstringToString(jsStackTrace).c_str(),
724-
NAPI_AUTO_LENGTH, &jsStackTraceNapi);
725-
napi_set_named_property(napiEnv, errObj, "stack", jsStackTraceNapi);
733+
NAPI_GUARD(napi_create_string_utf8(napiEnv, ArgConverter::jstringToString(jsStackTrace).c_str(),
734+
NAPI_AUTO_LENGTH, &jsStackTraceNapi)) {}
735+
NAPI_GUARD(napi_set_named_property(napiEnv, errObj, "stack", jsStackTraceNapi)) {}
726736
}
727737

728738
// Pass err to JS

0 commit comments

Comments
 (0)