diff --git a/Core/AppRuntime/Include/Babylon/AppRuntime.h b/Core/AppRuntime/Include/Babylon/AppRuntime.h index f6ca7dfd..654da357 100644 --- a/Core/AppRuntime/Include/Babylon/AppRuntime.h +++ b/Core/AppRuntime/Include/Babylon/AppRuntime.h @@ -45,6 +45,24 @@ namespace Babylon void Dispatch(Dispatchable callback); + // Routes an unhandled promise rejection to the embedder's UnhandledExceptionHandler (which + // defaults to a benign logger), so an embedder's crash/telemetry pipeline can observe + // fire-and-forget failures (e.g. an un-awaited fetch() that rejects) -- matching the browser + // `unhandledrejection` behavior. Reporting is deferred to the end of the turn, so a rejection + // handled synchronously (e.g. `const p = Promise.reject(e); p.catch(...)`) is not reported. + // + // Coverage is determined by whether the engine exposes a host promise-rejection hook: + // * V8 (Isolate::SetPromiseRejectCallback) -- supported on all platforms. + // * Apple JavaScriptCore (JSGlobalContextSetUnhandledRejectionCallback) -- supported. This + // is an SPI present only in Apple's JSC; the WebKitGTK JSC used on Linux does not expose + // it, so tracking is a no-op there. + // * Chakra (in-box/EdgeMode) and JSI -- no-op: neither exposes such a hook + // (JsSetHostPromiseRejectionTracker is ChakraCore-only, and neither jsi::Runtime nor + // V8JSI surfaces the V8 callback). + // + // Intended for internal (engine-implementation) use. + void OnUnhandledPromiseRejection(const Napi::Error& error); + // Default unhandled exception handler that outputs the error message to the program output. static void BABYLON_API DefaultUnhandledExceptionHandler(const Napi::Error& error); diff --git a/Core/AppRuntime/Source/AppRuntime.cpp b/Core/AppRuntime/Source/AppRuntime.cpp index 99298df2..acb8e20e 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -125,4 +125,11 @@ namespace Babylon }); }); } + + void AppRuntime::OnUnhandledPromiseRejection(const Napi::Error& error) + { + // The reason is wrapped into a Napi::Error by the engine implementation (the napi_value -> + // Napi::Value bridge is shim-specific), so this just forwards to the embedder's handler. + m_options.UnhandledExceptionHandler(error); + } } diff --git a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp index 2329ad30..50e998f1 100644 --- a/Core/AppRuntime/Source/AppRuntime_Chakra.cpp +++ b/Core/AppRuntime/Source/AppRuntime_Chakra.cpp @@ -50,6 +50,11 @@ namespace Babylon }); }, &dispatchFunction)); + + // Unhandled promise rejection tracking (OnUnhandledPromiseRejection) is a no-op on this + // backend: the OS EdgeMode Chakra runtime (chakrart.h) exposes no host promise-rejection + // hook (JsSetHostPromiseRejectionTracker is ChakraCore-only). See AppRuntime.h. + ThrowIfFailed(JsProjectWinRTNamespace(L"Windows")); if (m_options.EnableDebugger) diff --git a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp index a0322898..5215c341 100644 --- a/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp +++ b/Core/AppRuntime/Source/AppRuntime_JavaScriptCore.cpp @@ -1,8 +1,56 @@ #include "AppRuntime.h" #include +#if __APPLE__ +#include "AppRuntime_PromiseRejection.h" + +// JSGlobalContextSetUnhandledRejectionCallback is declared in the private JavaScriptCore header +// , which is not part of the public macOS/iOS SDK. The symbol +// is exported by the JavaScriptCore framework (SPI), so it is forward-declared here and the call is +// guarded with __builtin_available (it is JSC_API_AVAILABLE(macos(10.15.4), ios(13.4))). It registers +// a JS function invoked at the microtask checkpoint with (promise, reason) for each promise that is +// still unhandled at that point, so -- unlike V8 -- no deferral or candidate bookkeeping is needed. +// This is Apple-only: the WebKitGTK JavaScriptCore used on Linux exposes neither this SPI nor +// __builtin_available, so unhandled-rejection tracking is a no-op there (see AppRuntime.h). +extern "C" void JSGlobalContextSetUnhandledRejectionCallback(JSGlobalContextRef ctx, JSObjectRef function, JSValueRef* exception); +#endif + namespace Babylon { +#if __APPLE__ + namespace + { + // JSObjectMakeFunctionWithCallback takes no user-data argument; each AppRuntime owns its JSC + // context on a dedicated thread, so a thread_local associates the callback with this runtime. + struct JSCRejectionContext + { + AppRuntime* runtime{}; + napi_env env{}; + }; + + thread_local JSCRejectionContext* t_rejectionContext{nullptr}; + + // Mirrors ToNapi (js_native_api_javascriptcore.cc): napi_value is a JSValueRef in the + // JavaScriptCore Node-API shim. + napi_value JsValueToNapi(JSValueRef value) + { + return reinterpret_cast(const_cast(value)); + } + + JSValueRef OnUnhandledRejection(JSContextRef ctx, JSObjectRef, JSObjectRef, size_t argumentCount, const JSValueRef arguments[], JSValueRef*) + { + JSCRejectionContext* context{t_rejectionContext}; + if (context != nullptr && argumentCount >= 2) + { + const Napi::Env env{context->env}; + context->runtime->OnUnhandledPromiseRejection(Internal::ToError(env, JsValueToNapi(arguments[1]))); + } + + return JSValueMakeUndefined(ctx); + } + } +#endif + void AppRuntime::RunEnvironmentTier(const char*) { auto globalContext = JSGlobalContextCreateInGroup(nullptr, nullptr); @@ -16,8 +64,25 @@ namespace Babylon Napi::Env env = Napi::Attach(globalContext); +#if __APPLE__ + // Always track unhandled promise rejections (routed to the host UnhandledExceptionHandler). + JSCRejectionContext rejectionContext{this, env}; + t_rejectionContext = &rejectionContext; + if (__builtin_available(iOS 13.4, macOS 10.15.4, *)) + { + JSStringRef callbackName = JSStringCreateWithUTF8CString("onUnhandledRejection"); + JSObjectRef callback = JSObjectMakeFunctionWithCallback(globalContext, callbackName, OnUnhandledRejection); + JSStringRelease(callbackName); + JSGlobalContextSetUnhandledRejectionCallback(globalContext, callback, nullptr); + } +#endif + Run(env); +#if __APPLE__ + t_rejectionContext = nullptr; +#endif + JSGlobalContextRelease(globalContext); // Detach must come after JSGlobalContextRelease since it triggers finalizers which require env. diff --git a/Core/AppRuntime/Source/AppRuntime_PromiseRejection.h b/Core/AppRuntime/Source/AppRuntime_PromiseRejection.h new file mode 100644 index 00000000..6121561b --- /dev/null +++ b/Core/AppRuntime/Source/AppRuntime_PromiseRejection.h @@ -0,0 +1,85 @@ +#pragma once + +#include "AppRuntime.h" + +#include + +#include +#include +#include + +namespace Babylon::Internal +{ + // Wraps an unhandled-promise rejection reason as a Napi::Error: an Error-like object passes + // through (preserving message/stack/cause); any other value is stringified so the host handler + // always receives a Napi::Error. Lives here rather than in shared AppRuntime.cpp because the + // napi_value -> Napi::Value bridge is unavailable on the JSI Node-API shim, and only the engines + // that support rejection tracking (V8, JavaScriptCore) include this header. + inline Napi::Error ToError(Napi::Env env, napi_value reason) + { + const Napi::Value value{env, reason}; + return value.IsObject() + ? Napi::Error{env, reason} + : Napi::Error::New(env, value.ToString().Utf8Value()); + } + + // Engine-agnostic bookkeeping for engines that report an unhandled rejection immediately and a + // later handler-added event separately (V8): collect candidates as promises reject without a + // handler, drop them when a handler is attached, and report the survivors to the host handler at + // the end of the current turn -- so a rejection handled synchronously within the same turn is + // never reported. Engines whose host hook already fires only for still-unhandled rejections at + // the microtask checkpoint (JavaScriptCore) report directly and do not need this. + // + // CandidateT is engine-specific and must provide: + // void Report(AppRuntime& runtime, Napi::Env env) const; + // // convert its stored reason to a Napi::Error (via ToError) and call + // // runtime.OnUnhandledPromiseRejection + template + class PromiseRejectionTracker + { + public: + explicit PromiseRejectionTracker(AppRuntime& runtime) + : m_runtime{runtime} + { + } + + void Add(CandidateT candidate) + { + m_candidates.push_back(std::move(candidate)); + + if (!m_flushScheduled) + { + m_flushScheduled = true; + m_runtime.Dispatch([this](Napi::Env env) { Flush(env); }); + } + } + + template + void Remove(PredicateT predicate) + { + m_candidates.erase( + std::remove_if(m_candidates.begin(), m_candidates.end(), std::move(predicate)), + m_candidates.end()); + } + + private: + void Flush(Napi::Env env) + { + m_flushScheduled = false; + + // Move the candidates out before reporting: a host handler could synchronously reject + // another promise, re-entering Add() and mutating m_candidates mid-iteration. + const std::vector candidates = std::move(m_candidates); + m_candidates.clear(); + + for (const CandidateT& candidate : candidates) + { + candidate.Report(m_runtime, env); + } + } + + AppRuntime& m_runtime; + std::vector m_candidates; + bool m_flushScheduled{false}; + }; +} diff --git a/Core/AppRuntime/Source/AppRuntime_V8.cpp b/Core/AppRuntime/Source/AppRuntime_V8.cpp index 1297fdbb..2847797f 100644 --- a/Core/AppRuntime/Source/AppRuntime_V8.cpp +++ b/Core/AppRuntime/Source/AppRuntime_V8.cpp @@ -1,4 +1,5 @@ #include "AppRuntime.h" +#include "AppRuntime_PromiseRejection.h" #include #include @@ -61,6 +62,78 @@ namespace Babylon }; std::unique_ptr Module::s_module; + + // Mirrors v8impl::JsValueFromV8LocalValue (js_native_api_v8.h), which is internal to the + // Node-API V8 shim and not on the public include path. + static_assert(sizeof(v8::Local) == sizeof(napi_value), + "Cannot convert between v8::Local and napi_value"); + napi_value JsValueFromV8LocalValue(v8::Local local) + { + return reinterpret_cast(*local); + } + + // A promise rejected without a handler, awaiting end-of-turn reporting. The promise and + // reason are held in v8::Global handles so they survive until the deferred flush; the promise + // is retained so a later handler-added event can drop this candidate by object identity + // (v8::Object::GetIdentityHash is not unique, so identity comparison is used instead). + struct V8RejectionCandidate + { + v8::Isolate* isolate{}; + v8::Global promise; + v8::Global reason; + + void Report(AppRuntime& runtime, Napi::Env env) const + { + v8::HandleScope handleScope{isolate}; + runtime.OnUnhandledPromiseRejection(Internal::ToError(env, JsValueFromV8LocalValue(reason.Get(isolate)))); + } + }; + + using V8RejectionTracker = Internal::PromiseRejectionTracker; + + // The promise-rejection callback is a bare function pointer with no user-data argument. Each + // AppRuntime owns a dedicated isolate running on its own thread, and V8 invokes the callback + // on that thread, so a thread_local pointer associates the callback with the right tracker + // without risking isolate-data-slot collisions with the Node-API shim. + thread_local V8RejectionTracker* t_rejectionTracker{nullptr}; + + void OnPromiseReject(v8::PromiseRejectMessage message) + { + V8RejectionTracker* tracker{t_rejectionTracker}; + if (tracker == nullptr) + { + return; + } + + v8::Isolate* isolate{v8::Isolate::GetCurrent()}; + v8::HandleScope handleScope{isolate}; + const v8::Local promise{message.GetPromise()}; + + switch (message.GetEvent()) + { + case v8::kPromiseRejectWithNoHandler: + { + tracker->Add(V8RejectionCandidate{ + isolate, + v8::Global{isolate, promise}, + v8::Global{isolate, message.GetValue()}}); + break; + } + case v8::kPromiseHandlerAddedAfterReject: + { + tracker->Remove([isolate, promise](const V8RejectionCandidate& candidate) { + return candidate.promise.Get(isolate) == promise; + }); + break; + } + default: + { + // kPromiseRejectAfterResolved / kPromiseResolveAfterResolved carry no actionable + // unhandled-rejection signal. + break; + } + } + } } void AppRuntime::RunEnvironmentTier(const char* executablePath) @@ -81,6 +154,11 @@ namespace Babylon Napi::Env env = Napi::Attach(context); + // Always track unhandled promise rejections (routed to the host UnhandledExceptionHandler). + V8RejectionTracker rejectionTracker{*this}; + t_rejectionTracker = &rejectionTracker; + isolate->SetPromiseRejectCallback(OnPromiseReject); + #ifdef ENABLE_V8_INSPECTOR std::optional agent; if (m_options.EnableDebugger) @@ -104,6 +182,9 @@ namespace Babylon } #endif + isolate->SetPromiseRejectCallback(nullptr); + t_rejectionTracker = nullptr; + Napi::Detach(env); } diff --git a/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt b/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt index 0af5caa8..54dbda87 100644 --- a/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt +++ b/Tests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt @@ -22,6 +22,9 @@ add_library(UnitTestsJNI SHARED ${UNIT_TESTS_DIR}/Shared/Shared.cpp) target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}") +target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_NAPI_ENGINE="${NAPI_JAVASCRIPT_ENGINE}") +# Engine-specific compile-time gate (e.g. JSRUNTIMEHOST_NAPI_ENGINE_V8), matching Tests/UnitTests/CMakeLists.txt. +target_compile_definitions(UnitTestsJNI PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_${NAPI_JAVASCRIPT_ENGINE}) target_compile_definitions(UnitTestsJNI PRIVATE ARCANA_TEST_HOOKS) target_include_directories(UnitTestsJNI diff --git a/Tests/UnitTests/CMakeLists.txt b/Tests/UnitTests/CMakeLists.txt index 2dbc7619..ea8c6c82 100644 --- a/Tests/UnitTests/CMakeLists.txt +++ b/Tests/UnitTests/CMakeLists.txt @@ -45,12 +45,14 @@ endif() add_executable(UnitTests ${SOURCES} ${SCRIPTS} ${TYPE_SCRIPTS} ${ASSETS}) target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_PLATFORM="${JSRUNTIMEHOST_PLATFORM}") - -# The V8JSI Node-API shim does not implement napi_create_dataview, so the -# CreateDataViewRejectsOverflowingRange test is compiled out on that backend. -if(NAPI_JAVASCRIPT_ENGINE STREQUAL "JSI") - target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_JSI) -endif() +target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE="${NAPI_JAVASCRIPT_ENGINE}") +# Engine-specific compile-time gate so tests can select behavior by the active Node-API engine +# without a runtime string comparison, e.g.: +# JSRUNTIMEHOST_NAPI_ENGINE_V8, JSRUNTIMEHOST_NAPI_ENGINE_JavaScriptCore, +# JSRUNTIMEHOST_NAPI_ENGINE_Chakra, JSRUNTIMEHOST_NAPI_ENGINE_JSI. +# (JSI compiles out CreateDataViewRejectsOverflowingRange since the V8JSI shim lacks +# napi_create_dataview; V8/JavaScriptCore gate the unhandled-rejection handler tests.) +target_compile_definitions(UnitTests PRIVATE JSRUNTIMEHOST_NAPI_ENGINE_${NAPI_JAVASCRIPT_ENGINE}) target_link_libraries(UnitTests PRIVATE AppRuntime diff --git a/Tests/UnitTests/Shared/Shared.cpp b/Tests/UnitTests/Shared/Shared.cpp index a920fa1f..e773a57b 100644 --- a/Tests/UnitTests/Shared/Shared.cpp +++ b/Tests/UnitTests/Shared/Shared.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include namespace @@ -279,6 +280,66 @@ TEST(AppRuntime, DestroyDoesNotDeadlock) testThread.join(); } +TEST(AppRuntime, UnhandledPromiseRejectionReachesHandler) +{ + // Unhandled promise rejection tracking is implemented on the engines that expose a host + // promise-rejection hook: V8 (Isolate::SetPromiseRejectCallback) and Apple JavaScriptCore + // (JSGlobalContextSetUnhandledRejectionCallback, an SPI absent from WebKitGTK/Linux JSC). The OS + // EdgeMode Chakra runtime and the V8JSI (JSI) shim expose no such hook, so the body is compiled + // out there (and on non-Apple JSC) and the test is skipped. +#if !(defined(JSRUNTIMEHOST_NAPI_ENGINE_V8) || (defined(JSRUNTIMEHOST_NAPI_ENGINE_JavaScriptCore) && defined(__APPLE__))) + GTEST_SKIP() << "unhandled promise rejection tracking requires the V8 or Apple JavaScriptCore backend"; +#else + // A fire-and-forget rejected promise (no handler ever attached) must reach the embedder's + // UnhandledExceptionHandler. + Babylon::AppRuntime::Options options{}; + + std::promise rejectionMessage; + options.UnhandledExceptionHandler = [&rejectionMessage](const Napi::Error& error) { + rejectionMessage.set_value(error.Message()); + }; + + Babylon::AppRuntime runtime{options}; + + Babylon::ScriptLoader loader{runtime}; + loader.Eval("Promise.reject(new Error('boom from fire-and-forget'));", ""); + + auto future = rejectionMessage.get_future(); + ASSERT_EQ(future.wait_for(std::chrono::seconds(30)), std::future_status::ready) + << "unhandled rejection did not reach the host handler"; + EXPECT_NE(future.get().find("boom from fire-and-forget"), std::string::npos); +#endif +} + +TEST(AppRuntime, SynchronouslyHandledRejectionDoesNotReachHandler) +{ + // Only engines with a host promise-rejection hook implement this tracking (see the note above). +#if !(defined(JSRUNTIMEHOST_NAPI_ENGINE_V8) || (defined(JSRUNTIMEHOST_NAPI_ENGINE_JavaScriptCore) && defined(__APPLE__))) + GTEST_SKIP() << "unhandled promise rejection tracking requires the V8 or Apple JavaScriptCore backend"; +#else + // A rejection that is handled synchronously in the same turn must NOT reach the handler -- + // reporting is deferred to the end of the turn, by which point the .catch has been attached. + Babylon::AppRuntime::Options options{}; + + std::atomic handlerFired{false}; + options.UnhandledExceptionHandler = [&handlerFired](const Napi::Error&) { + handlerFired = true; + }; + + Babylon::AppRuntime runtime{options}; + + Babylon::ScriptLoader loader{runtime}; + loader.Eval("const p = Promise.reject(new Error('handled')); p.catch(() => {});", ""); + + // Round-trip a dispatch so any deferred rejection-flush task has run before we check. + std::promise drained; + loader.Dispatch([&drained](Napi::Env) { drained.set_value(); }); + drained.get_future().wait(); + + EXPECT_FALSE(handlerFired.load()) << "a synchronously-handled rejection must not reach the host handler"; +#endif +} + // The V8JSI Node-API shim does not implement napi_create_dataview / // napi_get_dataview_info (its DataView::New throws "TODO"), so this native test // only builds on the Chakra, V8, and JavaScriptCore backends. The size_t-width