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.

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

Feat/implement sentry error boundary #262

Merged
merged 9 commits into from
Nov 26, 2020

Conversation

kwajiehao
Copy link
Contributor

@kwajiehao kwajiehao commented Nov 25, 2020

Note

This PR introduces the following environment variables to the code:

  • REACT_APP_SENTRY_ENV: either dev, staging, or production
  • REACT_APP_SENTRY_DSN: the Sentry DSN which sends error information to our Sentry account

Make sure to add these environment variables before merging to staging or production.

Overview

This PR adds client-side monitoring for IsomerCMS. We do this using Sentry, which will help us to log the stack trace of any error which occurs in our application.

We also introduce error boundaries in this PR. React error boundaries catch errors during rendering or in React lifecycle
methods (refer to the official documentation - https://reactjs.org/docs/error-boundaries.html#introducing-error-boundaries) and allow us to render a fallback component when such an error occurs. In this PR, we introduce a placeholder fallback component and we will update it when the designers hand over the design.

However, error boundaries do not catch errors for the following events:

  • Event handlers
  • Asynchronous code (e.g. setTimeout or requestAnimationFrame callbacks)
  • Server side rendering
  • Errors thrown in the error boundary itself (rather than its children)

For that we might need to use a separate library such as react-error-boundary (https://github.com/bvaughn/react-error-boundary). This can be the subject of another PR, and is logged in issue #263

Note that while we are using the Sentry error boundaries, which are wrappers over the native React error boundaries, the above errors (event handlers, async code, etc.) will not be caught by the error boundary, but it will still be captured by Sentry.

Local Testing Notes

To test that Sentry works, you can set your SENTRY_ENV to staging and throw an error in your local code (just a throw new Error('test)in one of the buttononClick` handler functions and click on the button). Then, log into Sentry and verify that the error was recorded.

Note that we only initialize Sentry in staging or production environments since we have a limited number of errors in the free tier for Sentry so we shouldn't waste them in local dev.

Jie Hao Kwa added 7 commits November 25, 2020 15:01
This commit adds a new ENV environment variable which determines
whether the environment is a local development environment, staging,
or production
This commit initializes Sentry for our app if the environment is not
a local dev environment
This commit adds Sentry error boundaries for each route component
(such as Workspace, Files, etc.). Note that the Sentry error
boundary is simply a wrapper over the built-in React error boundary,
and so does not catch errors for the following events:
- Event handlers
- Asynchronous code (e.g. setTimeout or requestAnimationFrame callbacks)
- Server side rendering
- Errors thrown in the error boundary itself (rather than its children)

Source:
- https://reactjs.org/docs/error-boundaries.html#introducing-error-boundaries
- https://stackoverflow.com/questions/46835391/react-16-error-boundary-component-using-componentdidcatch-shows-uncaught-error

Error boundaries are meant to catch errors during rendering or lifecycle
methods.
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

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

Left a comment.

Also, I wasn't able to get the env vars working without prepending them with REACT_APP, not sure if this was an issue on your side as well? Link on why it needed this: https://create-react-app.dev/docs/adding-custom-environment-variables/

src/index.jsx Outdated Show resolved Hide resolved
@kwajiehao kwajiehao force-pushed the feat/implement-sentry-error-boundary branch from 47cd1b9 to a3e5096 Compare November 26, 2020 06:00
Copy link
Contributor

@alexanderleegs alexanderleegs left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error-handling: 404 pages and toaster notifications
2 participants