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

earlyjs: Integrate c++ pipeline with console.error #47169

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
12 changes: 0 additions & 12 deletions packages/react-native/Libraries/AppDelegate/RCTAppDelegate.mm
Original file line number Diff line number Diff line change
Expand Up @@ -178,18 +178,6 @@ - (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
{
}

#pragma mark - Bridge and Bridge Adapter properties

- (RCTBridge *)bridge
Expand Down
68 changes: 41 additions & 27 deletions packages/react-native/Libraries/Core/ExceptionsManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,24 +141,28 @@ let inExceptionHandler = false;
* Logs exceptions to the (native) console and displays them
*/
function handleException(e: mixed, isFatal: boolean) {
let error: Error;
if (e instanceof Error) {
error = e;
} else {
// Workaround for reporting errors caused by `throw 'some string'`
// Unfortunately there is no way to figure out the stacktrace in this
// case, so if you ended up here trying to trace an error, look for
// `throw '<error message>'` somewhere in your codebase.
error = new SyntheticError(e);
}
try {
inExceptionHandler = true;
/* $FlowFixMe[class-object-subtyping] added when improving typing for this
* parameters */
// $FlowFixMe[incompatible-call]
reportException(error, isFatal, /*reportToConsole*/ true);
} finally {
inExceptionHandler = false;
// TODO(T196834299): We should really use a c++ turbomodule for this
const reportToConsole = true;
if (!global.RN$handleException || !global.RN$handleException(e, isFatal)) {
let error: Error;
if (e instanceof Error) {
error = e;
} else {
// Workaround for reporting errors caused by `throw 'some string'`
// Unfortunately there is no way to figure out the stacktrace in this
// case, so if you ended up here trying to trace an error, look for
// `throw '<error message>'` somewhere in your codebase.
error = new SyntheticError(e);
}
try {
inExceptionHandler = true;
/* $FlowFixMe[class-object-subtyping] added when improving typing for this
* parameters */
// $FlowFixMe[incompatible-call]
reportException(error, isFatal, reportToConsole);
} finally {
inExceptionHandler = false;
}
}
}

Expand All @@ -170,7 +174,10 @@ function reactConsoleErrorHandler(...args) {
if (!console.reportErrorsAsExceptions) {
return;
}
if (inExceptionHandler) {
if (
inExceptionHandler ||
(global.RN$inExceptionHandler && global.RN$inExceptionHandler())
) {
// The fundamental trick here is that are multiple entry point to logging errors:
// (see D19743075 for more background)
//
Expand Down Expand Up @@ -224,14 +231,21 @@ function reactConsoleErrorHandler(...args) {
error.name = 'console.error';
}

reportException(
/* $FlowFixMe[class-object-subtyping] added when improving typing for this
* parameters */
// $FlowFixMe[incompatible-call]
error,
false, // isFatal
false, // reportToConsole
);
const isFatal = false;
const reportToConsole = false;
if (
!global.RN$handleException ||
!global.RN$handleException(error, isFatal, reportToConsole)
) {
reportException(
/* $FlowFixMe[class-object-subtyping] added when improving typing for this
* parameters */
// $FlowFixMe[incompatible-call]
error,
isFatal,
reportToConsole,
);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import typeof NativeExceptionsManager from '../NativeExceptionsManager';
export default ({
reportFatalException: jest.fn(),
reportSoftException: jest.fn(),
updateExceptionMessage: jest.fn(),
dismissRedbox: jest.fn(),
reportException: jest.fn(),
}: NativeExceptionsManager);
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,6 @@ function runExceptionsManagerTests() {
return {
default: {
reportException: jest.fn(),
// Used to show symbolicated messages, not part of this test.
updateExceptionMessage: () => {},
},
};
});
Expand Down
8 changes: 1 addition & 7 deletions packages/react-native/Libraries/Core/setUpErrorHandling.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,7 @@ ExceptionsManager.installConsoleErrorReporter();
if (!global.__fbDisableExceptionsManager) {
const handleError = (e: mixed, isFatal: boolean) => {
try {
// TODO(T196834299): We should really use a c++ turbomodule for this
if (
!global.RN$handleException ||
!global.RN$handleException(e, isFatal)
) {
ExceptionsManager.handleException(e, isFatal);
}
ExceptionsManager.handleException(e, isFatal);
} catch (ee) {
console.log('Failed to print error: ', ee.message);
throw e;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
NS_ASSUME_NONNULL_BEGIN

@protocol RCTExceptionsManagerDelegate <NSObject>

- (void)handleSoftJSExceptionWithMessage:(nullable NSString *)message
stack:(nullable NSArray *)stack
exceptionId:(NSNumber *)exceptionId
Expand All @@ -20,12 +19,6 @@ NS_ASSUME_NONNULL_BEGIN
stack:(nullable NSArray *)stack
exceptionId:(NSNumber *)exceptionId
extraDataAsJSON:(nullable NSString *)extraDataAsJSON;

@optional
- (void)updateJSExceptionWithMessage:(nullable NSString *)message
stack:(nullable NSArray *)stack
exceptionId:(NSNumber *)exceptionId;

@end

@interface RCTExceptionsManager : NSObject <RCTBridgeModule>
Expand All @@ -41,7 +34,7 @@ NS_ASSUME_NONNULL_BEGIN
- (void)reportJsException:(nullable NSString *)message
stack:(nullable NSArray<NSDictionary *> *)stack
exceptionId:(double)exceptionId
isFatal:(bool)isFatal;
isFatal:(bool)isFatal __attribute__((deprecated));

@property (nonatomic, weak) id<RCTExceptionsManagerDelegate> delegate;

Expand Down
21 changes: 0 additions & 21 deletions packages/react-native/React/CoreModules/RCTExceptionsManager.mm
Original file line number Diff line number Diff line change
Expand Up @@ -99,27 +99,6 @@ - (void)reportFatal:(NSString *)message
[self reportFatal:message stack:stack exceptionId:exceptionId extraDataAsJSON:nil];
}

RCT_EXPORT_METHOD(updateExceptionMessage
: (NSString *)message stack
: (NSArray<NSDictionary *> *)stack exceptionId
: (double)exceptionId)
{
if (RCTRedBoxGetEnabled()) {
RCTRedBox *redbox = [_moduleRegistry moduleForName:"RedBox"];
[redbox updateErrorMessage:message withStack:stack errorCookie:(int)exceptionId];
}

if (_delegate && [_delegate respondsToSelector:@selector(updateJSExceptionWithMessage:stack:exceptionId:)]) {
[_delegate updateJSExceptionWithMessage:message stack:stack exceptionId:[NSNumber numberWithDouble:exceptionId]];
}
}

// Deprecated. Use reportFatalException directly instead.
RCT_EXPORT_METHOD(reportUnhandledException : (NSString *)message stack : (NSArray<NSDictionary *> *)stack)
{
[self reportFatalException:message stack:stack exceptionId:-1];
}

RCT_EXPORT_METHOD(dismissRedbox) {}

RCT_EXPORT_METHOD(reportException : (JS::NativeExceptionsManager::ExceptionData &)data)
Expand Down
4 changes: 0 additions & 4 deletions packages/react-native/ReactAndroid/api/ReactAndroid.api
Original file line number Diff line number Diff line change
Expand Up @@ -2212,7 +2212,6 @@ public abstract class com/facebook/react/devsupport/DevSupportManagerBase : com/
public fun startInspector ()V
public fun stopInspector ()V
public fun toggleElementInspector ()V
public fun updateJSError (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;I)V
}

public abstract interface class com/facebook/react/devsupport/DevSupportManagerBase$CallbackWithBundleLoader {
Expand Down Expand Up @@ -2380,7 +2379,6 @@ public class com/facebook/react/devsupport/ReleaseDevSupportManager : com/facebo
public fun startInspector ()V
public fun stopInspector ()V
public fun toggleElementInspector ()V
public fun updateJSError (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;I)V
}

public class com/facebook/react/devsupport/StackTraceHelper {
Expand Down Expand Up @@ -2516,7 +2514,6 @@ public abstract interface class com/facebook/react/devsupport/interfaces/DevSupp
public abstract fun startInspector ()V
public abstract fun stopInspector ()V
public abstract fun toggleElementInspector ()V
public abstract fun updateJSError (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;I)V
}

public abstract interface class com/facebook/react/devsupport/interfaces/DevSupportManager$PackagerLocationCustomizer {
Expand Down Expand Up @@ -3138,7 +3135,6 @@ public class com/facebook/react/modules/core/ExceptionsManagerModule : com/faceb
public fun reportException (Lcom/facebook/react/bridge/ReadableMap;)V
public fun reportFatalException (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;D)V
public fun reportSoftException (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;D)V
public fun updateExceptionMessage (Ljava/lang/String;Lcom/facebook/react/bridge/ReadableArray;D)V
}

public class com/facebook/react/modules/core/HeadlessJsTaskSupportModule : com/facebook/fbreact/specs/NativeHeadlessJsTaskSupportSpec {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,26 +279,6 @@ public Pair<String, StackFrame[]> processErrorCustomizers(Pair<String, StackFram
return errorInfo;
}

@Override
public void updateJSError(
final String message, final ReadableArray details, final int errorCookie) {
UiThreadUtil.runOnUiThread(
() -> {
// Since we only show the first JS error in a succession of JS errors, make sure we only
// update the error message for that error message. This assumes that updateJSError
// belongs to the most recent showNewJSError
if ((mRedBoxSurfaceDelegate != null && !mRedBoxSurfaceDelegate.isShowing())
|| errorCookie != mLastErrorCookie) {
return;
}

// The RedBox surface delegate will always show the latest error
updateLastErrorInfo(
message, StackTraceHelper.convertJsStackTrace(details), errorCookie, ErrorType.JS);
mRedBoxSurfaceDelegate.show();
});
}

@Override
public void hideRedboxDialog() {
if (mRedBoxSurfaceDelegate == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ public open class ReleaseDevSupportManager : DevSupportManager {

override public fun destroyRootView(rootView: View?): Unit = Unit

override public fun updateJSError(
message: String?,
details: ReadableArray?,
errorCookie: Int
): Unit = Unit

override public fun hideRedboxDialog(): Unit = Unit

override public fun showDevOptionsDialog(): Unit = Unit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ public interface DevSupportManager : JSExceptionHandler {

public fun showNewJSError(message: String?, details: ReadableArray?, errorCookie: Int)

public fun updateJSError(message: String?, details: ReadableArray?, errorCookie: Int)

public fun hideRedboxDialog()

public fun showDevOptionsDialog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,6 @@ public open class ExceptionsManagerModule(private val devSupportManager: DevSupp
}
}

override fun updateExceptionMessage(
title: String?,
details: ReadableArray?,
exceptionIdDouble: Double
) {
val exceptionId = exceptionIdDouble.toInt()
if (devSupportManager.devSupportEnabled) {
devSupportManager.updateJSError(title, details, exceptionId)
}
}

override fun dismissRedbox() {
if (devSupportManager.devSupportEnabled) {
devSupportManager.hideRedboxDialog()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,34 @@ void objectAssign(
auto assign = Object.getPropertyAsFunction(runtime, "assign");
assign.callWithThis(runtime, Object, target, value);
}

jsi::Object wrapInErrorIfNecessary(
jsi::Runtime& runtime,
const jsi::Value& value) {
auto Error = runtime.global().getPropertyAsFunction(runtime, "Error");
auto isError =
value.isObject() && value.asObject(runtime).instanceOf(runtime, Error);
auto error = isError
? value.getObject(runtime)
: Error.callAsConstructor(runtime, value).getObject(runtime);
return error;
}

class SetFalseOnDestruct {
std::shared_ptr<bool> _value;

public:
SetFalseOnDestruct(const SetFalseOnDestruct&) = delete;
SetFalseOnDestruct& operator=(const SetFalseOnDestruct&) = delete;
SetFalseOnDestruct(SetFalseOnDestruct&&) = delete;
SetFalseOnDestruct& operator=(SetFalseOnDestruct&&) = delete;
explicit SetFalseOnDestruct(std::shared_ptr<bool> value)
: _value(std::move(value)) {}
~SetFalseOnDestruct() {
*_value = false;
}
};

} // namespace

namespace facebook::react {
Expand Down Expand Up @@ -150,7 +178,7 @@ std::ostream& operator<<(

JsErrorHandler::JsErrorHandler(JsErrorHandler::OnJsError onJsError)
: _onJsError(std::move(onJsError)),
_hasHandledFatalError(false){
_inErrorHandler(std::make_shared<bool>(false)){

};

Expand All @@ -159,7 +187,8 @@ JsErrorHandler::~JsErrorHandler() {}
void JsErrorHandler::handleError(
jsi::Runtime& runtime,
jsi::JSError& error,
bool isFatal) {
bool isFatal,
bool logToConsole) {
// TODO: Current error parsing works and is stable. Can investigate using
// REGEX_HERMES to get additional Hermes data, though it requires JS setup
if (_isRuntimeReady) {
Expand All @@ -179,15 +208,19 @@ void JsErrorHandler::handleError(
}
}

emitError(runtime, error, isFatal);
handleErrorWithCppPipeline(runtime, error, isFatal, logToConsole);
}

void JsErrorHandler::emitError(
void JsErrorHandler::handleErrorWithCppPipeline(
jsi::Runtime& runtime,
jsi::JSError& error,
bool isFatal) {
bool isFatal,
bool logToConsole) {
*_inErrorHandler = true;
SetFalseOnDestruct temp{_inErrorHandler};

auto message = error.getMessage();
auto errorObj = error.value().getObject(runtime);
auto errorObj = wrapInErrorIfNecessary(runtime, error.value());
auto componentStackValue = errorObj.getProperty(runtime, "componentStack");
if (!isLooselyNull(componentStackValue)) {
message += "\n" + stringifyToCpp(runtime, componentStackValue);
Expand Down Expand Up @@ -266,6 +299,14 @@ void JsErrorHandler::emitError(
isTruthy(runtime, errorObj.getProperty(runtime, "isComponentError"));
data.setProperty(runtime, "isComponentError", isComponentError);

if (logToConsole) {
auto console = runtime.global().getPropertyAsObject(runtime, "console");
auto errorFn = console.getPropertyAsFunction(runtime, "error");
auto finalMessage =
jsi::String::createFromUtf8(runtime, parsedError.message);
errorFn.callWithThis(runtime, console, finalMessage);
}

std::shared_ptr<bool> shouldPreventDefault = std::make_shared<bool>(false);
auto preventDefault = jsi::Function::createFromHostFunction(
runtime,
Expand Down Expand Up @@ -318,4 +359,8 @@ void JsErrorHandler::notifyOfFatalError() {
_hasHandledFatalError = true;
}

bool JsErrorHandler::inErrorHandler() {
return *_inErrorHandler;
}

} // namespace facebook::react
Loading
Loading