Skip to content

Commit

Permalink
Handle errors in Metro by showing a code frame
Browse files Browse the repository at this point in the history
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
  • Loading branch information
rickhanlonii authored and facebook-github-bot committed Feb 25, 2020
1 parent afc77bd commit 956359b
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 0 deletions.
3 changes: 3 additions & 0 deletions Libraries/LogBox/Data/LogBoxLog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -36,6 +37,7 @@ export type LogBoxLogData = $ReadOnly<{|

class LogBoxLog {
message: Message;
type: ?string;
category: Category;
componentStack: ComponentStack;
stack: Stack;
Expand All @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions Libraries/LogBox/Data/parseLogBoxLog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions Libraries/LogBox/UI/LogBoxInspector.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ function LogBoxInspectorBody(props) {
}, [props.log]);

const headerTitle =
props.log.type ??
headerTitleMap[props.log.isComponentError ? 'component' : props.log.level];

if (collapsed) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = `
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onRetry={[Function]}
Expand Down Expand Up @@ -82,6 +83,7 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = `
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onRetry={[Function]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ exports[`LogBoxNotificationContainer should render both an error and warning not
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onPressDismiss={[Function]}
Expand Down Expand Up @@ -76,6 +77,7 @@ exports[`LogBoxNotificationContainer should render both an error and warning not
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onPressDismiss={[Function]}
Expand Down Expand Up @@ -134,6 +136,7 @@ exports[`LogBoxNotificationContainer should render the latest error notification
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onPressDismiss={[Function]}
Expand Down Expand Up @@ -184,6 +187,7 @@ exports[`LogBoxNotificationContainer should render the latest warning notificati
"stack": null,
"status": "NONE",
},
"type": undefined,
}
}
onPressDismiss={[Function]}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand All @@ -50,6 +51,7 @@ exports[`LogBoxNotificationContainer should render inspector with logs, even whe
"stack": null,
"status": "NONE",
},
"type": undefined,
},
]
}
Expand Down

0 comments on commit 956359b

Please sign in to comment.