From 956359bee645f81ed9d05edb10bc2a76ed403205 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Tue, 25 Feb 2020 04:32:21 -0800 Subject: [PATCH] Handle errors in Metro by showing a code frame Summary: Right now the code frame and stack trace for metro errors are useless. This diff improved these errors by showing the metro code frame for the source of the issue, and stripping the stack trace. Ideally we could show the metro stack trace as well, but since stack traces are tightly coupled to symbolication (and metro source code does not need symbolicated, nor could it be), this is an acceptable incremental improvement. Arguably, even showing the code frame is inappropriate and we should show a generic crash screen with reload and report buttons. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D20057353 fbshipit-source-id: 5e999cea14c1cbd2f69737e3992a3e8d159fdf89 --- Libraries/LogBox/Data/LogBoxLog.js | 3 ++ Libraries/LogBox/Data/parseLogBoxLog.js | 33 +++++++++++++++++++ Libraries/LogBox/UI/LogBoxInspector.js | 1 + .../LogBoxInspector-test.js.snap | 2 ++ .../LogBoxInspectorContainer-test.js.snap | 4 +++ .../LogBoxNotificationContainer-test.js.snap | 2 ++ 6 files changed, 45 insertions(+) diff --git a/Libraries/LogBox/Data/LogBoxLog.js b/Libraries/LogBox/Data/LogBoxLog.js index f2f64f56475..2d019518306 100644 --- a/Libraries/LogBox/Data/LogBoxLog.js +++ b/Libraries/LogBox/Data/LogBoxLog.js @@ -26,6 +26,7 @@ export type LogLevel = 'warn' | 'error' | 'fatal' | 'syntax'; export type LogBoxLogData = $ReadOnly<{| level: LogLevel, + type?: ?string, message: Message, stack: Stack, category: string, @@ -36,6 +37,7 @@ export type LogBoxLogData = $ReadOnly<{| class LogBoxLog { message: Message; + type: ?string; category: Category; componentStack: ComponentStack; stack: Stack; @@ -55,6 +57,7 @@ class LogBoxLog { constructor(data: LogBoxLogData) { this.level = data.level; + this.type = data.type; this.message = data.message; this.stack = data.stack; this.category = data.category; diff --git a/Libraries/LogBox/Data/parseLogBoxLog.js b/Libraries/LogBox/Data/parseLogBoxLog.js index 42c43320fd0..dbd978bc9e6 100644 --- a/Libraries/LogBox/Data/parseLogBoxLog.js +++ b/Libraries/LogBox/Data/parseLogBoxLog.js @@ -17,6 +17,7 @@ import type {LogBoxLogData} from './LogBoxLog'; const BABEL_TRANSFORM_ERROR_FORMAT = /^(?:TransformError )?(?:SyntaxError: |ReferenceError: )(.*): (.*) \((\d+):(\d+)\)\n\n([\s\S]+)/; const BABEL_CODE_FRAME_ERROR_FORMAT = /^(?:TransformError )?(?:.*): (.*): ([\s\S]+?)\n([ >]{2}[\d\s]+ \|[\s\S]+|\u{001b}[\s\S]+)/u; +const METRO_ERROR_FORMAT = /^(?:InternalError Metro has encountered an error:) (.*): (.*) \((\d+):(\d+)\)\n\n([\s\S]+)/u; export type ExtendedExceptionData = ExceptionData & { isComponentError: boolean, @@ -151,6 +152,38 @@ export function parseLogBoxException( const message = error.originalMessage != null ? error.originalMessage : 'Unknown'; + const metroInternalError = message.match(METRO_ERROR_FORMAT); + if (metroInternalError) { + const [ + content, + fileName, + row, + column, + codeFrame, + ] = metroInternalError.slice(1); + + return { + level: 'fatal', + type: 'Metro Error', + stack: [], + isComponentError: false, + componentStack: [], + codeFrame: { + fileName, + location: { + row: parseInt(row, 10), + column: parseInt(column, 10), + }, + content: codeFrame, + }, + message: { + content, + substitutions: [], + }, + category: `${fileName}-${row}-${column}`, + }; + } + const babelTransformError = message.match(BABEL_TRANSFORM_ERROR_FORMAT); if (babelTransformError) { // Transform errors are thrown from inside the Babel transformer. diff --git a/Libraries/LogBox/UI/LogBoxInspector.js b/Libraries/LogBox/UI/LogBoxInspector.js index 62900e5b47c..9c2983264b0 100644 --- a/Libraries/LogBox/UI/LogBoxInspector.js +++ b/Libraries/LogBox/UI/LogBoxInspector.js @@ -102,6 +102,7 @@ function LogBoxInspectorBody(props) { }, [props.log]); const headerTitle = + props.log.type ?? headerTitleMap[props.log.isComponentError ? 'component' : props.log.level]; if (collapsed) { diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index 3a8b8b933c4..8259c01b67d 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -34,6 +34,7 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` "stack": null, "status": "NONE", }, + "type": undefined, } } onRetry={[Function]} @@ -82,6 +83,7 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` "stack": null, "status": "NONE", }, + "type": undefined, } } onRetry={[Function]} diff --git a/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap b/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap index 7182e10482b..99c5f74e76a 100644 --- a/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap +++ b/Libraries/LogBox/__tests__/__snapshots__/LogBoxInspectorContainer-test.js.snap @@ -40,6 +40,7 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "stack": null, "status": "NONE", }, + "type": undefined, } } onPressDismiss={[Function]} @@ -76,6 +77,7 @@ exports[`LogBoxNotificationContainer should render both an error and warning not "stack": null, "status": "NONE", }, + "type": undefined, } } onPressDismiss={[Function]} @@ -134,6 +136,7 @@ exports[`LogBoxNotificationContainer should render the latest error notification "stack": null, "status": "NONE", }, + "type": undefined, } } onPressDismiss={[Function]} @@ -184,6 +187,7 @@ exports[`LogBoxNotificationContainer should render the latest warning notificati "stack": null, "status": "NONE", }, + "type": undefined, } } onPressDismiss={[Function]} diff --git a/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap b/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap index 2c2253ad83b..c512ee317f2 100644 --- a/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap +++ b/Libraries/LogBox/__tests__/__snapshots__/LogBoxNotificationContainer-test.js.snap @@ -32,6 +32,7 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "stack": null, "status": "NONE", }, + "type": undefined, }, LogBoxLog { "category": "Some kind of message (latest)", @@ -50,6 +51,7 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe "stack": null, "status": "NONE", }, + "type": undefined, }, ] }