Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix ErrorBoundary Sentry reports #5936

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Fix ErrorBoundary Sentry reports #5936

merged 1 commit into from
Jul 24, 2024

Conversation

walmat
Copy link
Contributor

@walmat walmat commented Jul 15, 2024

Fixes APP-1659

What changed (plus any additional context for devs)

Biggest change that needs tested here is the version bump of react-native-exit-app from 1.1.0 to 2. I verified it quits the app in develop mode on iOS, but we need to test all environments thoroughly.

Other changes:

  • Changed the existing error boundary to use Sentry.ErrorBoundary when not IS_TEST.
  • Also added more error context to the logger report when the user chooses restarts the app.

Screen recordings / screenshots

What to test

@linear Linear
Copy link

linear bot commented Jul 15, 2024

APP-1659 Track critical errors to sentry if we have a white / black screen

@socket-security Socket Security
Copy link

socket-security bot commented Jul 15, 2024 •
edited
Loading

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: npm/[email protected]

View full report↗︎

walmat
walmat commented Jul 15, 2024
View reviewed changes
Copy link
Contributor Author

@walmat walmat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on this I had the thought that maybe we wanted to expose a way to completely reset the app via the error boundary in case they completely cannot get around it. Thoughts?

src/components/error-boundary/Fallback.tsx Show resolved Hide resolved
@walmat walmat marked this pull request as draft July 18, 2024 17:53
@walmat
Copy link
Contributor Author

walmat commented Jul 19, 2024 •
edited
Loading

  • to confirm if rn exit app is still desired or if we just want to clear internal state management ONLY

@walmat walmat marked this pull request as ready for review July 22, 2024 15:32
@walmat
Copy link
Contributor Author

walmat commented Jul 22, 2024

Confirmed that I received logs in ErrorBoundary on a brand new project, so this is code complete. Sentry config on sentry.io needs fixed.

@walmat
Copy link
Contributor Author

walmat commented Jul 24, 2024

/rebase

@jinchung jinchung force-pushed the @matthew/APP-1659 branch 3 times, most recently from a72f6f9 to 2c1e08d Compare July 24, 2024 04:06
@jinchung jinchung force-pushed the @matthew/APP-1659 branch from 2c1e08d to 58292b5 Compare July 24, 2024 04:46
jinchung
jinchung approved these changes Jul 24, 2024
View reviewed changes
Copy link
Member

@jinchung jinchung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌮 - merge once tests pass

@jinchung jinchung merged commit 16744e0 into develop Jul 24, 2024
6 checks passed
@jinchung jinchung deleted the @matthew/APP-1659 branch July 24, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jinchung jinchung jinchung approved these changes

@benisgold benisgold Awaiting requested review from benisgold

@ibrahimtaveras00 ibrahimtaveras00 Awaiting requested review from ibrahimtaveras00

@greg-schrammel greg-schrammel Awaiting requested review from greg-schrammel

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@walmat @jinchung