From 3a825c036065783aae6fb734028f986dbead80d7 Mon Sep 17 00:00:00 2001 From: Moti Zilberman Date: Tue, 16 Jul 2019 09:30:52 -0700 Subject: [PATCH] Introduce NativeExceptionsManager.reportException API Summary: @public `reportException` is a new method on `NativeExceptionsManager` that is designed to allow more structured and flexible JS error reporting. `reportFatalException` and `reportSoftException` are now deprecated. In addition to all the usual exception fields, `reportException` also accepts an `extraData` property which the JS exception handler can populate with arbitrary JSON-serialisable data (here: the raw stack trace, the current JS engine, and the number of frames popped off the call stack by the exception handler). The contents of `extraData` get attached as JSON to the `JavascriptException` instance (or just logged, in the case of `console.error`). This change is backwards compatible in two senses: 1. We have a JS fallback that uses `reportFatalException` and `reportSoftException` if the new native method is unavailable. 2. We have a Java fallback that implements `reportFatalException` and `reportSoftException` in terms of `reportException`. Naturally, both fallbacks mentioned above discard `extraData`. NOTE: The current implementation is Android-only; for the time being, iOS will continue to use the JS fallback. While we're in `ExceptionsManager.js`, this also changes `dismissRedbox()` to be optional (which it is, since it's Android-only); existing call sites already guard it with a null check so this requires no other changes. Reviewed By: mmmulani Differential Revision: D16133080 fbshipit-source-id: d0b209d58da40b736df63155bbea232e94ce635c --- Libraries/Core/ExceptionsManager.js | 24 ++++--- Libraries/Core/NativeExceptionsManager.js | 59 +++++++++++++++++- .../FBReactNativeSpec-generated.mm | 20 ++++++ .../FBReactNativeSpec/FBReactNativeSpec.h | 62 +++++++++++++++++++ .../modules/core/ExceptionsManagerModule.java | 45 ++++++++++---- .../main/java/com/facebook/react/util/BUCK | 3 +- .../react/util/ExceptionDataHelper.java | 36 +++++++++++ 7 files changed, 222 insertions(+), 27 deletions(-) create mode 100644 ReactAndroid/src/main/java/com/facebook/react/util/ExceptionDataHelper.java diff --git a/Libraries/Core/ExceptionsManager.js b/Libraries/Core/ExceptionsManager.js index c9491092fda..c6e6a3cf637 100644 --- a/Libraries/Core/ExceptionsManager.js +++ b/Libraries/Core/ExceptionsManager.js @@ -31,19 +31,17 @@ function reportException(e: ExtendedError, isFatal: boolean) { const currentExceptionID = ++exceptionID; const message = e.jsEngine == null ? e.message : `${e.message}, js engine: ${e.jsEngine}`; - if (isFatal) { - NativeExceptionsManager.reportFatalException( - message, - stack, - currentExceptionID, - ); - } else { - NativeExceptionsManager.reportSoftException( - message, - stack, - currentExceptionID, - ); - } + NativeExceptionsManager.reportException({ + message, + stack, + id: currentExceptionID, + isFatal, + extraData: { + jsEngine: e.jsEngine, + rawStack: e.stack, + framesPopped: e.framesToPop, + }, + }); if (__DEV__) { if (e.preventSymbolication === true) { return; diff --git a/Libraries/Core/NativeExceptionsManager.js b/Libraries/Core/NativeExceptionsManager.js index e2528fd7ec7..f79f2643914 100644 --- a/Libraries/Core/NativeExceptionsManager.js +++ b/Libraries/Core/NativeExceptionsManager.js @@ -20,24 +20,79 @@ export type StackFrame = {| methodName: string, |}; +export type ExceptionData = { + message: string, + stack: Array, + id: number, + isFatal: boolean, + extraData?: ?{}, +}; + export interface Spec extends TurboModule { + // Deprecated: Use `reportException` +reportFatalException: ( message: string, stack: Array, exceptionId: number, ) => void; + // Deprecated: Use `reportException` +reportSoftException: ( message: string, stack: Array, exceptionId: number, ) => void; + +reportException?: (data: ExceptionData) => void; +updateExceptionMessage: ( message: string, stack: Array, exceptionId: number, ) => void; // Android only - +dismissRedbox: () => void; + +dismissRedbox?: () => void; } -export default TurboModuleRegistry.getEnforcing('ExceptionsManager'); +const NativeModule = TurboModuleRegistry.getEnforcing( + 'ExceptionsManager', +); + +const ExceptionsManager = { + reportFatalException( + message: string, + stack: Array, + exceptionId: number, + ) { + NativeModule.reportFatalException(message, stack, exceptionId); + }, + reportSoftException( + message: string, + stack: Array, + exceptionId: number, + ) { + NativeModule.reportSoftException(message, stack, exceptionId); + }, + updateExceptionMessage( + message: string, + stack: Array, + exceptionId: number, + ) { + NativeModule.updateExceptionMessage(message, stack, exceptionId); + }, + dismissRedbox(): void { + if (NativeModule.dismissRedbox) { + NativeModule.dismissRedbox(); + } + }, + reportException(data: ExceptionData): void { + if (NativeModule.reportException) { + NativeModule.reportException(data); + return; + } + if (data.isFatal) { + ExceptionsManager.reportFatalException(data.message, data.stack, data.id); + } else { + ExceptionsManager.reportSoftException(data.message, data.stack, data.id); + } + }, +}; + +export default ExceptionsManager; diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm index 04bc36f10f6..b001f195758 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm +++ b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec-generated.mm @@ -642,6 +642,18 @@ namespace facebook { return facebook::react::managedPointer(json); } @end +@implementation RCTCxxConvert (NativeExceptionsManager_ExceptionDataExtraData) ++ (RCTManagedPointer *)JS_NativeExceptionsManager_ExceptionDataExtraData:(id)json +{ + return facebook::react::managedPointer(json); +} +@end +@implementation RCTCxxConvert (NativeExceptionsManager_ExceptionData) ++ (RCTManagedPointer *)JS_NativeExceptionsManager_ExceptionData:(id)json +{ + return facebook::react::managedPointer(json); +} +@end namespace facebook { namespace react { @@ -654,6 +666,10 @@ namespace facebook { return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "reportSoftException", @selector(reportSoftException:stack:exceptionId:), args, count); } + static facebook::jsi::Value __hostFunction_NativeExceptionsManagerSpecJSI_reportException(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { + return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "reportException", @selector(reportException:), args, count); + } + static facebook::jsi::Value __hostFunction_NativeExceptionsManagerSpecJSI_updateExceptionMessage(facebook::jsi::Runtime& rt, TurboModule &turboModule, const facebook::jsi::Value* args, size_t count) { return static_cast(turboModule).invokeObjCMethod(rt, VoidKind, "updateExceptionMessage", @selector(updateExceptionMessage:stack:exceptionId:), args, count); } @@ -672,6 +688,10 @@ namespace facebook { methodMap_["reportSoftException"] = MethodMetadata {3, __hostFunction_NativeExceptionsManagerSpecJSI_reportSoftException}; + methodMap_["reportException"] = MethodMetadata {1, __hostFunction_NativeExceptionsManagerSpecJSI_reportException}; + + setMethodArgConversionSelector(@"reportException", 0, @"JS_NativeExceptionsManager_ExceptionData:"); + methodMap_["updateExceptionMessage"] = MethodMetadata {3, __hostFunction_NativeExceptionsManagerSpecJSI_updateExceptionMessage}; diff --git a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h index 2b488e37db6..9a32e7c5a05 100644 --- a/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h +++ b/Libraries/FBReactNativeSpec/FBReactNativeSpec/FBReactNativeSpec.h @@ -696,6 +696,41 @@ namespace JS { @interface RCTCxxConvert (NativeExceptionsManager_StackFrame) + (RCTManagedPointer *)JS_NativeExceptionsManager_StackFrame:(id)json; @end + +namespace JS { + namespace NativeExceptionsManager { + struct ExceptionDataExtraData { + + ExceptionDataExtraData(NSDictionary *const v) : _v(v) {} + private: + NSDictionary *_v; + }; + } +} + +@interface RCTCxxConvert (NativeExceptionsManager_ExceptionDataExtraData) ++ (RCTManagedPointer *)JS_NativeExceptionsManager_ExceptionDataExtraData:(id)json; +@end + +namespace JS { + namespace NativeExceptionsManager { + struct ExceptionData { + NSString *message() const; + facebook::react::LazyVector stack() const; + double id_() const; + bool isFatal() const; + folly::Optional extraData() const; + + ExceptionData(NSDictionary *const v) : _v(v) {} + private: + NSDictionary *_v; + }; + } +} + +@interface RCTCxxConvert (NativeExceptionsManager_ExceptionData) ++ (RCTManagedPointer *)JS_NativeExceptionsManager_ExceptionData:(id)json; +@end @protocol NativeExceptionsManagerSpec - (void)reportFatalException:(NSString *)message @@ -704,6 +739,7 @@ namespace JS { - (void)reportSoftException:(NSString *)message stack:(NSArray *)stack exceptionId:(double)exceptionId; +- (void)reportException:(JS::NativeExceptionsManager::ExceptionData &)data; - (void)updateExceptionMessage:(NSString *)message stack:(NSArray *)stack exceptionId:(double)exceptionId; @@ -1980,6 +2016,32 @@ inline NSString *JS::NativeExceptionsManager::StackFrame::methodName() const id const p = _v[@"methodName"]; return RCTBridgingToString(p); } + +inline NSString *JS::NativeExceptionsManager::ExceptionData::message() const +{ + id const p = _v[@"message"]; + return RCTBridgingToString(p); +} +inline facebook::react::LazyVector JS::NativeExceptionsManager::ExceptionData::stack() const +{ + id const p = _v[@"stack"]; + return RCTBridgingToVec(p, ^JS::NativeExceptionsManager::StackFrame(id itemValue_0) { return JS::NativeExceptionsManager::StackFrame(itemValue_0); }); +} +inline double JS::NativeExceptionsManager::ExceptionData::id_() const +{ + id const p = _v[@"id"]; + return RCTBridgingToDouble(p); +} +inline bool JS::NativeExceptionsManager::ExceptionData::isFatal() const +{ + id const p = _v[@"isFatal"]; + return RCTBridgingToBool(p); +} +inline folly::Optional JS::NativeExceptionsManager::ExceptionData::extraData() const +{ + id const p = _v[@"extraData"]; + return (p == nil ? folly::none : folly::make_optional(JS::NativeExceptionsManager::ExceptionDataExtraData(p))); +} inline JS::NativeI18nManager::Constants::Builder::Builder(const Input i) : _factory(^{ NSMutableDictionary *d = [NSMutableDictionary new]; auto isRTL = i.isRTL.get(); diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java index 7c27b632111..40d3aff75f5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java +++ b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ExceptionsManagerModule.java @@ -8,12 +8,15 @@ package com.facebook.react.modules.core; import com.facebook.common.logging.FLog; import com.facebook.react.bridge.BaseJavaModule; +import com.facebook.react.bridge.JavaOnlyMap; import com.facebook.react.bridge.ReactMethod; import com.facebook.react.bridge.ReadableArray; +import com.facebook.react.bridge.ReadableMap; import com.facebook.react.common.JavascriptException; import com.facebook.react.common.ReactConstants; import com.facebook.react.devsupport.interfaces.DevSupportManager; import com.facebook.react.module.annotations.ReactModule; +import com.facebook.react.util.ExceptionDataHelper; import com.facebook.react.util.JSStackTrace; @ReactModule(name = ExceptionsManagerModule.NAME) @@ -33,24 +36,44 @@ public class ExceptionsManagerModule extends BaseJavaModule { } @ReactMethod - public void reportFatalException(String title, ReadableArray details, int exceptionId) { - showOrThrowError(title, details, exceptionId); + public void reportFatalException(String message, ReadableArray stack, int id) { + JavaOnlyMap data = new JavaOnlyMap(); + data.putString("message", message); + data.putArray("stack", stack); + data.putInt("id", id); + data.putBoolean("isFatal", true); + reportException(data); } @ReactMethod - public void reportSoftException(String title, ReadableArray details, int exceptionId) { - if (mDevSupportManager.getDevSupportEnabled()) { - mDevSupportManager.showNewJSError(title, details, exceptionId); - } else { - FLog.e(ReactConstants.TAG, JSStackTrace.format(title, details)); - } + public void reportSoftException(String message, ReadableArray stack, int id) { + JavaOnlyMap data = new JavaOnlyMap(); + data.putString("message", message); + data.putArray("stack", stack); + data.putInt("id", id); + data.putBoolean("isFatal", false); + reportException(data); } - private void showOrThrowError(String title, ReadableArray details, int exceptionId) { + @ReactMethod + public void reportException(ReadableMap data) { + String message = data.getString("message"); + ReadableArray stack = data.getArray("stack"); + int id = data.getInt("id"); + boolean isFatal = data.getBoolean("isFatal"); if (mDevSupportManager.getDevSupportEnabled()) { - mDevSupportManager.showNewJSError(title, details, exceptionId); + mDevSupportManager.showNewJSError(message, stack, id); } else { - throw new JavascriptException(JSStackTrace.format(title, details)); + String extraDataAsJson = ExceptionDataHelper.getExtraDataAsJson(data); + if (isFatal) { + throw new JavascriptException(JSStackTrace.format(message, stack)) + .setExtraDataAsJson(extraDataAsJson); + } else { + FLog.e(ReactConstants.TAG, JSStackTrace.format(message, stack)); + if (extraDataAsJson != null) { + FLog.d(ReactConstants.TAG, "extraData: %s", extraDataAsJson); + } + } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/util/BUCK b/ReactAndroid/src/main/java/com/facebook/react/util/BUCK index 7f700ab80a7..93288f7267b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/util/BUCK +++ b/ReactAndroid/src/main/java/com/facebook/react/util/BUCK @@ -1,4 +1,4 @@ -load("//tools/build_defs/oss:rn_defs.bzl", "react_native_target", "rn_android_library") +load("//tools/build_defs/oss:rn_defs.bzl", "react_native_dep", "react_native_target", "rn_android_library") rn_android_library( name = "util", @@ -7,6 +7,7 @@ rn_android_library( "PUBLIC", ], deps = [ + react_native_dep("third-party/java/jsr-305:jsr-305"), react_native_target("java/com/facebook/react/bridge:bridge"), ], ) diff --git a/ReactAndroid/src/main/java/com/facebook/react/util/ExceptionDataHelper.java b/ReactAndroid/src/main/java/com/facebook/react/util/ExceptionDataHelper.java new file mode 100644 index 00000000000..dffac94ef77 --- /dev/null +++ b/ReactAndroid/src/main/java/com/facebook/react/util/ExceptionDataHelper.java @@ -0,0 +1,36 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + *

This source code is licensed under the MIT license found in the LICENSE file in the root + * directory of this source tree. + */ +package com.facebook.react.util; + +import android.util.JsonWriter; +import com.facebook.react.bridge.JsonWriterHelper; +import com.facebook.react.bridge.ReadableMap; +import com.facebook.react.bridge.ReadableType; +import java.io.IOException; +import java.io.StringWriter; +import java.io.Writer; +import javax.annotation.Nullable; + +public class ExceptionDataHelper { + static final String EXTRA_DATA_FIELD = "extraData"; + + public static String getExtraDataAsJson(@Nullable ReadableMap metadata) { + if (metadata == null || metadata.getType(EXTRA_DATA_FIELD) == ReadableType.Null) { + return null; + } + try { + Writer extraDataWriter = new StringWriter(); + JsonWriter json = new JsonWriter(extraDataWriter); + JsonWriterHelper.value(json, metadata.getDynamic(EXTRA_DATA_FIELD)); + json.close(); + extraDataWriter.close(); + return extraDataWriter.toString(); + } catch (IOException ex) { + } + return null; + } +}