Skip to content

Commit

Permalink
easy: Align earlyjs c++ native data structures with js
Browse files Browse the repository at this point in the history
Summary:
For the js error handling pipeline, the javascript data structure looks like [this](https://www.internalfb.com/code/fbsource/[6181b57f4ba3619f58056bcec65382650d6ff59a]/xplat/js/react-native-github/packages/react-native/src/private/specs/modules/NativeExceptionsManager.js?lines=17-35):

```
export type StackFrame = {|
  column: ?number,
  file: ?string,
  lineNumber: ?number,
  methodName: string,
  collapse?: boolean,
|};
export type ExceptionData = {
  message: string,
  originalMessage: ?string,
  name: ?string,
  componentStack: ?string,
  stack: Array<StackFrame>,
  id: number,
  isFatal: boolean,
  // flowlint-next-line unclear-type:off
  extraData?: Object,
  ...
};
```

So, I made the c++ data structure look similar
```
  struct ParsedError {
    struct StackFrame {
      std::optional<std::string> file;
      std::string methodName;
      std::optional<int> lineNumber;
      std::optional<int> column;
    };

    std::string message;
    std::optional<std::string> originalMessage;
    std::optional<std::string> name;
    std::optional<std::string> componentStack;
    std::vector<StackFrame> stack;
    int id;
    bool isFatal;
    jsi::Object extraData;
  };
```

Notes:
* [parseErrorStack](https://fburl.com/code/e27q9gkc) doesn't actually generate a collapse property on the error object. So, I omitted it from the c++.
* ExceptionsManager [always provides an extraData field](https://fburl.com/code/2bvcsxac). So, I made it required.
* In C++, I just stored extraData as a jsi::Object. I wanted the freedom to store arbitrary key/value pairs. But, I also didn't want to use folly::dynamic.

Changelog: [Internal]

Reviewed By: alanleedev

Differential Revision: D63929580
  • Loading branch information
RSNara authored and facebook-github-bot committed Oct 9, 2024
1 parent 7fb3d83 commit 6e329ae
Show file tree
Hide file tree
Showing 17 changed files with 258 additions and 81 deletions.
4 changes: 4 additions & 0 deletions packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,12 @@ - (void)hostDidStart:(RCTHost *)host
- (void)host:(RCTHost *)host
didReceiveJSErrorStack:(NSArray<NSDictionary<NSString *, id> *> *)stack
message:(NSString *)message
originalMessage:(NSString *_Nullable)originalMessage
name:(NSString *_Nullable)name
componentStack:(NSString *_Nullable)componentStack
exceptionId:(NSUInteger)exceptionId
isFatal:(BOOL)isFatal
extraData:(NSDictionary<NSString *, id> *)extraData
{
}

Expand Down
18 changes: 13 additions & 5 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -2396,12 +2396,16 @@ public class com/facebook/react/devsupport/ReleaseDevSupportManager : com/facebo

public class com/facebook/react/devsupport/StackTraceHelper {
public static final field COLUMN_KEY Ljava/lang/String;
public static final field COMPONENT_STACK_KEY Ljava/lang/String;
public static final field EXTRA_DATA_KEY Ljava/lang/String;
public static final field FILE_KEY Ljava/lang/String;
public static final field ID_KEY Ljava/lang/String;
public static final field IS_FATAL_KEY Ljava/lang/String;
public static final field LINE_NUMBER_KEY Ljava/lang/String;
public static final field MESSAGE_KEY Ljava/lang/String;
public static final field METHOD_NAME_KEY Ljava/lang/String;
public static final field NAME_KEY Ljava/lang/String;
public static final field ORIGINAL_MESSAGE_KEY Ljava/lang/String;
public static final field STACK_KEY Ljava/lang/String;
public fun <init> ()V
public static fun convertJavaStackTrace (Ljava/lang/Throwable;)[Lcom/facebook/react/devsupport/interfaces/StackFrame;
Expand Down Expand Up @@ -2893,16 +2897,20 @@ public abstract interface class com/facebook/react/interfaces/TaskInterface {
}

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 getComponentStack ()Ljava/lang/String;
public abstract fun getExtraData ()Lcom/facebook/react/bridge/ReadableMap;
public abstract fun getId ()I
public abstract fun getMessage ()Ljava/lang/String;
public abstract fun getName ()Ljava/lang/String;
public abstract fun getOriginalMessage ()Ljava/lang/String;
public abstract fun getStack ()Ljava/util/List;
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 getColumn ()Ljava/lang/Integer;
public abstract fun getFile ()Ljava/lang/String;
public abstract fun getLineNumber ()Ljava/lang/Integer;
public abstract fun getMethodName ()Ljava/lang/String;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,13 @@ public class StackTraceHelper {
public static final String METHOD_NAME_KEY = "methodName";

public static final String MESSAGE_KEY = "message";
public static final String ORIGINAL_MESSAGE_KEY = "originalMessage";
public static final String NAME_KEY = "name";
public static final String COMPONENT_STACK_KEY = "componentStack";
public static final String STACK_KEY = "stack";
public static final String ID_KEY = "id";
public static final String IS_FATAL_KEY = "isFatal";
public static final String EXTRA_DATA_KEY = "extraData";

private static final Pattern STACK_FRAME_PATTERN1 =
Pattern.compile("^(?:(.*?)@)?(.*?)\\:([0-9]+)\\:([0-9]+)$");
Expand Down Expand Up @@ -260,22 +264,32 @@ public static String formatStackTrace(String title, StackFrame[] stack) {
}

public static JavaOnlyMap convertParsedError(ParsedError error) {
List<ParsedError.StackFrame> frames = error.getFrames();
List<ParsedError.StackFrame> frames = error.getStack();
List<ReadableMap> readableMapList = new ArrayList<>();
for (ParsedError.StackFrame frame : frames) {
JavaOnlyMap map = new JavaOnlyMap();
map.putDouble(COLUMN_KEY, frame.getColumnNumber());
map.putDouble(COLUMN_KEY, frame.getColumn());
map.putDouble(LINE_NUMBER_KEY, frame.getLineNumber());
map.putString(FILE_KEY, (String) frame.getFileName());
map.putString(FILE_KEY, (String) frame.getFile());
map.putString(METHOD_NAME_KEY, (String) frame.getMethodName());
readableMapList.add(map);
}

JavaOnlyMap data = new JavaOnlyMap();
data.putString(MESSAGE_KEY, error.getMessage());
if (error.getOriginalMessage() != null) {
data.putString(ORIGINAL_MESSAGE_KEY, error.getOriginalMessage());
}
if (error.getName() != null) {
data.putString(NAME_KEY, error.getName());
}
if (error.getComponentStack() != null) {
data.putString(COMPONENT_STACK_KEY, error.getComponentStack());
}
data.putArray(STACK_KEY, JavaOnlyArray.from(readableMapList));
data.putInt(ID_KEY, error.getExceptionId());
data.putInt(ID_KEY, error.getId());
data.putBoolean(IS_FATAL_KEY, error.isFatal());
data.putMap(EXTRA_DATA_KEY, error.getExtraData());

return data;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
package com.facebook.react.interfaces.exceptionmanager

import com.facebook.proguard.annotations.DoNotStripAny
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.bridge.ReadableNativeMap
import com.facebook.react.common.annotations.UnstableReactNativeAPI
import java.util.ArrayList

Expand All @@ -18,32 +20,40 @@ public fun interface ReactJsExceptionHandler {
public interface ParsedError {
@DoNotStripAny
public interface StackFrame {
public val fileName: String
public val file: String?
public val methodName: String
public val lineNumber: Int
public val columnNumber: Int
public val lineNumber: Int?
public val column: Int?
}

public val frames: List<StackFrame>
public val message: String
public val exceptionId: Int
public val originalMessage: String?
public val name: String?
public val componentStack: String?
public val stack: List<StackFrame>
public val id: Int
public val isFatal: Boolean
public val extraData: ReadableMap
}

@DoNotStripAny
private data class ParsedStackFrameImpl(
override val fileName: String,
override val file: String?,
override val methodName: String,
override val lineNumber: Int,
override val columnNumber: Int,
override val lineNumber: Int?,
override val column: Int?,
) : ParsedError.StackFrame

@DoNotStripAny
private data class ParsedErrorImpl(
override val frames: ArrayList<ParsedStackFrameImpl>,
override val message: String,
override val exceptionId: Int,
override val originalMessage: String?,
override val name: String?,
override val componentStack: String?,
override val stack: ArrayList<ParsedStackFrameImpl>,
override val id: Int,
override val isFatal: Boolean,
override val extraData: ReadableNativeMap,
) : ParsedError

public fun reportJsException(errorMap: ParsedError)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
#include <fbjni/fbjni.h>
#include <glog/logging.h>
#include <jni.h>
#include <jsi/JSIDynamic.h>
#include <jsi/jsi.h>
#include <react/jni/ReadableNativeMap.h>

namespace facebook::react {

Expand All @@ -28,7 +31,10 @@ class ParsedStackFrameImpl
static facebook::jni::local_ref<ParsedStackFrameImpl> create(
const JsErrorHandler::ParsedError::StackFrame& frame) {
return newInstance(
frame.fileName, frame.methodName, frame.lineNumber, frame.columnNumber);
frame.file ? jni::make_jstring(*frame.file) : nullptr,
frame.methodName,
frame.lineNumber ? jni::JInteger::valueOf(*frame.lineNumber) : nullptr,
frame.column ? jni::JInteger::valueOf(*frame.column) : nullptr);
}
};

Expand All @@ -39,27 +45,42 @@ class ParsedErrorImpl
"Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler$ParsedErrorImpl;";

static facebook::jni::local_ref<ParsedErrorImpl> create(
jsi::Runtime& runtime,
const JsErrorHandler::ParsedError& error) {
auto stackFrames =
facebook::jni::JArrayList<ParsedStackFrameImpl>::create();
for (const auto& frame : error.frames) {
stackFrames->add(ParsedStackFrameImpl::create(frame));
auto stack = facebook::jni::JArrayList<ParsedStackFrameImpl>::create();
for (const auto& frame : error.stack) {
stack->add(ParsedStackFrameImpl::create(frame));
}

auto extraDataDynamic =
jsi::dynamicFromValue(runtime, jsi::Value(runtime, error.extraData));

auto extraData =
ReadableNativeMap::createWithContents(std::move(extraDataDynamic));

return newInstance(
stackFrames, error.message, error.exceptionId, error.isFatal);
error.message,
error.originalMessage ? jni::make_jstring(*error.originalMessage)
: nullptr,
error.name ? jni::make_jstring(*error.name) : nullptr,
error.componentStack ? jni::make_jstring(*error.componentStack)
: nullptr,
stack,
error.id,
error.isFatal,
extraData);
}
};

} // namespace

void JReactExceptionManager::reportJsException(
jsi::Runtime& runtime,
const JsErrorHandler::ParsedError& error) {
static const auto method =
javaClassStatic()->getMethod<void(jni::alias_ref<ParsedError>)>(
"reportJsException");
if (self() != nullptr) {
method(self(), ParsedErrorImpl::create(error));
method(self(), ParsedErrorImpl::create(runtime, error));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ class JReactExceptionManager
static auto constexpr kJavaDescriptor =
"Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;";

void reportJsException(const JsErrorHandler::ParsedError& error);
void reportJsException(
jsi::Runtime& runtime,
const JsErrorHandler::ParsedError& error);
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,11 @@ JReactInstance::JReactInstance(
jReactExceptionManager_ = jni::make_global(jReactExceptionManager);
auto onJsError =
[weakJReactExceptionManager = jni::make_weak(jReactExceptionManager)](
jsi::Runtime& runtime,
const JsErrorHandler::ParsedError& error) mutable noexcept {
if (auto jReactExceptionManager =
weakJReactExceptionManager.lockLocal()) {
jReactExceptionManager->reportJsException(error);
jReactExceptionManager->reportJsException(runtime, error);
}
};

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

package com.facebook.react.devsupport

import com.facebook.react.bridge.JavaOnlyMap
import com.facebook.react.bridge.ReadableMap
import com.facebook.react.common.annotations.UnstableReactNativeAPI
import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler.*
Expand Down Expand Up @@ -98,27 +99,31 @@ class StackTraceHelperTest {
private fun getParsedErrorTestData(): ParsedError {
val frame1 =
object : ParsedError.StackFrame {
override val fileName = "file1"
override val file = "file1"
override val methodName = "method1"
override val lineNumber = 1
override val columnNumber = 10
override val column = 10
}

val frame2 =
object : ParsedError.StackFrame {
override val fileName = "file2"
override val file = "file2"
override val methodName = "method2"
override val lineNumber = 2
override val columnNumber = 20
override val column = 20
}

val frames = listOf(frame1, frame2)

return object : ParsedError {
override val frames = frames
override val message = "error message"
override val exceptionId = 123
override val originalMessage = null
override val name = null
override val componentStack = null
override val stack = frames
override val id = 123
override val isFatal = true
override val extraData = JavaOnlyMap()
}
}
}
Loading

0 comments on commit 6e329ae

Please sign in to comment.