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..568a18ae 100644 --- a/Core/AppRuntime/Source/AppRuntime.cpp +++ b/Core/AppRuntime/Source/AppRuntime.cpp @@ -125,4 +125,23 @@ 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. + // + // The handler is embedder code and may throw. JavaScriptCore reports rejections from inside + // an engine callback, where an escaping C++ exception would unwind through the engine's own + // frames; on the V8 path it would reach Dispatch, whose catch-all aborts the process. There + // is nothing useful left to do with a failure to report a failure, so it is swallowed: + // diagnostics must never be what takes the process down. + try + { + m_options.UnhandledExceptionHandler(error); + } + catch (...) + { + } + } } 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..c6159897 --- /dev/null +++ b/Core/AppRuntime/Source/AppRuntime_PromiseRejection.h @@ -0,0 +1,108 @@ +#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) + { + try + { + bool isError{false}; + if (napi_is_error(env, reason, &isError) == napi_ok && isError) + { + return Napi::Error{env, reason}; + } + + // Not a native Error, but an object carrying a string `message` is error-like enough to + // pass through with its message and stack intact -- a reason from another realm, or a + // polyfill's own error type. Testing only for "is an object" would also let a plain + // object such as `{code: 42}` through, and that yields an error whose message is empty. + const Napi::Value value{env, reason}; + if (value.IsObject() && value.As().Get("message").IsString()) + { + return Napi::Error{env, reason}; + } + + return Napi::Error::New(env, value.ToString().Utf8Value()); + } + catch (...) + { + // Both the property read and the string conversion run script (getters, toString) and so + // can throw. This is called from engine callbacks, where an escaping C++ exception would + // unwind through the engine's own frames, so it must not propagate. + return Napi::Error::New(env, "unhandled promise rejection with a reason that could not be converted to an error"); + } + } + + // 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..7d10a709 100644 --- a/Tests/UnitTests/Shared/Shared.cpp +++ b/Tests/UnitTests/Shared/Shared.cpp @@ -279,6 +279,123 @@ 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; + auto future = rejectionMessage.get_future(); + + // UnhandledExceptionHandler is the runtime's handler for every unhandled error, not just + // rejections, so it can fire more than once; a second set_value would throw std::future_error + // out of the handler. + std::atomic reported{false}; + options.UnhandledExceptionHandler = [&rejectionMessage, &reported](const Napi::Error& error) { + if (!reported.exchange(true)) + { + rejectionMessage.set_value(error.Message()); + } + }; + + Babylon::AppRuntime runtime{options}; + + Babylon::ScriptLoader loader{runtime}; + loader.Eval("Promise.reject(new Error('boom from fire-and-forget'));", ""); + + 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 +} + +TEST(AppRuntime, NonErrorRejectionReasonsStillCarryAMessage) +{ + // 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 promise can be rejected with any value, but the host handler takes a Napi::Error. Whatever + // the reason is, the error it arrives as has to carry a usable message -- reporting a rejection + // with an empty message is barely better than not reporting it. + const auto reportedMessageFor = [](const char* script) { + Babylon::AppRuntime::Options options{}; + + std::promise rejectionMessage; + auto future = rejectionMessage.get_future(); + + std::atomic reported{false}; + options.UnhandledExceptionHandler = [&rejectionMessage, &reported](const Napi::Error& error) { + if (!reported.exchange(true)) + { + rejectionMessage.set_value(error.Message()); + } + }; + + Babylon::AppRuntime runtime{options}; + + Babylon::ScriptLoader loader{runtime}; + loader.Eval(script, ""); + + if (future.wait_for(std::chrono::seconds(30)) != std::future_status::ready) + { + return std::string{""}; + } + + return future.get(); + }; + + // A string reason is stringified. + EXPECT_NE(reportedMessageFor("Promise.reject('a plain string reason');").find("a plain string reason"), std::string::npos); + + // An object carrying a message is error-like enough to pass through with it, even though it is + // not a native Error. + EXPECT_NE(reportedMessageFor("Promise.reject({ message: 'error-like object' });").find("error-like object"), std::string::npos); + + // A plain object has no message to pass through, so it must be stringified rather than yielding + // an error with an empty message. + EXPECT_FALSE(reportedMessageFor("Promise.reject({ code: 42 });").empty()); +#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