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/.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. diff --git a/packages/host/android/CMakeLists.txt b/packages/host/android/CMakeLists.txt index 2a45e96d..95032383 100644 --- a/packages/host/android/CMakeLists.txt +++ b/packages/host/android/CMakeLists.txt @@ -12,8 +12,6 @@ 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 ) 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 0440027c..00000000 --- a/packages/host/cpp/RuntimeNodeApi.hpp +++ /dev/null @@ -1,32 +0,0 @@ -#pragma once - -#include "node_api.h" - -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 -// `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. -// Do not remove this one the same way without replacing the logcat routing. -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"