From f6b1fa3197154f85f8ba8d99ecb5b0a56ea5cce4 Mon Sep 17 00:00:00 2001 From: Rick Hanlon Date: Fri, 1 Nov 2019 16:05:02 -0700 Subject: [PATCH] Add fatal handling Summary: This diff adds handling to fatal errors such as thrown exceptions by popping them full screen and not allowing the user to dismiss them. They say that they're "Fatal Errors" and explain that "Fatal errors require a reload". Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D18257185 fbshipit-source-id: ca051027b19c3cd2410ae59764d7b98a78f08dca --- Libraries/LogBox/Data/LogBoxData.js | 24 +- Libraries/LogBox/Data/LogBoxLog.js | 2 +- Libraries/LogBox/UI/LogBoxContainer.js | 27 ++ Libraries/LogBox/UI/LogBoxInspector.js | 2 + Libraries/LogBox/UI/LogBoxInspectorFooter.js | 62 ++++- Libraries/LogBox/UI/LogBoxInspectorHeader.js | 32 ++- .../LogBox/UI/LogBoxInspectorMessageHeader.js | 5 +- Libraries/LogBox/UI/LogBoxStyle.js | 8 + .../UI/__tests__/LogBoxContainer-test.js | 82 ++++++ .../UI/__tests__/LogBoxInspector-test.js | 17 +- .../__tests__/LogBoxInspectorFooter-test.js | 18 +- .../LogBoxInspectorMesageHeader-test.js | 20 +- .../LogBoxContainer-test.js.snap | 132 ++++++++++ .../LogBoxInspector-test.js.snap | 18 +- .../LogBoxInspectorFooter-test.js.snap | 21 ++ .../LogBoxInspectorMesageHeader-test.js.snap | 234 ++++++++++++------ 16 files changed, 588 insertions(+), 116 deletions(-) diff --git a/Libraries/LogBox/Data/LogBoxData.js b/Libraries/LogBox/Data/LogBoxData.js index c914775a1e..b0e06cd84e 100644 --- a/Libraries/LogBox/Data/LogBoxData.js +++ b/Libraries/LogBox/Data/LogBoxData.js @@ -127,17 +127,21 @@ export function addException(error: ExceptionData): void { if (lastLog && lastLog.category === category) { lastLog.incrementCount(); } else { - logs.add( - new LogBoxLog( - 'error', - message, - error.stack, - category, - error.componentStack != null - ? parseComponentStack(error.componentStack) - : [], - ), + const newLog = new LogBoxLog( + error.isFatal ? 'fatal' : 'error', + message, + error.stack, + category, + error.componentStack != null + ? parseComponentStack(error.componentStack) + : [], ); + + // Start symbolicating now so it's warm when it renders. + if (error.isFatal) { + symbolicateLogLazy(newLog); + } + logs.add(newLog); } handleUpdate(); diff --git a/Libraries/LogBox/Data/LogBoxLog.js b/Libraries/LogBox/Data/LogBoxLog.js index bffbf9bb8e..f922970317 100644 --- a/Libraries/LogBox/Data/LogBoxLog.js +++ b/Libraries/LogBox/Data/LogBoxLog.js @@ -15,7 +15,7 @@ import * as LogBoxSymbolication from './LogBoxSymbolication'; import type {Category, Message, ComponentStack} from './parseLogBoxLog'; import type {Stack} from './LogBoxSymbolication'; -export type LogLevel = 'warn' | 'error'; +export type LogLevel = 'warn' | 'error' | 'fatal'; export type SymbolicationRequest = $ReadOnly<{| abort: () => void, diff --git a/Libraries/LogBox/UI/LogBoxContainer.js b/Libraries/LogBox/UI/LogBoxContainer.js index e328c0bc4b..fb44b5a5a9 100644 --- a/Libraries/LogBox/UI/LogBoxContainer.js +++ b/Libraries/LogBox/UI/LogBoxContainer.js @@ -51,12 +51,38 @@ function LogBoxContainer(props: Props): React.Node { function openLog(log: LogBoxLog) { let index = logs.length - 1; + + // Stop at zero because if we don't find any log, we'll open the first log. while (index > 0 && logs[index] !== log) { index -= 1; } setSelectedLog(index); } + let fatalIndex = logs.length - 1; + while (fatalIndex >= 0) { + if (logs[fatalIndex].level === 'fatal') { + break; + } + + fatalIndex -= 1; + } + + if (selectedLogIndex == null && fatalIndex >= 0) { + return ( + + + + ); + } + if (selectedLogIndex != null) { return ( @@ -65,6 +91,7 @@ function LogBoxContainer(props: Props): React.Node { onMinimize={handleInspectorMinimize} onChangeSelectedIndex={setSelectedLog} logs={logs} + hasFatal={fatalIndex != null} selectedIndex={selectedLogIndex} /> diff --git a/Libraries/LogBox/UI/LogBoxInspector.js b/Libraries/LogBox/UI/LogBoxInspector.js index 6190a18228..691348efa3 100644 --- a/Libraries/LogBox/UI/LogBoxInspector.js +++ b/Libraries/LogBox/UI/LogBoxInspector.js @@ -32,6 +32,7 @@ type Props = $ReadOnly<{| onMinimize: () => void, logs: $ReadOnlyArray, selectedIndex: number, + hasFatal: boolean, |}>; function LogBoxInspector(props: Props): React.Node { @@ -77,6 +78,7 @@ function LogBoxInspector(props: Props): React.Node { ); diff --git a/Libraries/LogBox/UI/LogBoxInspectorFooter.js b/Libraries/LogBox/UI/LogBoxInspectorFooter.js index 013e3a3d64..e10a5c16e1 100644 --- a/Libraries/LogBox/UI/LogBoxInspectorFooter.js +++ b/Libraries/LogBox/UI/LogBoxInspectorFooter.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @flow strict-local + * @flow * @format */ @@ -21,9 +21,18 @@ import * as LogBoxStyle from './LogBoxStyle'; type Props = $ReadOnly<{| onDismiss: () => void, onMinimize: () => void, + hasFatal: boolean, |}>; function LogBoxInspectorFooter(props: Props): React.Node { + if (props.hasFatal) { + return ( + + + + ); + } + return ( @@ -54,6 +63,55 @@ function FooterButton(props: ButtonProps): React.Node { ); } +function FatalButton(): React.Node { + return ( + + require('../../Utilities/DevSettings').reload('LogBox fatal') + } + style={fatalStyles.button}> + + Reload + + Fatal errors require a full reload + + + + + ); +} + +const fatalStyles = StyleSheet.create({ + button: { + flex: 1, + }, + content: { + alignItems: 'center', + height: 60, + justifyContent: 'center', + }, + subtext: { + height: 60, + }, + label: { + color: LogBoxStyle.getTextColor(1), + fontSize: 14, + fontWeight: '600', + includeFontPadding: false, + lineHeight: 20, + }, + subtextLabel: { + color: LogBoxStyle.getTextColor(0.8), + fontSize: 11, + includeFontPadding: false, + lineHeight: 12, + }, +}); + const buttonStyles = StyleSheet.create({ button: { flex: 1, @@ -67,7 +125,7 @@ const buttonStyles = StyleSheet.create({ color: LogBoxStyle.getTextColor(1), fontSize: 14, includeFontPadding: false, - lineHeight: 18, + lineHeight: 20, }, }); diff --git a/Libraries/LogBox/UI/LogBoxInspectorHeader.js b/Libraries/LogBox/UI/LogBoxInspectorHeader.js index 9d5ecc4977..19edfbf348 100644 --- a/Libraries/LogBox/UI/LogBoxInspectorHeader.js +++ b/Libraries/LogBox/UI/LogBoxInspectorHeader.js @@ -59,6 +59,22 @@ function LogBoxInspectorHeader(props: Props): React.Node { ); } +const backgroundForLevel = (level: LogLevel) => + ({ + warn: { + default: LogBoxStyle.getWarningColor(), + pressed: LogBoxStyle.getWarningDarkColor(), + }, + error: { + default: LogBoxStyle.getErrorColor(), + pressed: LogBoxStyle.getErrorDarkColor(), + }, + fatal: { + default: LogBoxStyle.getFatalColor(), + pressed: LogBoxStyle.getFatalDarkColor(), + }, + }[level]); + function LogBoxInspectorHeaderButton( props: $ReadOnly<{| disabled: boolean, @@ -69,16 +85,7 @@ function LogBoxInspectorHeaderButton( ): React.Node { return ( {props.disabled ? null : ( @@ -96,7 +103,7 @@ const headerStyles = StyleSheet.create({ alignItems: 'center', aspectRatio: 1, justifyContent: 'center', - marginTop: 3, + marginTop: 5, marginRight: 6, marginLeft: 6, marginBottom: -8, @@ -108,6 +115,9 @@ const headerStyles = StyleSheet.create({ }); const styles = StyleSheet.create({ + fatal: { + backgroundColor: LogBoxStyle.getFatalColor(), + }, warn: { backgroundColor: LogBoxStyle.getWarningColor(), }, diff --git a/Libraries/LogBox/UI/LogBoxInspectorMessageHeader.js b/Libraries/LogBox/UI/LogBoxInspectorMessageHeader.js index 46359d8977..4a2bff04b3 100644 --- a/Libraries/LogBox/UI/LogBoxInspectorMessageHeader.js +++ b/Libraries/LogBox/UI/LogBoxInspectorMessageHeader.js @@ -51,7 +51,7 @@ function LogBoxInspectorMessageHeader(props: Props): React.Node { - {props.level === 'warn' ? 'Warning' : 'Error'} + {{warn: 'Warning', error: 'Error', fatal: 'Fatal Error'}[props.level]} {renderShowMore()} @@ -106,6 +106,9 @@ const messageStyles = StyleSheet.create({ error: { color: LogBoxStyle.getErrorColor(1), }, + fatal: { + color: LogBoxStyle.getFatalColor(1), + }, messageText: { color: LogBoxStyle.getTextColor(0.6), }, diff --git a/Libraries/LogBox/UI/LogBoxStyle.js b/Libraries/LogBox/UI/LogBoxStyle.js index e1aa75fdf2..c01f8cb7f3 100644 --- a/Libraries/LogBox/UI/LogBoxStyle.js +++ b/Libraries/LogBox/UI/LogBoxStyle.js @@ -30,6 +30,14 @@ export function getWarningDarkColor(opacity?: number): string { return `rgba(224, 167, 8, ${opacity == null ? 1 : opacity})`; } +export function getFatalColor(opacity?: number): string { + return `rgba(243, 83, 105, ${opacity == null ? 1 : opacity})`; +} + +export function getFatalDarkColor(opacity?: number): string { + return `rgba(208, 75, 95, ${opacity == null ? 1 : opacity})`; +} + export function getErrorColor(opacity?: number): string { return `rgba(243, 83, 105, ${opacity == null ? 1 : opacity})`; } diff --git a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js index 5ecd910b14..d49ad9a56f 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxContainer-test.js @@ -137,4 +137,86 @@ describe('LogBoxContainer', () => { expect(output).toMatchSnapshot(); }); + + it('should render fatal before warn and error', () => { + const output = render.shallowRender( + {}} + onDismissWarns={() => {}} + onDismissErrors={() => {}} + logs={ + new Set([ + new LogBoxLog( + 'fatal', + { + content: 'Some kind of fatal message', + substitutions: [], + }, + [], + 'Some kind of fatal message', + [], + ), + new LogBoxLog( + 'warn', + { + content: 'Some kind of message', + substitutions: [], + }, + [], + 'Some kind of message', + [], + ), + new LogBoxLog( + 'error', + { + content: 'Some kind of message (latest)', + substitutions: [], + }, + [], + 'Some kind of message (latest)', + [], + ), + ]) + } + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render most recent fatal', () => { + const output = render.shallowRender( + {}} + onDismissWarns={() => {}} + onDismissErrors={() => {}} + logs={ + new Set([ + new LogBoxLog( + 'fatal', + { + content: 'Should not be selected', + substitutions: [], + }, + [], + 'Some kind of fatal message', + [], + ), + new LogBoxLog( + 'fatal', + { + content: 'Should be selected', + substitutions: [], + }, + [], + 'Some kind of message', + [], + ), + ]) + } + />, + ); + + expect(output).toMatchSnapshot(); + }); }); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js index 42f85d220b..d6aeb38dfb 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspector-test.js @@ -37,6 +37,16 @@ const logs = [ 'Some kind of message (second)', [], ), + new LogBoxLog( + 'fatal', + { + content: 'Some kind of message (third)', + substitutions: [], + }, + [], + 'Some kind of message (third)', + [], + ), ]; describe('LogBoxContainer', () => { @@ -48,6 +58,7 @@ describe('LogBoxContainer', () => { onChangeSelectedIndex={() => {}} logs={[]} selectedIndex={0} + hasFatal={false} />, ); @@ -62,20 +73,22 @@ describe('LogBoxContainer', () => { onChangeSelectedIndex={() => {}} logs={logs} selectedIndex={0} + hasFatal={false} />, ); expect(output).toMatchSnapshot(); }); - it('should render error with selectedIndex 1', () => { + it('should render fatal with selectedIndex 2', () => { const output = render.shallowRender( {}} onMinimize={() => {}} onChangeSelectedIndex={() => {}} logs={logs} - selectedIndex={1} + selectedIndex={2} + hasFatal={true} />, ); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js index b3ccafe0dc..da70c9f69c 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorFooter-test.js @@ -18,7 +18,23 @@ const render = require('../../../../jest/renderer'); describe('LogBoxInspectorFooter', () => { it('should render two buttons', () => { const output = render.shallowRender( - {}} onDismiss={() => {}} />, + {}} + onDismiss={() => {}} + hasFatal={false} + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render fatal button', () => { + const output = render.shallowRender( + {}} + onDismiss={() => {}} + hasFatal + />, ); expect(output).toMatchSnapshot(); diff --git a/Libraries/LogBox/UI/__tests__/LogBoxInspectorMesageHeader-test.js b/Libraries/LogBox/UI/__tests__/LogBoxInspectorMesageHeader-test.js index feca3ceb99..51dd106b30 100644 --- a/Libraries/LogBox/UI/__tests__/LogBoxInspectorMesageHeader-test.js +++ b/Libraries/LogBox/UI/__tests__/LogBoxInspectorMesageHeader-test.js @@ -17,13 +17,29 @@ const LogBoxInspectorMessageHeader = require('../LogBoxInspectorMessageHeader') const render = require('../../../../jest/renderer'); describe('LogBoxInspectorMessageHeader', () => { - it('should not render error', () => { + it('should render error', () => { const output = render.shallowRender( {}} + />, + ); + + expect(output).toMatchSnapshot(); + }); + + it('should render fatal', () => { + const output = render.shallowRender( + {}} diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap index 8f3685d15b..562f154bb3 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxContainer-test.js.snap @@ -91,6 +91,138 @@ exports[`LogBoxContainer should render both an error and warning 1`] = ` `; +exports[`LogBoxContainer should render fatal before warn and error 1`] = ` + + + +`; + +exports[`LogBoxContainer should render most recent fatal 1`] = ` + + + +`; + exports[`LogBoxContainer should render null with no logs 1`] = `null`; exports[`LogBoxContainer should render the latest error 1`] = ` diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap index a609973125..75dafd41a0 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspector-test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`LogBoxContainer should render error with selectedIndex 1 1`] = ` +exports[`LogBoxContainer should render fatal with selectedIndex 2 1`] = ` @@ -60,7 +61,7 @@ exports[`LogBoxContainer should render warning with selectedIndex 0 1`] = ` level="warn" onSelectIndex={[Function]} selectedIndex={0} - total={2} + total={3} /> diff --git a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap index 600a765b9a..14a7af6364 100644 --- a/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap +++ b/Libraries/LogBox/UI/__tests__/__snapshots__/LogBoxInspectorFooter-test.js.snap @@ -1,5 +1,26 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`LogBoxInspectorFooter should render fatal button 1`] = ` + + + +`; + exports[`LogBoxInspectorFooter should render two buttons 1`] = ` `; -exports[`LogBoxInspectorMessageHeader should not render error 1`] = ` - - - - Error - - - - - - -`; - exports[`LogBoxInspectorMessageHeader should render "collapse" if expanded 1`] = ` `; + +exports[`LogBoxInspectorMessageHeader should render error 1`] = ` + + + + Error + + + + + + +`; + +exports[`LogBoxInspectorMessageHeader should render fatal 1`] = ` + + + + Fatal Error + + + + + + +`;