Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mapbuffer from early js error handling #43951

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include <fbjni/fbjni.h>
#include <jni.h>
#include <react/common/mapbuffer/JReadableMapBuffer.h>
#include <jserrorhandler/JsErrorHandler.h>

namespace facebook::react {

Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand Down
65 changes: 33 additions & 32 deletions packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,14 @@
*/

#include "JsErrorHandler.h"
#include <react/renderer/mapbuffer/MapBufferBuilder.h>
#include <regex>
#include <sstream>
#include <string>
#include <vector>

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
Expand All @@ -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
32 changes: 17 additions & 15 deletions packages/react-native/ReactCommon/jserrorhandler/JsErrorHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)>;

JsErrorHandler(JsErrorHandlingFunc jsErrorHandlingFunc);
struct ParsedError {
struct StackFrame {
std::string fileName;
std::string methodName;
int lineNumber;
int columnNumber;
};

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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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_ =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

#include "ReactNativeMocks.h"
#include <glog/logging.h>

namespace facebook::react::jsinspector_modern {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <react/utils/jsi-utils.h>
#include <iostream>
#include <memory>
#include <tuple>
#include <utility>

namespace facebook::react {
Expand All @@ -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_),
Expand Down
Loading
Loading