-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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(remix): Add v2 support #8415
Conversation
size-limit report 📦
|
3108a7e
to
8c7636e
Compare
8e4745c
to
e30946a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Just a comment about the types but once this is addressed we can merge from my PoV.
import { V2_ErrorBoundaryComponent } from '@remix-run/react/dist/routeModules'; | ||
import { captureRemixErrorBoundaryError, withSentry } from '@sentry/remix'; | ||
|
||
export const ErrorBoundary: V2_ErrorBoundaryComponent = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remix question: The ErrorBoundary is picked up during build I assume, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It covers both build and runtime. Client-side React errors, and runtime server errors also end up here. For server-side runtime errors, handleError
gives us the stacktraces (ErrorBoundary does not), so this implementation skips capturing for that case.
packages/remix/src/utils/types.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these types 1:1 copies from Remix? If yes, then let's please move them to the vendor
sub directory for consistency with the rest of our vendored code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 1:1 for all types, some of them are shallowly vendored, but still vendored :) Moving to the sub directory.
90e1118
to
6f14958
Compare
Ref: #7810
Docs: getsentry/sentry-docs#7320
Adds support for new error handling utilities of Remix v2. (ErrorBoundary, handleError)
ErrorBoundary
v2Remix's
ErrorBoundary
captures all client / server / SSR errors and shows a customizable error page.In v1, to capture client-side errors we were wrapping the whole Remix application with
@sentry/react
sErrorBoundary
which caused inconsistencies in error pages. (See: #5762)v2 implementation does not wrap user's application with
@sentry/react
's ErrorBoundary, instead it exports a capturing utility to be used inside the Remix application'sErrorBoundary
function.Can be used like:
It also requires
v2_errorBoundary
future flag to be enabled.handleError
For server-side errors apart from 'Error Responses' (thrown responses are handled in
ErrorBoundary
), this implementation exports another utility to be used inhandleError
function. The errors we capture inhandleError
also appear onErrorBoundary
functions but stacktraces are not available. So, we skip those errors incaptureRemixErrorBoundaryError
function.handleError
can be instrumented as below: