Page MenuHomePhabricator

[web] [feat/chore] [ENG-813] add error boundary to the app
ClosedPublic

Authored by benschac on Apr 21 2022, 11:02 AM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 5, 5:09 AM
Unknown Object (File)
Tue, Nov 5, 5:09 AM
Unknown Object (File)
Tue, Nov 5, 5:09 AM
Unknown Object (File)
Tue, Nov 5, 5:09 AM
Unknown Object (File)
Tue, Nov 5, 5:09 AM
Unknown Object (File)
Tue, Nov 5, 5:07 AM
Unknown Object (File)
Sat, Oct 26, 9:51 PM
Unknown Object (File)
Fri, Oct 25, 12:04 AM

Details

Summary

add error boundary to the web app, when the project crashes, end user gets a message.

https://linear.app/comm/issue/ENG-813/add-error-boundaries

Image 2022-04-21 at 1.57.40 PM.jpg (2×3 px, 180 KB)

Test Plan

This is just a stripped down version of the error-boundry.react.js file in native

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

web/error-boundary.react.js
30 ↗(On Diff #11747)

this is never used. Going to remove it.

ashoat requested changes to this revision.Apr 22 2022, 6:24 AM
ashoat added inline comments.
web/error-boundary.react.js
9–16 ↗(On Diff #11757)

Once again you're writing code without making types $ReadOnly. What is it going to take to get this right once-and-for-all, so you never submit code with this error again?

This revision now requires changes to proceed.Apr 22 2022, 6:24 AM

update types

web/error-boundary.react.js
9–16 ↗(On Diff #11757)

Obviously, not an excuse. I should have caught this on my own. I did a copy/pasta of native/error-boundry.react.js and should have double checked.

I'm going to fix this in native too. Sorry!

web/error-boundary.react.js
9–16 ↗(On Diff #11757)

What is it going to take to get this right once-and-for-all, so you never submit code with this error again?

I'm not really sure this is a constructive question/dialog to have in code review. If I knew I wouldn't have made the error in the first place.

Happy to talk IRL.

Followed up offline, @benschac has a code snippet he'll be using going forward which should hopefully help, and he's going to try to move a bit slower with stuff as a tradeoff to catch more things

This revision is now accepted and ready to land.Apr 24 2022, 4:32 PM

Added reference to the snippets in notion: https://www.notion.so/commapp/VSCode-setup-1ec2d3fefb0c416e88adce8a8ebad21c if anyone else is interested in using them.