[NO QA]Feature: Critical Error Handling #3527

kidroca · 2021-06-10T21:26:03Z

Details

Created a new ErrorBoundary component
This component can serve to warp the whole app, or specific parts like documented here: https://reactjs.org/docs/error-boundaries.html#where-to-place-error-boundaries

@react-native-firebase/crashlytics is available only for iOS and Android so a platform specific component was created to handle the logging - Android and iOS would log to E.cash server and crashlytics, while desktop and web would log only to the E.cash server (Reference: invertase/react-native-firebase#1530)

Wrapped the main component Expensify with ErrorBoundary
For the moment we're not rendering fallback UI, but just log the error that crashed React to the server and crashlytics

Fixed Issues

Fixes #3339

Tests

To test this you can trigger an exception in the render method of some component
In my sample video (iOS screenshots section) I've changed ReportScreen to throw an error when a specific report is opened

QA Steps

N/A

Tested On

I've verified the app starts and works on these platforms

Screenshots

Web

Mobile Web

Desktop

iOS

Screen.Recording.2021-06-10.at.23.20.01.mov

Android

… logging @react-native-firebase/crashlytics does not work for web and desktop

tests/unit/loginTest.js

kidroca · 2021-06-10T21:53:07Z

Here's a sample content logged to the server on error:

[alrt] [App] A Crash was intercepted by the main error boundary -
{
    "error": "This is a dev test for the Error Boundary component",
    "errorInfo": {
        "componentStack": "
    at ReportScreen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:261999:81)
    at StaticContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27865:16)
    at EnsureSingleNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27513:23)
    at SceneView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27752:21)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at Background (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:34048:20)
    at Screen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:35217:107)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at NativeScreen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:176589:81)
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:195457:32)
    at AnimatedComponentWrapper
    at MaybeScreen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:32895:23)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at MaybeScreenContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:32881:22)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:161863:83)
    at AnimatedComponentWrapper
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:161863:83)
    at AnimatedComponentWrapper
    at Dummy (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:32815:23)
    at DrawerView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:33080:81)
    at DrawerViewBase (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:32556:21)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at SafeAreaProviderCompat (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:35166:23)
    at DrawerView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:32777:26)
    at DrawerNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:31778:31)
    at MainDrawerNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:251550:69)
    at withOnyx (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:148923:85)
    at WithWindowDimensions (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:248567:83)
    at StaticContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27865:16)
    at EnsureSingleNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27513:23)
    at SceneView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27752:21)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at CardSheet (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:42498:22)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:195457:32)
    at AnimatedComponentWrapper
    at Dummy (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:40970:23)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:195457:32)
    at AnimatedComponentWrapper
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at Card (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:41684:81)
    at CardContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:42207:20)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at NativeScreen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:176589:81)
    at AnimatedComponent (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:195457:32)
    at AnimatedComponentWrapper
    at MaybeScreen (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:41532:23)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at MaybeScreenContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:41520:22)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at Background (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:35468:20)
    at CardStack (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:42766:81)
    at SafeAreaProviderCompat (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:36586:23)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at StackView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:43178:81)
    at CustomRootStackNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:251852:23)
    at AuthScreens (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:251241:81)
    at withOnyx (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:148923:85)
    at WithWindowDimensions (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:248567:83)
    at AppNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:251907:16)
    at EnsureSingleNavigator (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27513:23)
    at BaseNavigationContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:27140:27)
    at ThemeProvider (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:37335:20)
    at NavigationContainerInner (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:37016:25)
    at NavigationRoot (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:252229:81)
    at Expensify (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:237802:81)
    at withOnyx (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:148923:85)
    at BaseErrorBoundary (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:240229:81)
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at NativeSafeAreaView (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:176002:23)
    at SafeAreaProvider (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:176183:24)
    at App
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at div
    at http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:186665:25
    at AppContainer (http://localhost:8080/app-20d106c1d4efa146f27f.bundle.js:177100:24)"
    }
}

Produced by

Log.alert(errorMessage, 0, {error: error.message, errorInfo}, false);

I've deliberately inserted new lines in the printout here, the log sent to the server is condensed as one line

The error logs as "{}" in the log, probably because it's a special object Updated to pass the error message instead

The stack trace is not interesting as it just traces to the moment we call `Log.alert` in `logError` Also the `errorInfo` contains the actual stack from React

kidroca · 2021-06-10T22:28:47Z

Updated

As per my last commit messages, I've spotted some small issues

update parameters because the error message is not logging
The error logs as "{}" in the log, probably because it's a special object
Updated to pass the error message instead

refactor: remove stack trace and set captured messages
The stack trace is not interesting as it just traces to
the moment we call Log.alert in logError
Also the errorInfo contains the actual stack from React

I'm done making any more updates, until the review passes

Jag96

Such a great improvement! I added a few comments and some screenshots after testing

src/App.js

src/components/ErrorBoundary/index.native.js

tests/unit/loginTest.js

kidroca · 2021-06-11T08:59:31Z

Thanks,
The requested changes are applied

NikkiWines

This looks good to me, leaving the final review to @Jag96

Jag96

Looks good and tests well 👍

OSBotify · 2021-06-11T20:34:41Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-06-15T17:14:46Z

🚀 Deployed to staging in version: 1.0.68-5🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2021-06-25T18:33:00Z

🚀 Deployed to production in version: 1.0.73-3🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

kidroca added 3 commits June 10, 2021 23:24

feat: Crate ErrorBoundary component

4de2896

feat: wrap the root component with an error boundary

52102f0

refactor: extract platform specific implementation due to crashlytics…

5100744

… logging @react-native-firebase/crashlytics does not work for web and desktop

kidroca requested a review from a team as a code owner June 10, 2021 21:26

MelvinBot requested review from NikkiWines and removed request for a team June 10, 2021 21:26

kidroca added 2 commits June 11, 2021 00:35

refactor: use Log.alert instead of Log.info

48eb85d

test: fix failing "loginTest"

c021123

kidroca commented Jun 10, 2021

View reviewed changes

tests/unit/loginTest.js Outdated Show resolved Hide resolved

kidroca added 2 commits June 11, 2021 01:16

refactor: update parameters because the error message is not logging

80d5309

The error logs as "{}" in the log, probably because it's a special object Updated to pass the error message instead

refactor: remove stack trace and set captured messages

570845a

The stack trace is not interesting as it just traces to the moment we call `Log.alert` in `logError` Also the `errorInfo` contains the actual stack from React

Jag96 suggested changes Jun 10, 2021

View reviewed changes

src/App.js Outdated Show resolved Hide resolved

src/App.js Outdated Show resolved Hide resolved

src/components/ErrorBoundary/index.native.js Outdated Show resolved Hide resolved

tests/unit/loginTest.js Outdated Show resolved Hide resolved

refactor: address PR comments

d01196e

kidroca requested a review from Jag96 June 11, 2021 08:59

NikkiWines assigned kidroca Jun 11, 2021

NikkiWines approved these changes Jun 11, 2021

View reviewed changes

Jag96 approved these changes Jun 11, 2021

View reviewed changes

Jag96 merged commit be7fee0 into Expensify:main Jun 11, 2021

OSBotify mentioned this pull request Jun 15, 2021

Deploy Checklist: Expensify.cash 2021-06-15 #3593

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[NO QA]Feature: Critical Error Handling #3527

[NO QA]Feature: Critical Error Handling #3527

kidroca commented Jun 10, 2021 •

edited

Loading

kidroca commented Jun 10, 2021 •

edited

Loading

kidroca commented Jun 10, 2021

Jag96 left a comment

kidroca commented Jun 11, 2021

NikkiWines left a comment

Jag96 left a comment

OSBotify commented Jun 11, 2021

OSBotify commented Jun 15, 2021

OSBotify commented Jun 25, 2021

[NO QA]Feature: Critical Error Handling #3527

[NO QA]Feature: Critical Error Handling #3527

Conversation

kidroca commented Jun 10, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

kidroca commented Jun 10, 2021 • edited Loading

kidroca commented Jun 10, 2021

Jag96 left a comment

Choose a reason for hiding this comment

kidroca commented Jun 11, 2021

NikkiWines left a comment

Choose a reason for hiding this comment

Jag96 left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 11, 2021

OSBotify commented Jun 15, 2021

OSBotify commented Jun 25, 2021

kidroca commented Jun 10, 2021 •

edited

Loading

kidroca commented Jun 10, 2021 •

edited

Loading