Emit soft error for warning

Summary:
This diff adds a default behavior for the unified logger on Android.

Added the call site in the CXXNativeModule.

Changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D30377767

fbshipit-source-id: 000014828f2f245dc9492e3617218895d9a33536

ReactAndroid/src/main/jni/react/jni/Android.mk

@@ -77,7 +77,7 @@ LOCAL_CFLAGS += -fexceptions -frtti -Wno-unused-lambda-capture
 LOCAL_LDLIBS += -landroid
 
 # The dynamic libraries (.so files) that this module depends on.
-LOCAL_SHARED_LIBRARIES := libreactnativeutilsjni libfolly_json libfb libfbjni libglog_init libyoga
+LOCAL_SHARED_LIBRARIES := libreactnativeutilsjni libfolly_json libfb libfbjni libglog_init libyoga logger
 
 # The static libraries (.a files) that this module depends on.
 LOCAL_STATIC_LIBRARIES := libreactnative libruntimeexecutor libcallinvokerholder
@@ -124,6 +124,7 @@ $(call import-module,yogajni)
 $(call import-module,cxxreact)
 $(call import-module,jsi)
 $(call import-module,jsiexecutor)
+$(call import-module,logger)
 $(call import-module,callinvoker)
 $(call import-module,reactperflogger)
 $(call import-module,hermes)

ReactAndroid/src/main/jni/react/jni/BUCK

@@ -67,6 +67,7 @@ rn_xplat_cxx_library(
         react_native_xplat_target("cxxreact:module"),
         react_native_xplat_target("jsinspector:jsinspector"),
         react_native_xplat_target("runtimeexecutor:runtimeexecutor"),
+        react_native_xplat_target("logger:logger"),
         react_native_xplat_dep("jsi:jsi"),
         FBJNI_TARGET,
     ]) if not IS_OSS_BUILD else [],

ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp

@@ -26,6 +26,9 @@
 #include <fb/log.h>
 #include <fbjni/ByteBuffer.h>
 #include <folly/dynamic.h>
+#include <glog/logging.h>
+
+#include <logger/react_native_log.h>
 
 #include "CxxModuleWrapper.h"
 #include "JNativeRunnable.h"
@@ -93,13 +96,8 @@ CatalystInstanceImpl::initHybrid(jni::alias_ref<jclass>) {
 CatalystInstanceImpl::CatalystInstanceImpl()
     : instance_(std::make_unique<Instance>()) {}
 
-void logSoftException(std::string message) {
-  JReactSoftExceptionLogger::logNoThrowSoftExceptionWithMessage(
-      "ReactNativeLogger#warning", message);
-}
-
 void CatalystInstanceImpl::warnOnLegacyNativeModuleSystemUse() {
-  CxxNativeModule::setWarnOnUsageLogger(&logSoftException);
+  CxxNativeModule::setShouldWarnOnUse(true);
 }
 
 void CatalystInstanceImpl::registerNatives() {
@@ -145,6 +143,25 @@ void CatalystInstanceImpl::registerNatives() {
   JNativeRunnable::registerNatives();
 }
 
+void log(ReactNativeLogLevel level, const char *message) {
+  switch (level) {
+    case ReactNativeLogLevelInfo:
+      LOG(INFO) << message;
+      break;
+    case ReactNativeLogLevelWarning:
+      LOG(WARNING) << message;
+      JReactSoftExceptionLogger::logNoThrowSoftExceptionWithMessage(
+          "react_native_log#warning", message);
+      break;
+    case ReactNativeLogLevelError:
+      LOG(ERROR) << message;
+      break;
+    case ReactNativeLogLevelFatal:
+      LOG(FATAL) << message;
+      break;
+  }
+}
+
 void CatalystInstanceImpl::initializeBridge(
     jni::alias_ref<ReactCallback::javaobject> callback,
     // This executor is actually a factory holder.
@@ -155,6 +172,8 @@ void CatalystInstanceImpl::initializeBridge(
         javaModules,
     jni::alias_ref<jni::JCollection<ModuleHolder::javaobject>::javaobject>
         cxxModules) {
+  set_react_native_logfunc(&log);
+
   // TODO mhorowitz: how to assert here?
   // Assertions.assertCondition(mBridge == null, "initializeBridge should be
   // called once");

ReactCommon/cxxreact/Android.mk

@@ -20,7 +20,7 @@ LOCAL_CFLAGS := \
 LOCAL_CFLAGS += -fexceptions -frtti -Wno-unused-lambda-capture
 
 LOCAL_STATIC_LIBRARIES := boost jsi callinvoker reactperflogger runtimeexecutor
-LOCAL_SHARED_LIBRARIES := jsinspector libfolly_json glog
+LOCAL_SHARED_LIBRARIES := jsinspector libfolly_json glog logger
 
 include $(BUILD_STATIC_LIBRARY)
 
@@ -34,3 +34,4 @@ $(call import-module,jsi)
 $(call import-module,jsinspector)
 $(call import-module,hermes/inspector)
 $(call import-module,hermes/executor)
+$(call import-module,logger)

ReactCommon/cxxreact/BUCK

@@ -152,6 +152,7 @@ rn_xplat_cxx_library(
         react_native_xplat_target("microprofiler:microprofiler"),
         react_native_xplat_target("runtimeexecutor:runtimeexecutor"),
         react_native_xplat_target("reactperflogger:reactperflogger"),
+        react_native_xplat_target("logger:logger"),
         "//third-party/glog:glog",
         "//xplat/folly:optional",
     ],

ReactCommon/cxxreact/CxxNativeModule.cpp

@@ -16,6 +16,8 @@
 #include "MessageQueueThread.h"
 #include "SystraceSection.h"
 
+#include <logger/react_native_log.h>
+
 using facebook::xplat::module::CxxModule;
 namespace facebook {
 namespace react {
@@ -54,10 +56,24 @@ CxxModule::Callback convertCallback(
 
 } // namespace
 
-WarnOnUsageLogger CxxNativeModule::warnOnUsageLogger_ = nullptr;
+bool CxxNativeModule::shouldWarnOnUse_ = false;
 
-void CxxNativeModule::setWarnOnUsageLogger(WarnOnUsageLogger logger) {
-  warnOnUsageLogger_ = logger;
+void CxxNativeModule::setShouldWarnOnUse(bool value) {
+  shouldWarnOnUse_ = value;
+}
+
+void CxxNativeModule::emitWarnIfWarnOnUsage(
+    const std::string &method_name,
+    const std::string &module_name) {
+  if (shouldWarnOnUse_) {
+    std::string message = folly::to<std::string>(
+        "Calling ",
+        method_name,
+        " on Cxx NativeModule (name = \"",
+        module_name,
+        "\").");
+    react_native_log_warn(message.c_str());
+  }
 }
 
 std::string CxxNativeModule::getName() {
@@ -93,11 +109,7 @@ folly::dynamic CxxNativeModule::getConstants() {
     return nullptr;
   }
 
-  if (warnOnUsageLogger_) {
-    warnOnUsageLogger_(
-        "Calling getConstants() on Cxx NativeModule (name = \"" + getName() +
-        "\").");
-  }
+  emitWarnIfWarnOnUsage("getConstants()", getName());
 
   folly::dynamic constants = folly::dynamic::object();
   for (auto &pair : module_->getConstants()) {
@@ -133,11 +145,7 @@ void CxxNativeModule::invoke(
         "Method ", method.name, " is synchronous but invoked asynchronously"));
   }
 
-  if (warnOnUsageLogger_) {
-    warnOnUsageLogger_(
-        "Calling " + method.name + "() on Cxx NativeModule (name = \"" +
-        getName() + "\").");
-  }
+  emitWarnIfWarnOnUsage(method.name, getName());
 
   if (params.size() < method.callbacks) {
     throw std::invalid_argument(folly::to<std::string>(
@@ -222,11 +230,7 @@ MethodCallResult CxxNativeModule::callSerializableNativeHook(
         "Method ", method.name, " is asynchronous but invoked synchronously"));
   }
 
-  if (warnOnUsageLogger_) {
-    warnOnUsageLogger_(
-        "Calling " + method.name + "() on Cxx NativeModule (name = \"" +
-        getName() + "\").");
-  }
+  emitWarnIfWarnOnUsage(method.name, getName());
 
   return method.syncFunc(std::move(args));
 }

ReactCommon/cxxreact/CxxNativeModule.h

@@ -48,7 +48,7 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
       unsigned int hookId,
       folly::dynamic &&args) override;
 
-  static void setWarnOnUsageLogger(WarnOnUsageLogger logger);
+  static void setShouldWarnOnUse(bool value);
 
  private:
   void lazyInit();
@@ -59,8 +59,11 @@ class RN_EXPORT CxxNativeModule : public NativeModule {
   std::shared_ptr<MessageQueueThread> messageQueueThread_;
   std::unique_ptr<xplat::module::CxxModule> module_;
   std::vector<xplat::module::CxxModule::Method> methods_;
+  void emitWarnIfWarnOnUsage(
+      const std::string &method_name,
+      const std::string &module_name);
 
-  static WarnOnUsageLogger warnOnUsageLogger_;
+  static bool shouldWarnOnUse_;
 };
 
 } // namespace react

ReactCommon/cxxreact/React-cxxreact.podspec

@@ -45,4 +45,5 @@ Pod::Spec.new do |s|
   s.dependency "React-runtimeexecutor", version
   s.dependency "React-perflogger", version
   s.dependency "React-jsi", version
+  s.dependency "React-logger", version
 end

packages/rn-tester/Podfile.lock

@@ -262,6 +262,7 @@ PODS:
     - React-callinvoker (= 1000.0.0)
     - React-jsi (= 1000.0.0)
     - React-jsinspector (= 1000.0.0)
+    - React-logger (= 1000.0.0)
     - React-perflogger (= 1000.0.0)
     - React-runtimeexecutor (= 1000.0.0)
   - React-Fabric (1000.0.0):
@@ -885,7 +886,7 @@ SPEC CHECKSUMS:
   React-callinvoker: c5d61e29df57793f0dc10ec2bc01c846f863e51f
   React-Core: 15d3fbb3cc863fa9990cc14c303a021cc66892a5
   React-CoreModules: 5ee1ed4f8b7f8bdbd45ed155a15c601dca9c73dd
-  React-cxxreact: 20a63475c83c5450442c754305738c6db6070214
+  React-cxxreact: 2fe718ab7094db2941ddaf35b2a44b8b57a7cece
   React-Fabric: 7641eab239c5fc5669ef22ea08cf383f9066fdcb
   React-graphics: db797c4609216593a1a12d1661716898d303900f
   React-jsi: 7cc3d3691803478047e7d2c8eb5d4c2f9c6d2922