From e37d2613858049c9f69c1fab8e247e48ccb3f43f Mon Sep 17 00:00:00 2001 From: Marc Rousavy Date: Mon, 11 Nov 2024 17:38:09 +0100 Subject: [PATCH] feat: Explicitly catch `ClassNotFound` and `NoSuchMethod` errors (#318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Explicitly catch `ClassNotFound` errors * feat: Also check for NoSuchMethodError * chore: Use tick ` * Adjust message * feat: Move JNI lookup code to `AutolinkedHybridObject` to avoid spaghetti (#319) * feat: Move JNI lookup code to `AutolinkedHybridObject` * fix: Extract name & format * fix: Remove null check from generated code * Fix typename * whoops * Update AutolinkedHybridObject.hpp * Update AutolinkedHybridObject.hpp * hä geht eh so oida * `AutolinkedHybridObject` -> `ConstructableHybridObject` --- .../kotlin/KotlinHybridObjectRegistration.ts | 16 ++-- .../generated/android/NitroImageOnLoad.cpp | 45 +++-------- .../registry/DefaultConstructableObject.hpp | 79 +++++++++++++++++++ 3 files changed, 95 insertions(+), 45 deletions(-) create mode 100644 packages/react-native-nitro-modules/android/src/main/cpp/registry/DefaultConstructableObject.hpp diff --git a/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts b/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts index 08f9559f1..8c664ecd1 100644 --- a/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts +++ b/packages/nitrogen/src/syntax/kotlin/KotlinHybridObjectRegistration.ts @@ -33,20 +33,18 @@ export function createJNIHybridObjectRegistration({ language: 'c++', space: 'system', }, + { + name: 'NitroModules/DefaultConstructableObject.hpp', + language: 'c++', + space: 'system', + }, ], cppCode: ` HybridObjectRegistry::registerHybridObjectConstructor( "${hybridObjectName}", []() -> std::shared_ptr { - static auto javaClass = jni::findClassStatic("${jniNamespace}"); - static auto defaultConstructor = javaClass->getConstructor<${JHybridTSpec}::javaobject()>(); - - auto instance = javaClass->newObject(defaultConstructor); -#ifdef NITRO_DEBUG - if (instance == nullptr) [[unlikely]] { - throw std::runtime_error("Failed to create an instance of \\"${JHybridTSpec}\\" - the constructor returned null!"); - } -#endif + static DefaultConstructableObject<${JHybridTSpec}::javaobject> object("${jniNamespace}"); + auto instance = object.create(); auto globalRef = jni::make_global(instance); return JNISharedPtr::make_shared_from_jni<${JHybridTSpec}>(globalRef); } diff --git a/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp b/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp index 1a1a30fa7..e9406c02a 100644 --- a/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp +++ b/packages/react-native-nitro-image/nitrogen/generated/android/NitroImageOnLoad.cpp @@ -21,6 +21,7 @@ #include "JHybridBaseSpec.hpp" #include "JHybridChildSpec.hpp" #include +#include #include "HybridTestObjectCpp.hpp" namespace margelo::nitro::image { @@ -49,15 +50,8 @@ int initialize(JavaVM* vm) { HybridObjectRegistry::registerHybridObjectConstructor( "ImageFactory", []() -> std::shared_ptr { - static auto javaClass = jni::findClassStatic("com/margelo/nitro/image/ImageFactory"); - static auto defaultConstructor = javaClass->getConstructor(); - - auto instance = javaClass->newObject(defaultConstructor); - #ifdef NITRO_DEBUG - if (instance == nullptr) [[unlikely]] { - throw std::runtime_error("Failed to create an instance of \"JHybridImageFactorySpec\" - the constructor returned null!"); - } - #endif + static DefaultConstructableObject object("com/margelo/nitro/image/ImageFactory"); + auto instance = object.create(); auto globalRef = jni::make_global(instance); return JNISharedPtr::make_shared_from_jni(globalRef); } @@ -74,15 +68,8 @@ int initialize(JavaVM* vm) { HybridObjectRegistry::registerHybridObjectConstructor( "TestObjectSwiftKotlin", []() -> std::shared_ptr { - static auto javaClass = jni::findClassStatic("com/margelo/nitro/image/HybridTestObjectKotlin"); - static auto defaultConstructor = javaClass->getConstructor(); - - auto instance = javaClass->newObject(defaultConstructor); - #ifdef NITRO_DEBUG - if (instance == nullptr) [[unlikely]] { - throw std::runtime_error("Failed to create an instance of \"JHybridTestObjectSwiftKotlinSpec\" - the constructor returned null!"); - } - #endif + static DefaultConstructableObject object("com/margelo/nitro/image/HybridTestObjectKotlin"); + auto instance = object.create(); auto globalRef = jni::make_global(instance); return JNISharedPtr::make_shared_from_jni(globalRef); } @@ -90,15 +77,8 @@ int initialize(JavaVM* vm) { HybridObjectRegistry::registerHybridObjectConstructor( "Base", []() -> std::shared_ptr { - static auto javaClass = jni::findClassStatic("com/margelo/nitro/image/HybridBase"); - static auto defaultConstructor = javaClass->getConstructor(); - - auto instance = javaClass->newObject(defaultConstructor); - #ifdef NITRO_DEBUG - if (instance == nullptr) [[unlikely]] { - throw std::runtime_error("Failed to create an instance of \"JHybridBaseSpec\" - the constructor returned null!"); - } - #endif + static DefaultConstructableObject object("com/margelo/nitro/image/HybridBase"); + auto instance = object.create(); auto globalRef = jni::make_global(instance); return JNISharedPtr::make_shared_from_jni(globalRef); } @@ -106,15 +86,8 @@ int initialize(JavaVM* vm) { HybridObjectRegistry::registerHybridObjectConstructor( "Child", []() -> std::shared_ptr { - static auto javaClass = jni::findClassStatic("com/margelo/nitro/image/HybridChild"); - static auto defaultConstructor = javaClass->getConstructor(); - - auto instance = javaClass->newObject(defaultConstructor); - #ifdef NITRO_DEBUG - if (instance == nullptr) [[unlikely]] { - throw std::runtime_error("Failed to create an instance of \"JHybridChildSpec\" - the constructor returned null!"); - } - #endif + static DefaultConstructableObject object("com/margelo/nitro/image/HybridChild"); + auto instance = object.create(); auto globalRef = jni::make_global(instance); return JNISharedPtr::make_shared_from_jni(globalRef); } diff --git a/packages/react-native-nitro-modules/android/src/main/cpp/registry/DefaultConstructableObject.hpp b/packages/react-native-nitro-modules/android/src/main/cpp/registry/DefaultConstructableObject.hpp new file mode 100644 index 000000000..959bd8148 --- /dev/null +++ b/packages/react-native-nitro-modules/android/src/main/cpp/registry/DefaultConstructableObject.hpp @@ -0,0 +1,79 @@ +// +// DefaultConstructableObject.hpp +// react-native-nitro +// +// Created by Marc Rousavy on 11.11.24. +// + +#pragma once + +#include "NitroDefines.hpp" +#include + +namespace margelo::nitro { + +using namespace facebook; + +template +class DefaultConstructableObject { +public: + explicit DefaultConstructableObject(const char* javaClassDescriptor) { + try { + // Find JNI class and default constructor + _javaClass = jni::findClassStatic(javaClassDescriptor); + _defaultConstructor = _javaClass->getConstructor(); + } catch (const jni::JniException& exc) { + std::string message = exc.what(); + std::string descriptor = javaClassDescriptor; + std::string className = findClassName(descriptor); + if (message.find("ClassNotFoundException")) { + // Java class cannot be found + throw std::runtime_error( + "Couldn't find class `" + descriptor + + "`!\n" + "- Make sure the class exists in the specified namespace.\n" + "- Make sure the class is not stripped. If you are using ProGuard, add `@Keep` and `@DoNotStrip` annotations to `" + + className + "`."); + } else if (message.find("NoSuchMethodError")) { + // Default Constructor cannot be found + throw std::runtime_error( + "Couldn't find " + className + + "'s default constructor!\n" + "- If you want to autolink " + + className + + ", add a default constructor that takes zero arguments.\n" + "- If you need arguments to create instances of " + + className + + ", create a separate HybridObject that acts as a factory for this HybridObject to create instances of it with parameters.\n" + "- If you already have a default constructor, make sure it is not being stripped. If you are using ProGuard, add `@Keep` and " + "`@DoNotStrip` annotations to the default constructor."); + } else { + throw; + } + } + } + +public: + jni::local_ref create() const { + // Calls the class's default constructor + auto instance = _javaClass->newObject(_defaultConstructor); +#ifdef NITRO_DEBUG + if (instance == nullptr) [[unlikely]] { + throw std::runtime_error("Failed to create an instance of \"JHybridTestObjectSwiftKotlinSpec\" - the constructor returned null!"); + } +#endif + return instance; + } + +private: + static std::string findClassName(const std::string& jniDescriptor) { + size_t lastSlash = jniDescriptor.rfind('/'); + return jniDescriptor.substr(lastSlash + 1); + } + +private: + jni::alias_ref _javaClass; + jni::JConstructor _defaultConstructor; +}; + +} // namespace margelo::nitro