-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
Make onUncaughtError and onCaughtError Configurable #28641
Conversation
5979a9a
to
6b0f939
Compare
We don't export these default implementations so if you override them yourself, you have to reimplement them. Meaning you lose out on the reportError polyfill and the DEV only addendum that we add by default. |
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.
Sick
// Caught by error boundary | ||
if (__DEV__) { | ||
const componentNameMessage = componentName | ||
? `The above error occurred in the <${componentName}> component:` |
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.
Since this in in the same log now, does this need to say "the above error occurred in"?
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 doesn't really but it's still above within that log. I'm not sure if we want to reformat this further once we have owner stacks in DEV though since we should favor those.
I think that in that world we'd really not include this addendum at all and instead rely on the native stack traces in the console.
So I didn't think too deeply about this formatting since it'll likely go away.
`using the error boundary you provided, ${ | ||
errorBoundaryName || 'Anonymous' | ||
}.`, | ||
); |
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.
Noting for myself that this will probably need a LogBox update to the regexes
global.IS_REACT_ACT_ENVIRONMENT = true; | ||
|
||
await expect(async () => { | ||
await act(() => { |
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.
Note for myself to remember to update the act docs about throwing errors in 19
fbc9398
to
bd1e511
Compare
Pass the Fiber root to both as an argument. Since for commit phase updates, we don't have access to the root until after we schedule and update, I split the creation of class error updates into two parts. First an object that gets scheduled and then that object gets mutated once we've found the root.
bd1e511
to
3bc6080
Compare
49f2f7e
to
f01dd3d
Compare
f01dd3d
to
56874ae
Compare
358e960
to
5e04136
Compare
We don't warn for the www entry point.
5e04136
to
14edf65
Compare
Stacked on #28627. This makes error logging configurable using these `createRoot`/`hydrateRoot` options: ``` onUncaughtError(error: mixed, errorInfo: {componentStack?: ?string}) => void onCaughtError(error: mixed, errorInfo: {componentStack?: ?string, errorBoundary?: ?React.Component<any, any>}) => void onRecoverableError(error: mixed, errorInfo: {digest?: ?string, componentStack?: ?string}) => void ``` We already have the `onRecoverableError` option since before. Overriding these can be used to implement custom error dialogs (with access to the `componentStack`). It can also be used to silence caught errors when testing an error boundary or if you prefer not getting logs for caught errors that you've already handled in an error boundary. I currently expose the error boundary instance but I think we should probably remove that since it doesn't make sense for non-class error boundaries and isn't very useful anyway. It's also unclear what it should do when an error is rethrown from one boundary to another. Since these are public APIs now we can implement the ReactFiberErrorDialog forks using these options at the roots of the builds. So I unforked those files and instead passed a custom option for the native and www builds. To do this I had to fork the ReactDOMLegacy file into ReactDOMRootFB which is a duplication but that will go away as soon as the FB fork is the only legacy root. DiffTrain build for [a053716](a053716)
Stacked on facebook#28627. This makes error logging configurable using these `createRoot`/`hydrateRoot` options: ``` onUncaughtError(error: mixed, errorInfo: {componentStack?: ?string}) => void onCaughtError(error: mixed, errorInfo: {componentStack?: ?string, errorBoundary?: ?React.Component<any, any>}) => void onRecoverableError(error: mixed, errorInfo: {digest?: ?string, componentStack?: ?string}) => void ``` We already have the `onRecoverableError` option since before. Overriding these can be used to implement custom error dialogs (with access to the `componentStack`). It can also be used to silence caught errors when testing an error boundary or if you prefer not getting logs for caught errors that you've already handled in an error boundary. I currently expose the error boundary instance but I think we should probably remove that since it doesn't make sense for non-class error boundaries and isn't very useful anyway. It's also unclear what it should do when an error is rethrown from one boundary to another. Since these are public APIs now we can implement the ReactFiberErrorDialog forks using these options at the roots of the builds. So I unforked those files and instead passed a custom option for the native and www builds. To do this I had to fork the ReactDOMLegacy file into ReactDOMRootFB which is a duplication but that will go away as soon as the FB fork is the only legacy root.
Stacked on #28627. This makes error logging configurable using these `createRoot`/`hydrateRoot` options: ``` onUncaughtError(error: mixed, errorInfo: {componentStack?: ?string}) => void onCaughtError(error: mixed, errorInfo: {componentStack?: ?string, errorBoundary?: ?React.Component<any, any>}) => void onRecoverableError(error: mixed, errorInfo: {digest?: ?string, componentStack?: ?string}) => void ``` We already have the `onRecoverableError` option since before. Overriding these can be used to implement custom error dialogs (with access to the `componentStack`). It can also be used to silence caught errors when testing an error boundary or if you prefer not getting logs for caught errors that you've already handled in an error boundary. I currently expose the error boundary instance but I think we should probably remove that since it doesn't make sense for non-class error boundaries and isn't very useful anyway. It's also unclear what it should do when an error is rethrown from one boundary to another. Since these are public APIs now we can implement the ReactFiberErrorDialog forks using these options at the roots of the builds. So I unforked those files and instead passed a custom option for the native and www builds. To do this I had to fork the ReactDOMLegacy file into ReactDOMRootFB which is a duplication but that will go away as soon as the FB fork is the only legacy root. DiffTrain build for commit a053716.
Stacked on #28627.
This makes error logging configurable using these
createRoot
/hydrateRoot
options:We already have the
onRecoverableError
option since before.Overriding these can be used to implement custom error dialogs (with access to the
componentStack
).It can also be used to silence caught errors when testing an error boundary or if you prefer not getting logs for caught errors that you've already handled in an error boundary.
I currently expose the error boundary instance but I think we should probably remove that since it doesn't make sense for non-class error boundaries and isn't very useful anyway. It's also unclear what it should do when an error is rethrown from one boundary to another.
Since these are public APIs now we can implement the ReactFiberErrorDialog forks using these options at the roots of the builds. So I unforked those files and instead passed a custom option for the native and www builds.
To do this I had to fork the ReactDOMLegacy file into ReactDOMRootFB which is a duplication but that will go away as soon as the FB fork is the only legacy root.