Remove mapbuffer from early js error handling (#43951)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/43951

## Context
The **early js** error reporting pipeline catches javascript exceptions.

After errors are caught, the pipeline parses the exception, and puts the data into into a map buffer. 🤨

## Problems
We don't need to use a mapbuffer here: The structure of this exception data is known and never changes. (A map buffer is a type-unsafe bag of key/value pairs).

Instead, we could just use lower-level type-safe language primitives: regular C++ struct, and java class w/ fbjni.

## Changes
Migrate the **early js** error handling infra to C++ structs/fbjni.

## Impact
Now, there is no mapbuffer usage on iOS. We could re-introduce it when there is a need.

Changelog: [Internal]

Reviewed By: fkgozali

Differential Revision: D55265170

fbshipit-source-id: cda97633d4c6ccaad541e5d416067390fe6f61b2

packages/react-native/ReactAndroid/api/ReactAndroid.api

@@ -2830,6 +2830,20 @@ public abstract interface class com/facebook/react/interfaces/TaskInterface {
 	public abstract fun waitForCompletion (JLjava/util/concurrent/TimeUnit;)Z
 }
 
+public abstract interface class com/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedError {
+	public abstract fun getExceptionId ()I
+	public abstract fun getFrames ()Ljava/util/List;
+	public abstract fun getMessage ()Ljava/lang/String;
+	public abstract fun isFatal ()Z
+}
+
+public abstract interface class com/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedError$StackFrame {
+	public abstract fun getColumnNumber ()I
+	public abstract fun getFileName ()Ljava/lang/String;
+	public abstract fun getLineNumber ()I
+	public abstract fun getMethodName ()Ljava/lang/String;
+}
+
 public abstract interface class com/facebook/react/interfaces/fabric/ReactSurface {
 	public abstract fun clear ()V
 	public abstract fun detach ()V

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler.kt

@@ -9,10 +9,42 @@ package com.facebook.react.interfaces.exceptionmanager
 
 import com.facebook.proguard.annotations.DoNotStripAny
 import com.facebook.react.common.annotations.UnstableReactNativeAPI
-import com.facebook.react.common.mapbuffer.ReadableMapBuffer
+import java.util.ArrayList
 
 @DoNotStripAny
 @UnstableReactNativeAPI
 public fun interface ReactJsExceptionHandler {
-  public fun reportJsException(errorMap: ReadableMapBuffer?)
+  @DoNotStripAny
+  public interface ParsedError {
+    @DoNotStripAny
+    public interface StackFrame {
+      public val fileName: String
+      public val methodName: String
+      public val lineNumber: Int
+      public val columnNumber: Int
+    }
+
+    public val frames: List<StackFrame>
+    public val message: String
+    public val exceptionId: Int
+    public val isFatal: Boolean
+  }
+
+  @DoNotStripAny
+  private data class ParsedStackFrameImpl(
+      override val fileName: String,
+      override val methodName: String,
+      override val lineNumber: Int,
+      override val columnNumber: Int,
+  ) : ParsedError.StackFrame
+
+  @DoNotStripAny
+  private data class ParsedErrorImpl(
+      override val frames: ArrayList<ParsedStackFrameImpl>,
+      override val message: String,
+      override val exceptionId: Int,
+      override val isFatal: Boolean,
+  ) : ParsedError
+
+  public fun reportJsException(errorMap: ParsedError)
 }

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactExceptionManager.cpp

@@ -9,17 +9,57 @@
 #include <fbjni/fbjni.h>
 #include <glog/logging.h>
 #include <jni.h>
-#include <iostream>
 
 namespace facebook::react {
 
+namespace {
+class ParsedError : public facebook::jni::JavaClass<ParsedError> {
+ public:
+  static auto constexpr kJavaDescriptor =
+      "Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedError;";
+};
+
+class ParsedStackFrameImpl
+    : public facebook::jni::JavaClass<ParsedStackFrameImpl> {
+ public:
+  static auto constexpr kJavaDescriptor =
+      "Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedStackFrameImpl;";
+
+  static facebook::jni::local_ref<ParsedStackFrameImpl> create(
+      const JsErrorHandler::ParsedError::StackFrame& frame) {
+    return newInstance(
+        frame.fileName, frame.methodName, frame.lineNumber, frame.columnNumber);
+  }
+};
+
+class ParsedErrorImpl
+    : public facebook::jni::JavaClass<ParsedErrorImpl, ParsedError> {
+ public:
+  static auto constexpr kJavaDescriptor =
+      "Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedErrorImpl;";
+
+  static facebook::jni::local_ref<ParsedErrorImpl> create(
+      const JsErrorHandler::ParsedError& error) {
+    auto stackFrames =
+        facebook::jni::JArrayList<ParsedStackFrameImpl>::create();
+    for (const auto& frame : error.frames) {
+      stackFrames->add(ParsedStackFrameImpl::create(frame));
+    }
+
+    return newInstance(
+        stackFrames, error.message, error.exceptionId, error.isFatal);
+  }
+};
+
+} // namespace
+
 void JReactExceptionManager::reportJsException(
-    const JReadableMapBuffer::javaobject errorMapBuffer) {
+    const JsErrorHandler::ParsedError& error) {
   static const auto method =
-      javaClassStatic()->getMethod<void(JReadableMapBuffer::javaobject)>(
+      javaClassStatic()->getMethod<void(jni::alias_ref<ParsedError>)>(
           "reportJsException");
   if (self() != nullptr) {
-    method(self(), errorMapBuffer);
+    method(self(), ParsedErrorImpl::create(error));
   }
 }
 

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactExceptionManager.h

@@ -9,7 +9,7 @@
 
 #include <fbjni/fbjni.h>
 #include <jni.h>
-#include <react/common/mapbuffer/JReadableMapBuffer.h>
+#include <jserrorhandler/JsErrorHandler.h>
 
 namespace facebook::react {
 
@@ -19,7 +19,7 @@ class JReactExceptionManager
   static auto constexpr kJavaDescriptor =
       "Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;";
 
-  void reportJsException(const JReadableMapBuffer::javaobject errorMapBuffer);
+  void reportJsException(const JsErrorHandler::ParsedError& error);
 };
 
 } // namespace facebook::react

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactInstance.cpp

@@ -21,7 +21,6 @@
 #include <react/common/mapbuffer/JReadableMapBuffer.h>
 #include <react/jni/JRuntimeExecutor.h>
 #include <react/jni/JSLogging.h>
-#include <react/renderer/mapbuffer/MapBuffer.h>
 #include <react/runtime/BridgelessJSCallInvoker.h>
 #include <react/runtime/BridgelessNativeMethodCallInvoker.h>
 #include "JavaTimerRegistry.h"
@@ -52,13 +51,14 @@ JReactInstance::JReactInstance(
   jsTimerExecutor->cthis()->setTimerManager(timerManager);
 
   jReactExceptionManager_ = jni::make_global(jReactExceptionManager);
-  auto jsErrorHandlingFunc = [this](MapBuffer errorMap) noexcept {
-    if (jReactExceptionManager_ != nullptr) {
-      auto jErrorMap =
-          JReadableMapBuffer::createWithContents(std::move(errorMap));
-      jReactExceptionManager_->reportJsException(jErrorMap.get());
-    }
-  };
+  auto jsErrorHandlingFunc =
+      [weakJReactExceptionManager = jni::make_weak(jReactExceptionManager)](
+          const JsErrorHandler::ParsedError& error) mutable noexcept {
+        if (auto jReactExceptionManager =
+                weakJReactExceptionManager.lockLocal()) {
+          jReactExceptionManager->reportJsException(error);
+        }
+      };
 
   jBindingsInstaller_ = jni::make_global(jBindingsInstaller);
 

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp

@@ -6,7 +6,6 @@
  */
 
 #include "JsErrorHandler.h"
-#include <react/renderer/mapbuffer/MapBufferBuilder.h>
 #include <regex>
 #include <sstream>
 #include <string>
@@ -14,9 +13,7 @@
 
 namespace facebook::react {
 
-using facebook::react::JSErrorHandlerKey;
-
-static MapBuffer
+static JsErrorHandler::ParsedError
 parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
   /**
    * This parses the different stack traces and puts them into one format
@@ -43,63 +40,67 @@ parseErrorStack(const jsi::JSError& error, bool isFatal, bool isHermes) {
   std::string line;
   std::stringstream strStream(error.getStack());
 
-  auto errorObj = MapBufferBuilder();
-  std::vector<MapBuffer> frames;
+  std::vector<JsErrorHandler::ParsedError::StackFrame> frames;
 
   while (std::getline(strStream, line, '\n')) {
-    auto frame = MapBufferBuilder();
     auto searchResults = std::smatch{};
 
     if (isHermes) {
       if (std::regex_search(line, searchResults, REGEX_HERMES)) {
         std::string str2 = std::string(searchResults[2]);
         if (str2.compare("native")) {
-          frame.putString(kFrameFileName, std::string(searchResults[4]));
-          frame.putString(kFrameMethodName, std::string(searchResults[1]));
-          frame.putInt(kFrameLineNumber, std::stoi(searchResults[5]));
-          frame.putInt(kFrameColumnNumber, std::stoi(searchResults[6]));
-          frames.push_back(frame.build());
+          frames.push_back({
+              .fileName = std::string(searchResults[4]),
+              .methodName = std::string(searchResults[1]),
+              .lineNumber = std::stoi(searchResults[5]),
+              .columnNumber = std::stoi(searchResults[6]),
+          });
         }
       }
     } else {
       if (std::regex_search(line, searchResults, REGEX_GECKO)) {
-        frame.putString(kFrameFileName, std::string(searchResults[3]));
-        frame.putString(kFrameMethodName, std::string(searchResults[1]));
-        frame.putInt(kFrameLineNumber, std::stoi(searchResults[4]));
-        frame.putInt(kFrameColumnNumber, std::stoi(searchResults[5]));
+        frames.push_back({
+            .fileName = std::string(searchResults[3]),
+            .methodName = std::string(searchResults[1]),
+            .lineNumber = std::stoi(searchResults[4]),
+            .columnNumber = std::stoi(searchResults[5]),
+        });
       } else if (
           std::regex_search(line, searchResults, REGEX_CHROME) ||
           std::regex_search(line, searchResults, REGEX_NODE)) {
-        frame.putString(kFrameFileName, std::string(searchResults[2]));
-        frame.putString(kFrameMethodName, std::string(searchResults[1]));
-        frame.putInt(kFrameLineNumber, std::stoi(searchResults[3]));
-        frame.putInt(kFrameColumnNumber, std::stoi(searchResults[4]));
+        frames.push_back({
+            .fileName = std::string(searchResults[2]),
+            .methodName = std::string(searchResults[1]),
+            .lineNumber = std::stoi(searchResults[3]),
+            .columnNumber = std::stoi(searchResults[4]),
+        });
       } else {
         continue;
       }
-      frames.push_back(frame.build());
     }
   }
-  errorObj.putMapBufferList(kAllStackFrames, std::move(frames));
-  errorObj.putString(kErrorMessage, "EarlyJsError: " + error.getMessage());
-  // TODO: If needed, can increment exceptionId by 1 each time
-  errorObj.putInt(kExceptionId, 0);
-  errorObj.putBool(kIsFatal, isFatal);
-  return errorObj.build();
+
+  return {
+      .frames = std::move(frames),
+      .message = "EarlyJsError: " + error.getMessage(),
+      .exceptionId = 0,
+      .isFatal = isFatal,
+  };
 }
 
 JsErrorHandler::JsErrorHandler(
-    JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc) {
-  this->_jsErrorHandlingFunc = jsErrorHandlingFunc;
-};
+    JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc)
+    : _jsErrorHandlingFunc(std::move(jsErrorHandlingFunc)){
+
+      };
 
 JsErrorHandler::~JsErrorHandler() {}
 
 void JsErrorHandler::handleJsError(const jsi::JSError& error, bool isFatal) {
   // TODO: Current error parsing works and is stable. Can investigate using
   // REGEX_HERMES to get additional Hermes data, though it requires JS setup.
-  MapBuffer errorMap = parseErrorStack(error, isFatal, false);
-  _jsErrorHandlingFunc(std::move(errorMap));
+  ParsedError parsedError = parseErrorStack(error, isFatal, false);
+  _jsErrorHandlingFunc(parsedError);
 }
 
 } // namespace facebook::react

packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h

@@ -8,26 +8,28 @@
 #pragma once
 
 #include <jsi/jsi.h>
-#include <react/renderer/mapbuffer/MapBuffer.h>
 
 namespace facebook::react {
 
-enum JSErrorHandlerKey : uint16_t {
-  kFrameFileName = 0,
-  kFrameMethodName = 1,
-  kFrameLineNumber = 2,
-  kFrameColumnNumber = 3,
-  kAllStackFrames = 4,
-  kErrorMessage = 5,
-  kExceptionId = 6,
-  kIsFatal = 7
-};
-
 class JsErrorHandler {
  public:
-  using JsErrorHandlingFunc = std::function<void(MapBuffer errorMap)>;
+  struct ParsedError {
+    struct StackFrame {
+      std::string fileName;
+      std::string methodName;
+      int lineNumber;
+      int columnNumber;
+    };
 
-  JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc);
+    std::vector<StackFrame> frames;
+    std::string message;
+    int exceptionId;
+    bool isFatal;
+  };
+
+  using JsErrorHandlingFunc = std::function<void(const ParsedError& error)>;
+
+  explicit JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc);
   ~JsErrorHandler();
 
   void handleJsError(const jsi::JSError& error, bool isFatal);

packages/react-native/ReactCommon/jserrorhandler/React-jserrorhandler.podspec

@@ -48,6 +48,5 @@ Pod::Spec.new do |s|
   s.dependency folly_dep_name, folly_version
   s.dependency "React-jsi"
   add_dependency(s, "React-debug")
-  add_dependency(s, "React-Mapbuffer")
 
 end

packages/react-native/ReactCommon/jsinspector-modern/tests/ReactInstanceIntegrationTest.cpp

@@ -34,12 +34,22 @@ void ReactInstanceIntegrationTest::SetUp() {
   auto timerManager =
       std::make_shared<react::TimerManager>(std::move(mockRegistry));
 
-  auto jsErrorHandlingFunc = [](react::MapBuffer error) noexcept {
-    LOG(INFO) << "Error: \nFile: " << error.getString(react::kFrameFileName)
-              << "\nLine: " << error.getInt(react::kFrameLineNumber)
-              << "\nColumn: " << error.getInt(react::kFrameColumnNumber)
-              << "\nMethod: " << error.getString(react::kFrameMethodName);
-  };
+  auto jsErrorHandlingFunc =
+      [](const JsErrorHandler::ParsedError& errorMap) noexcept {
+        LOG(INFO) << "[jsErrorHandlingFunc called]";
+        LOG(INFO) << "message: " << errorMap.message;
+        LOG(INFO) << "exceptionId: " << std::to_string(errorMap.exceptionId);
+        LOG(INFO) << "isFatal: "
+                  << std::to_string(static_cast<int>(errorMap.isFatal));
+        auto frames = errorMap.frames;
+        for (const auto& mapBuffer : frames) {
+          LOG(INFO) << "[Frame]" << std::endl
+                    << "\tfile: " << mapBuffer.fileName;
+          LOG(INFO) << "\tmethodName: " << mapBuffer.methodName;
+          LOG(INFO) << "\tlineNumber: " << std::to_string(mapBuffer.lineNumber);
+          LOG(INFO) << "\tcolumn: " << std::to_string(mapBuffer.columnNumber);
+        }
+      };
 
   auto jsRuntimeFactory = std::make_unique<react::HermesInstance>();
   std::unique_ptr<react::JSRuntime> runtime_ =

packages/react-native/ReactCommon/jsinspector-modern/tests/ReactNativeMocks.cpp

@@ -6,6 +6,7 @@
  */
 
 #include "ReactNativeMocks.h"
+#include <glog/logging.h>
 
 namespace facebook::react::jsinspector_modern {
 

packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp

@@ -23,7 +23,6 @@
 #include <react/utils/jsi-utils.h>
 #include <iostream>
 #include <memory>
-#include <tuple>
 #include <utility>
 
 namespace facebook::react {
@@ -37,7 +36,7 @@ ReactInstance::ReactInstance(
     : runtime_(std::move(runtime)),
       jsMessageQueueThread_(jsMessageQueueThread),
       timerManager_(std::move(timerManager)),
-      jsErrorHandler_(jsErrorHandlingFunc),
+      jsErrorHandler_(std::move(jsErrorHandlingFunc)),
       hasFatalJsError_(std::make_shared<bool>(false)),
       parentInspectorTarget_(parentInspectorTarget) {
   RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_),

packages/react-native/ReactCommon/react/runtime/ReactInstance.h

@@ -34,7 +34,7 @@ class ReactInstance final : private jsinspector_modern::InstanceTargetDelegate {
       std::unique_ptr<JSRuntime> runtime,
       std::shared_ptr<MessageQueueThread> jsMessageQueueThread,
       std::shared_ptr<TimerManager> timerManager,
-      JsErrorHandler::JsErrorHandlingFunc JsErrorHandlingFunc,
+      JsErrorHandler::JsErrorHandlingFunc jsErrorHandlingFunc,
       jsinspector_modern::HostTarget* parentInspectorTarget = nullptr);
 
   RuntimeExecutor getUnbufferedRuntimeExecutor() noexcept;

packages/react-native/ReactCommon/react/runtime/platform/ios/ReactCommon/RCTInstance.mm

@@ -245,7 +245,7 @@ void RCTInstanceSetRuntimeDiagnosticFlags(NSString *flags)
   objCTimerRegistryRawPtr->setTimerManager(timerManager);
 
   __weak __typeof(self) weakSelf = self;
-  auto jsErrorHandlingFunc = [=](MapBuffer errorMap) { [weakSelf _handleJSErrorMap:std::move(errorMap)]; };
+  auto jsErrorHandlingFunc = [=](const JsErrorHandler::ParsedError &error) { [weakSelf _handleJSError:error]; };
 
   // Create the React Instance
   _reactInstance = std::make_unique<ReactInstance>(
@@ -496,28 +496,23 @@ void RCTInstanceSetRuntimeDiagnosticFlags(NSString *flags)
   }
 }
 
-- (void)_handleJSErrorMap:(facebook::react::MapBuffer)errorMap
+- (void)_handleJSError:(const JsErrorHandler::ParsedError &)error
 {
-  NSString *message = [NSString stringWithCString:errorMap.getString(JSErrorHandlerKey::kErrorMessage).c_str()
-                                         encoding:[NSString defaultCStringEncoding]];
-  std::vector<facebook::react::MapBuffer> frames = errorMap.getMapBufferList(JSErrorHandlerKey::kAllStackFrames);
+  NSString *message = [NSString stringWithCString:error.message.c_str() encoding:[NSString defaultCStringEncoding]];
   NSMutableArray<NSDictionary<NSString *, id> *> *stack = [NSMutableArray new];
-  for (const facebook::react::MapBuffer &mapBuffer : frames) {
-    NSDictionary *frame = @{
-      @"file" : [NSString stringWithCString:mapBuffer.getString(JSErrorHandlerKey::kFrameFileName).c_str()
-                                   encoding:[NSString defaultCStringEncoding]],
-      @"methodName" : [NSString stringWithCString:mapBuffer.getString(JSErrorHandlerKey::kFrameMethodName).c_str()
-                                         encoding:[NSString defaultCStringEncoding]],
-      @"lineNumber" : [NSNumber numberWithInt:mapBuffer.getInt(JSErrorHandlerKey::kFrameLineNumber)],
-      @"column" : [NSNumber numberWithInt:mapBuffer.getInt(JSErrorHandlerKey::kFrameColumnNumber)],
-    };
-    [stack addObject:frame];
+  for (const JsErrorHandler::ParsedError::StackFrame &frame : error.frames) {
+    [stack addObject:@{
+      @"file" : [NSString stringWithCString:frame.fileName.c_str() encoding:[NSString defaultCStringEncoding]],
+      @"methodName" : [NSString stringWithCString:frame.methodName.c_str() encoding:[NSString defaultCStringEncoding]],
+      @"lineNumber" : [NSNumber numberWithInt:frame.lineNumber],
+      @"column" : [NSNumber numberWithInt:frame.columnNumber],
+    }];
   }
   [_delegate instance:self
       didReceiveJSErrorStack:stack
                      message:message
-                 exceptionId:errorMap.getInt(JSErrorHandlerKey::kExceptionId)
-                     isFatal:errorMap.getBool(JSErrorHandlerKey::kIsFatal)];
+                 exceptionId:error.exceptionId
+                     isFatal:error.isFatal];
 }
 
 - (void)didReceiveReloadCommand

packages/react-native/ReactCommon/react/runtime/tests/cxx/ReactInstanceTest.cpp

@@ -13,8 +13,8 @@
 
 #include <ReactCommon/RuntimeExecutor.h>
 #include <hermes/hermes.h>
+#include <jserrorhandler/JsErrorHandler.h>
 #include <jsi/jsi.h>
-#include <react/renderer/mapbuffer/MapBuffer.h>
 #include <react/runtime/ReactInstance.h>
 
 using ::testing::_;
@@ -123,9 +123,10 @@ class ReactInstanceTest : public ::testing::Test {
     auto mockRegistry = std::make_unique<MockTimerRegistry>();
     mockRegistry_ = mockRegistry.get();
     timerManager_ = std::make_shared<TimerManager>(std::move(mockRegistry));
-    auto jsErrorHandlingFunc = [](MapBuffer errorMap) noexcept {
-      // Do nothing
-    };
+    auto jsErrorHandlingFunc =
+        [](const JsErrorHandler::ParsedError& errorMap) noexcept {
+          // Do nothing
+        };
     instance_ = std::make_unique<ReactInstance>(
         std::move(runtime),
         messageQueueThread_,