Add outer RemixErrorBoundary to catch root boundary-thrown errors #5012
Conversation
🦋 Changeset detectedLatest commit: f789bbd The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -875,3 +875,329 @@ test.describe("loaderData in ErrorBoundary", () => { | |||
| }); | |||
| } | |||
| }); | |||
|
|
|||
| test.describe("Default ErrorBoundary", () => { | |||
There was a problem hiding this comment.
Document request + SPA transition tests for:
- no root boundary
- root boundary
- root boundary that throws
| @@ -259,7 +259,7 @@ async function handleDocumentRequestRR( | |||
| ); | |||
| } catch (error: unknown) { | |||
| if (serverMode !== ServerMode.Test) { | |||
| console.log("Error in entry.server handleDocumentRequest:", error); | |||
| console.error("Error in entry.server handleDocumentRequest:", error); | |||
There was a problem hiding this comment.
console.error is what we used to use - and explains why we started seeing these in integration tests since we only blew away console.error
| <RouterProvider router={router} fallbackElement={null} /> | ||
| <RemixErrorBoundary | ||
| location={location} | ||
| component={RemixRootDefaultErrorBoundary} | ||
| > | ||
| <RouterProvider router={router} fallbackElement={null} /> | ||
| </RemixErrorBoundary> |
There was a problem hiding this comment.
Wrap everything in a last-resort error boundary, and send the router location in so users can back-button out of it
| if (serverMode !== ServerMode.Test) { | ||
| console.log("Error in entry.server handleDocumentRequest:", error); | ||
| } |
There was a problem hiding this comment.
This turns out to be a dup - we have a console.error in returnLastResortErrorResponse and in remix 1.7.6 that's the only one we would log so removed this to remain consistent (and since we had a unit test asserting this!). Eventually I think we'll have some hooks to get at this kind of stuff where users can do their own logging.
Needed to add back in a global
RemixErrorBoundarywrapper around the full application in case the root route threw from it's own boundary.