remix-run / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
29.77k stars 2.51k forks source link

Nested route ErrorBoundaries can cause "Unexpected Server Error" during SSR, if error is thrown below Outlet #9609

Open nramovs-sr opened 4 months ago

nramovs-sr commented 4 months ago

Reproduction

Go to https://stackblitz.com/edit/remix-run-remix-e3e3jp

Click link (child HAS boundary) Throw below outlet (crash)

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    @remix-run/dev: * => 2.9.2 
    @remix-run/node: * => 2.9.2 
    @remix-run/react: * => 2.9.2 
    @remix-run/serve: * => 2.9.2 
    vite: ^5.1.0 => 5.2.13

Used Package Manager

npm

Expected Behavior

A route with ErrorBoundary export should render the error boundary when runtime exception is thrown, regardless if exception was encountered above or below <Outlet />

Actual Behavior

When

  1. parent route has ErrorBoundary
  2. child route also has ErrorBoundary
  3. in parent route a component after the <Outlet /> throws an exception during SSR Remix will fail server render with "Unexpected Server Error"
brophdawg11 commented 2 months ago

I'm not sure there's anything Remix can do about this internally unfortunately. React doesn't support Error Boundaries during SSR so Remix is doing it's best to simulate them and tracks how deep we are able to render so we know the deepest successfully rendered route with an error boundary. That way when we catch an error from SSR we can look "up" to find the nearest rendered ancestor error boundary and we assign the error to that route and SSR a second time.

The problem with this approximation is that it's done in the components, but we don't really know when they start and finish rendering. So when you do:

<Throw throws={err === "1"} />
<Outlet />
<Throw throws={err === "2"} />

You successfully render <Outlet/> and that sets the withboundary route as the deepest successfully rendered route. Then after <Outlet> renders, it throws during the second <Throw> rendering and incorrectly thinks it should try to render that error in the withboundary route ErrorBoundary.

When it tries to do so, it renders the Layout component a second time on the way down to the boundary and it throws again and then Remix gives up and returns the 500 Unexpected Server Error.

I don't know if there's a way for us to detect after the withboundary child renders fine that the layout thrw after the Outlet so we can reset the tracked boundary?

One option you could do in userland today would be to use a Suspense boundary to catch SSR errors and retry client rendering. Using this in your component will cause effectively an empty <Outlet/> to render on the server and then it will retry full rendering on the client and that will properly bubble errors:

      <React.Suspense>
        <Throw throws={err === "1"} />
        <Outlet />
        <Throw throws={err === "2"} />
      </React.Suspense>

I wonder if in the future, Remix could leverage this Suspense behavior to try to replace our current "best effort" approach.

The RFC above also hints at this being better handled ion the future:

Although it's possible there will later be a separate error boundary API for the server, in the meantime, this change provides a natural way for the app to recover from error.