From 1c2775aa9c5ec253eee7487d6c700e03572a4177 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Sat, 31 Oct 2026 21:20:21 +0100 Subject: [PATCH 1/4] feat(host): log Hermes fatal errors through a fatal handler Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/hermes-fatal-handler.md | 5 ++ packages/host/android/CMakeLists.txt | 2 + packages/host/android/src/main/cpp/OnLoad.cpp | 2 + .../host/apple/NodeApiHostModuleProvider.mm | 2 + packages/host/cpp/HermesFatalHandler.cpp | 18 +++++++ packages/host/cpp/HermesFatalHandler.hpp | 19 +++++++ packages/host/cpp/RuntimeNodeApi.hpp | 9 +--- packages/host/tests/CMakeLists.txt | 1 + .../host/tests/test_hermes_fatal_handler.cpp | 51 +++++++++++++++++++ 9 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 .changeset/hermes-fatal-handler.md create mode 100644 packages/host/cpp/HermesFatalHandler.cpp create mode 100644 packages/host/cpp/HermesFatalHandler.hpp create mode 100644 packages/host/tests/test_hermes_fatal_handler.cpp diff --git a/.changeset/hermes-fatal-handler.md b/.changeset/hermes-fatal-handler.md new file mode 100644 index 00000000..05bf16cf --- /dev/null +++ b/.changeset/hermes-fatal-handler.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": patch +--- + +Install a Hermes fatal handler when the host library loads, on Android and Apple platforms. Hermes' fatal errors, including those that never pass through Node-API, are now logged through the host logger before the process aborts, so they reach logcat on Android instead of only stderr. The handler is process-wide: it replaces one set before the host library loads, and one set afterwards replaces it. diff --git a/packages/host/android/CMakeLists.txt b/packages/host/android/CMakeLists.txt index 2a45e96d..933359e0 100644 --- a/packages/host/android/CMakeLists.txt +++ b/packages/host/android/CMakeLists.txt @@ -16,6 +16,8 @@ add_library(node-api-host SHARED ../cpp/RuntimeNodeApi.hpp ../cpp/HermesNapiHost.cpp ../cpp/HermesNapiHost.hpp + ../cpp/HermesFatalHandler.cpp + ../cpp/HermesFatalHandler.hpp ) target_include_directories(node-api-host PRIVATE diff --git a/packages/host/android/src/main/cpp/OnLoad.cpp b/packages/host/android/src/main/cpp/OnLoad.cpp index 3cd6f306..58d8741a 100644 --- a/packages/host/android/src/main/cpp/OnLoad.cpp +++ b/packages/host/android/src/main/cpp/OnLoad.cpp @@ -3,10 +3,12 @@ #include #include +#include #include // Called when the library is loaded jint JNI_OnLoad(JavaVM *vm, void *reserved) { + callstack::react_native_node_api::installHermesFatalHandler(); callstack::react_native_node_api::injectIntoWeakNodeApi(); // Register the C++ TurboModule facebook::react::registerCxxModuleToGlobalModuleMap( diff --git a/packages/host/apple/NodeApiHostModuleProvider.mm b/packages/host/apple/NodeApiHostModuleProvider.mm index 1ea633fe..a3117e5d 100644 --- a/packages/host/apple/NodeApiHostModuleProvider.mm +++ b/packages/host/apple/NodeApiHostModuleProvider.mm @@ -1,4 +1,5 @@ #import "CxxNodeApiHostModule.hpp" +#import "HermesFatalHandler.hpp" #import "WeakNodeApiInjector.hpp" #import @@ -8,6 +9,7 @@ @interface NodeApiHostPackage : NSObject @implementation NodeApiHostPackage + (void)load { + callstack::react_native_node_api::installHermesFatalHandler(); callstack::react_native_node_api::injectIntoWeakNodeApi(); facebook::react::registerCxxModuleToGlobalModuleMap( diff --git a/packages/host/cpp/HermesFatalHandler.cpp b/packages/host/cpp/HermesFatalHandler.cpp new file mode 100644 index 00000000..97a9e00a --- /dev/null +++ b/packages/host/cpp/HermesFatalHandler.cpp @@ -0,0 +1,18 @@ +#include "HermesFatalHandler.hpp" + +#include + +namespace callstack::react_native_node_api { + +void installHermesFatalHandler() { + auto *setter = + facebook::jsi::castInterface( + facebook::hermes::makeHermesRootAPI()); + if (setter == nullptr) { + log_warning("Hermes doesn't support setting a fatal handler"); + return; + } + setter->setFatalHandler(handleHermesFatal); +} + +} // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/HermesFatalHandler.hpp b/packages/host/cpp/HermesFatalHandler.hpp new file mode 100644 index 00000000..6a061e44 --- /dev/null +++ b/packages/host/cpp/HermesFatalHandler.hpp @@ -0,0 +1,19 @@ +#pragma once + +#include "Logger.hpp" + +#include +#include + +namespace callstack::react_native_node_api { + +[[noreturn]] inline void handleHermesFatal(const std::string &reason) noexcept { + log_error("Hermes fatal error: %s", reason.c_str()); + std::abort(); +} + +/// Hermes keeps a single process-wide handler that can't be read back: this +/// replaces one set earlier, and one set later replaces this. +void installHermesFatalHandler(); + +} // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/RuntimeNodeApi.hpp b/packages/host/cpp/RuntimeNodeApi.hpp index 0440027c..1abedb21 100644 --- a/packages/host/cpp/RuntimeNodeApi.hpp +++ b/packages/host/cpp/RuntimeNodeApi.hpp @@ -4,12 +4,6 @@ namespace callstack::react_native_node_api { -// Hermes' first-party Node-API implementation (API/napi/hermes_napi.cpp) -// already defines napi_fatal_error, routing it through hermes_fatal() -> -// llvh::report_fatal_error(), which writes to stderr. On Android stderr is -// not logcat, so that message would be lost exactly when it matters most: -// right before the process aborts. -// // This declaration is deliberately kept so it shadows Hermes' symbol: the // generated injector (scripts/generate-injector.mts) resolves each // NodeApiHost field by unqualified name inside @@ -23,7 +17,8 @@ namespace callstack::react_native_node_api { // napi_get_version, napi_get_node_version) was removed in favor of letting // the same lookup fall through to Hermes' own implementation - see // https://github.com/callstackincubator/react-native-node-api/issues/428. -// Do not remove this one the same way without replacing the logcat routing. +// Remove this one too once a Node-API fatal error is confirmed to reach logcat +// on a device through installHermesFatalHandler(). void __attribute__((noreturn)) napi_fatal_error(const char *location, size_t location_len, const char *message, diff --git a/packages/host/tests/CMakeLists.txt b/packages/host/tests/CMakeLists.txt index 43723bbb..98d93975 100644 --- a/packages/host/tests/CMakeLists.txt +++ b/packages/host/tests/CMakeLists.txt @@ -21,6 +21,7 @@ FetchContent_MakeAvailable(Catch2) add_executable(node-api-host-tests test_hermes_napi_host.cpp + test_hermes_fatal_handler.cpp ../cpp/HermesNapiHost.cpp ../cpp/Logger.cpp ) diff --git a/packages/host/tests/test_hermes_fatal_handler.cpp b/packages/host/tests/test_hermes_fatal_handler.cpp new file mode 100644 index 00000000..cc0edab6 --- /dev/null +++ b/packages/host/tests/test_hermes_fatal_handler.cpp @@ -0,0 +1,51 @@ +// Skipped on Windows, which has no fork(). +#if !defined(_WIN32) + +#include + +#include + +#include +#include +#include + +#include +#include + +using namespace callstack::react_native_node_api; + +TEST_CASE("handleHermesFatal logs the reason, then aborts") { + // Printf conversions must reach the log verbatim, not be interpreted. + const std::string reason = "FATAL ERROR: some_location 100% %s %n"; + + int pipeFds[2]; + REQUIRE(pipe(pipeFds) == 0); + fflush(nullptr); + + const pid_t pid = fork(); + REQUIRE(pid >= 0); + if (pid == 0) { + // abort() doesn't flush stdio, and the host-platform logger uses stdout. + setvbuf(stdout, nullptr, _IONBF, 0); + dup2(pipeFds[1], STDOUT_FILENO); + dup2(pipeFds[1], STDERR_FILENO); + handleHermesFatal(reason); + } + close(pipeFds[1]); + + std::string output; + char buffer[256]; + ssize_t count = 0; + while ((count = read(pipeFds[0], buffer, sizeof(buffer))) > 0) { + output.append(buffer, static_cast(count)); + } + close(pipeFds[0]); + + int status = 0; + REQUIRE(waitpid(pid, &status, 0) == pid); + REQUIRE(WIFSIGNALED(status)); + REQUIRE(WTERMSIG(status) == SIGABRT); + REQUIRE(output.find("Hermes fatal error: " + reason) != std::string::npos); +} + +#endif From a6516bf7d99eaec45aa43aef6029381cf8a7669e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Sat, 31 Oct 2026 21:22:13 +0100 Subject: [PATCH 2/4] chore: trigger CI with labels Co-Authored-By: Claude Opus 5.5 (1M context) From bd15e1fe43917c2dbebd5b9f633530a102a80b46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Sat, 31 Oct 2026 21:28:45 +0100 Subject: [PATCH 3/4] refactor(host): drop the shadowing napi_fatal_error Replaces the fatal handler with a plain deletion: Hermes already handles fatal errors, and the handler slot is process-wide, so a library can't safely own it. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/drop-shadowing-napi-fatal-error.md | 5 ++ .changeset/hermes-fatal-handler.md | 5 -- packages/host/android/CMakeLists.txt | 4 -- packages/host/android/src/main/cpp/OnLoad.cpp | 2 - .../host/apple/NodeApiHostModuleProvider.mm | 2 - packages/host/cpp/HermesFatalHandler.cpp | 18 ------- packages/host/cpp/HermesFatalHandler.hpp | 19 ------- packages/host/cpp/RuntimeNodeApi.cpp | 22 -------- packages/host/cpp/RuntimeNodeApi.hpp | 27 ---------- packages/host/scripts/generate-injector.mts | 1 - packages/host/tests/CMakeLists.txt | 1 - .../host/tests/test_hermes_fatal_handler.cpp | 51 ------------------- 12 files changed, 5 insertions(+), 152 deletions(-) create mode 100644 .changeset/drop-shadowing-napi-fatal-error.md delete mode 100644 .changeset/hermes-fatal-handler.md delete mode 100644 packages/host/cpp/HermesFatalHandler.cpp delete mode 100644 packages/host/cpp/HermesFatalHandler.hpp delete mode 100644 packages/host/cpp/RuntimeNodeApi.cpp delete mode 100644 packages/host/cpp/RuntimeNodeApi.hpp delete mode 100644 packages/host/tests/test_hermes_fatal_handler.cpp diff --git a/.changeset/drop-shadowing-napi-fatal-error.md b/.changeset/drop-shadowing-napi-fatal-error.md new file mode 100644 index 00000000..86c1d79c --- /dev/null +++ b/.changeset/drop-shadowing-napi-fatal-error.md @@ -0,0 +1,5 @@ +--- +"react-native-node-api": patch +--- + +Drop the host's shadowing `napi_fatal_error` in favor of Hermes' own implementation. Fatal Node-API errors still abort the process, but on Android their message now goes to stderr instead of logcat. diff --git a/.changeset/hermes-fatal-handler.md b/.changeset/hermes-fatal-handler.md deleted file mode 100644 index 05bf16cf..00000000 --- a/.changeset/hermes-fatal-handler.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"react-native-node-api": patch ---- - -Install a Hermes fatal handler when the host library loads, on Android and Apple platforms. Hermes' fatal errors, including those that never pass through Node-API, are now logged through the host logger before the process aborts, so they reach logcat on Android instead of only stderr. The handler is process-wide: it replaces one set before the host library loads, and one set afterwards replaces it. diff --git a/packages/host/android/CMakeLists.txt b/packages/host/android/CMakeLists.txt index 933359e0..95032383 100644 --- a/packages/host/android/CMakeLists.txt +++ b/packages/host/android/CMakeLists.txt @@ -12,12 +12,8 @@ add_library(node-api-host SHARED ../cpp/Logger.cpp ../cpp/CxxNodeApiHostModule.cpp ../cpp/WeakNodeApiInjector.cpp - ../cpp/RuntimeNodeApi.cpp - ../cpp/RuntimeNodeApi.hpp ../cpp/HermesNapiHost.cpp ../cpp/HermesNapiHost.hpp - ../cpp/HermesFatalHandler.cpp - ../cpp/HermesFatalHandler.hpp ) target_include_directories(node-api-host PRIVATE diff --git a/packages/host/android/src/main/cpp/OnLoad.cpp b/packages/host/android/src/main/cpp/OnLoad.cpp index 58d8741a..3cd6f306 100644 --- a/packages/host/android/src/main/cpp/OnLoad.cpp +++ b/packages/host/android/src/main/cpp/OnLoad.cpp @@ -3,12 +3,10 @@ #include #include -#include #include // Called when the library is loaded jint JNI_OnLoad(JavaVM *vm, void *reserved) { - callstack::react_native_node_api::installHermesFatalHandler(); callstack::react_native_node_api::injectIntoWeakNodeApi(); // Register the C++ TurboModule facebook::react::registerCxxModuleToGlobalModuleMap( diff --git a/packages/host/apple/NodeApiHostModuleProvider.mm b/packages/host/apple/NodeApiHostModuleProvider.mm index a3117e5d..1ea633fe 100644 --- a/packages/host/apple/NodeApiHostModuleProvider.mm +++ b/packages/host/apple/NodeApiHostModuleProvider.mm @@ -1,5 +1,4 @@ #import "CxxNodeApiHostModule.hpp" -#import "HermesFatalHandler.hpp" #import "WeakNodeApiInjector.hpp" #import @@ -9,7 +8,6 @@ @interface NodeApiHostPackage : NSObject @implementation NodeApiHostPackage + (void)load { - callstack::react_native_node_api::installHermesFatalHandler(); callstack::react_native_node_api::injectIntoWeakNodeApi(); facebook::react::registerCxxModuleToGlobalModuleMap( diff --git a/packages/host/cpp/HermesFatalHandler.cpp b/packages/host/cpp/HermesFatalHandler.cpp deleted file mode 100644 index 97a9e00a..00000000 --- a/packages/host/cpp/HermesFatalHandler.cpp +++ /dev/null @@ -1,18 +0,0 @@ -#include "HermesFatalHandler.hpp" - -#include - -namespace callstack::react_native_node_api { - -void installHermesFatalHandler() { - auto *setter = - facebook::jsi::castInterface( - facebook::hermes::makeHermesRootAPI()); - if (setter == nullptr) { - log_warning("Hermes doesn't support setting a fatal handler"); - return; - } - setter->setFatalHandler(handleHermesFatal); -} - -} // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/HermesFatalHandler.hpp b/packages/host/cpp/HermesFatalHandler.hpp deleted file mode 100644 index 6a061e44..00000000 --- a/packages/host/cpp/HermesFatalHandler.hpp +++ /dev/null @@ -1,19 +0,0 @@ -#pragma once - -#include "Logger.hpp" - -#include -#include - -namespace callstack::react_native_node_api { - -[[noreturn]] inline void handleHermesFatal(const std::string &reason) noexcept { - log_error("Hermes fatal error: %s", reason.c_str()); - std::abort(); -} - -/// Hermes keeps a single process-wide handler that can't be read back: this -/// replaces one set earlier, and one set later replaces this. -void installHermesFatalHandler(); - -} // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/RuntimeNodeApi.cpp b/packages/host/cpp/RuntimeNodeApi.cpp deleted file mode 100644 index 880d904c..00000000 --- a/packages/host/cpp/RuntimeNodeApi.cpp +++ /dev/null @@ -1,22 +0,0 @@ -#include "RuntimeNodeApi.hpp" -#include "Logger.hpp" - -#include - -namespace callstack::react_native_node_api { - -// See the comment on the declaration in RuntimeNodeApi.hpp for why this -// deliberately shadows Hermes' own napi_fatal_error. -void napi_fatal_error(const char *location, size_t location_len, - const char *message, size_t message_len) { - if (location && location_len) { - log_error("Fatal Node-API error: %.*s %.*s", static_cast(location_len), - location, static_cast(message_len), message); - } else { - log_error("Fatal Node-API error: %.*s", static_cast(message_len), - message); - } - abort(); -} - -} // namespace callstack::react_native_node_api diff --git a/packages/host/cpp/RuntimeNodeApi.hpp b/packages/host/cpp/RuntimeNodeApi.hpp deleted file mode 100644 index 1abedb21..00000000 --- a/packages/host/cpp/RuntimeNodeApi.hpp +++ /dev/null @@ -1,27 +0,0 @@ -#pragma once - -#include "node_api.h" - -namespace callstack::react_native_node_api { - -// This declaration is deliberately kept so it shadows Hermes' symbol: the -// generated injector (scripts/generate-injector.mts) resolves each -// NodeApiHost field by unqualified name inside -// `namespace callstack::react_native_node_api`, and this header is included -// there, so unqualified lookup finds this declaration before it would reach -// Hermes' exported symbol. That lets the host's implementation win, which -// logs via the host logger to logcat with the "NodeApiHost" tag (see -// Logger.cpp) before aborting. -// -// Every other Node-API function the host used to shim here (buffers, -// napi_get_version, napi_get_node_version) was removed in favor of letting -// the same lookup fall through to Hermes' own implementation - see -// https://github.com/callstackincubator/react-native-node-api/issues/428. -// Remove this one too once a Node-API fatal error is confirmed to reach logcat -// on a device through installHermesFatalHandler(). -void __attribute__((noreturn)) napi_fatal_error(const char *location, - size_t location_len, - const char *message, - size_t message_len); - -} // namespace callstack::react_native_node_api diff --git a/packages/host/scripts/generate-injector.mts b/packages/host/scripts/generate-injector.mts index c58bb2f5..74a5402f 100644 --- a/packages/host/scripts/generate-injector.mts +++ b/packages/host/scripts/generate-injector.mts @@ -19,7 +19,6 @@ export function generateSource(functions: FunctionDecl[]) { #include #include - #include #if defined(__APPLE__) #define WEAK_NODE_API_LIBRARY_NAME "@rpath/weak-node-api.framework/weak-node-api" diff --git a/packages/host/tests/CMakeLists.txt b/packages/host/tests/CMakeLists.txt index 98d93975..43723bbb 100644 --- a/packages/host/tests/CMakeLists.txt +++ b/packages/host/tests/CMakeLists.txt @@ -21,7 +21,6 @@ FetchContent_MakeAvailable(Catch2) add_executable(node-api-host-tests test_hermes_napi_host.cpp - test_hermes_fatal_handler.cpp ../cpp/HermesNapiHost.cpp ../cpp/Logger.cpp ) diff --git a/packages/host/tests/test_hermes_fatal_handler.cpp b/packages/host/tests/test_hermes_fatal_handler.cpp deleted file mode 100644 index cc0edab6..00000000 --- a/packages/host/tests/test_hermes_fatal_handler.cpp +++ /dev/null @@ -1,51 +0,0 @@ -// Skipped on Windows, which has no fork(). -#if !defined(_WIN32) - -#include - -#include - -#include -#include -#include - -#include -#include - -using namespace callstack::react_native_node_api; - -TEST_CASE("handleHermesFatal logs the reason, then aborts") { - // Printf conversions must reach the log verbatim, not be interpreted. - const std::string reason = "FATAL ERROR: some_location 100% %s %n"; - - int pipeFds[2]; - REQUIRE(pipe(pipeFds) == 0); - fflush(nullptr); - - const pid_t pid = fork(); - REQUIRE(pid >= 0); - if (pid == 0) { - // abort() doesn't flush stdio, and the host-platform logger uses stdout. - setvbuf(stdout, nullptr, _IONBF, 0); - dup2(pipeFds[1], STDOUT_FILENO); - dup2(pipeFds[1], STDERR_FILENO); - handleHermesFatal(reason); - } - close(pipeFds[1]); - - std::string output; - char buffer[256]; - ssize_t count = 0; - while ((count = read(pipeFds[0], buffer, sizeof(buffer))) > 0) { - output.append(buffer, static_cast(count)); - } - close(pipeFds[0]); - - int status = 0; - REQUIRE(waitpid(pid, &status, 0) == pid); - REQUIRE(WIFSIGNALED(status)); - REQUIRE(WTERMSIG(status) == SIGABRT); - REQUIRE(output.find("Hermes fatal error: " + reason) != std::string::npos); -} - -#endif From 9634e4bf853d9497acb114e2646d8a7c7b59dfe5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Sat, 31 Oct 2026 22:05:49 +0100 Subject: [PATCH 4/4] docs: replace the removed RuntimeNodeApi.cpp in the label example Co-Authored-By: Claude Opus 5.5 (1M context) --- .claude/CLAUDE.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 31fda3c4..b97bcb1a 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -79,8 +79,8 @@ is exactly the case above. **Whenever you open a PR here:** | `Ferric 🦀` | Ferric Apple triplets build | `packages/ferric`, `packages/ferric-example`, or anything napi-rs/Cargo related | | `weak-node-api` | weak-node-api tests | `packages/weak-node-api` | -A PR can need more than one label — e.g. a change to `RuntimeNodeApi.cpp` -that alters buffer semantics warrants `Host 🏡` plus `Apple 🍎` and +A PR can need more than one label — e.g. a change to `HermesNapiHost.cpp` +that alters async work semantics warrants `Host 🏡` plus `Apple 🍎` and `Android 🤖`, since the actual behavior change can only be verified on a real device.