solidjs / solid-router

A universal router for Solid inspired by Ember and React Router
MIT License
1.14k stars 146 forks source link

[Bug?]: ErrorBoundary not functioning as described in the docs #374

Closed JacobSNGoodwin closed 5 months ago

JacobSNGoodwin commented 8 months ago

Duplicates

Latest version

Current behavior 😯

I have created a basic reproducible version of using an ErrorBoundary to handle errors thrown inside of a FileRouter load function. An example of this is provided in the HttpStatusCode Page, where an Error boundary is used for handling an error thrown in a cached function.

I have produced a similar example below with a newly scaffolded version 0.5.1 solid-start app. Navigating to this page produces an uncaught error and crashes the development server.

// @refresh reload
import {
  createAsync,
  RouteDefinition,
  cache,
} from '@solidjs/router';
import { ErrorBoundary } from 'solid-js/web';

const getData = cache(async () => {
  const shouldThrow = true;

  const data = 'Bill';

  if (shouldThrow) {
    throw new Error('ya failed');
  }
  return data;
}, 'some-data');

export const route = {
  load: async () => {
    getData();
  },
} satisfies RouteDefinition;

const Home = () => {
  const data = createAsync(getData);

  return (
    <ErrorBoundary fallback={<div>Ya failed</div>}>
      <h1>Ya name: {data()}</h1>
    </ErrorBoundary>
  );
};

export default Home;

Expected behavior 🤔

The ErrorBoundary fallback property should be displayed.

Steps to reproduce 🕹

Steps:

  1. Clone the repository: https://github.com/JacobSNGoodwin/solid-error-boundary.git
  2. Install dependencies (I used both pnpm and bun
  3. Run bun dev or equivalent
  4. Open localhost:3000
  5. Observe the dev server's unhandled exception, and nothing displayed in the browser.

Context 🔦

I am trying to handle errors in general. I was recently working on an authorization callback and handler and was facing issues with the dev server crashing when cached functions (or functions which they called) threw errors.

https://github.com/JacobSNGoodwin/scavenge-solid/blob/lucia-auth/src/routes/authorize/%5Bprovider%5D.tsx

Your environment 🌎

System:
    OS:  macOS 14.2.1
    Arch: Apple M1 Pro
Binaries:
    Node: v18.16.1
    pnpm: 8.6.6
    bun: 1.0.26
npmPackages:
    "@solidjs/router": "^0.11.3",
    "@solidjs/start": "^0.5.1",
    "solid-js": "^1.8.14",
    "vinxi": "^0.2.1"
ryansolid commented 8 months ago

That's interesting. Probably because it is throwing in the load function outside of the error boundary. I wonder what is best to do there. I guess load function has no consequence so we could just squash any errors thrown during them. But generally they are throwing outside of the component at the root level. It's a good question really. Let me think about that.

JacobSNGoodwin commented 8 months ago

Thanks for considering this.

And maybe classic redirects to error pages is fine. I suppose my goal is to have some sort of fallback pattern in cases of errors. So maybe the issue is really just an education issue for myself (and others, I'm sure) on how we would do this in this kind of framework.

I know I've played with the HttpHeader and HttpStatusCode components and have seen seroval issues as well when trying to handle errors. So I have resorted to using vinxi/http (formerly re-exported vinxi/server) to try to modify the responses, but I'm not sure what solid-start does under the hood for converting between web Response and h3 responses.

ryansolid commented 7 months ago

I'm going to move this to the router. Because this is really a router issue not a Start issue.

ryansolid commented 7 months ago

It's interesting. With the latest(Start 0.6.x) this doesn't cause the dev mode to crash. But it is still difficult in that it does throw twice. The first that happens during load doesn't really do anything but it is thrown into space because we don't await the cache function and the cache function doesn't know not the throw at that point. It doesn't really break anything though. I think the immediate bug is fixed but I'm not sure I'm done here.

Please confirm the latest works for you.

JacobSNGoodwin commented 7 months ago

@ryansolid, confirmed.

I updated recently, and indeed the "end" result is working and not crashing my application.

sudhagar3 commented 6 months ago

I was banging my head against this for a couple of hours. Glad I found this thread.

Ya commenting out route load fixed the problem.

paularmstrong commented 5 months ago

Has this regressed further? #399 looks identical and I can reproduce this without a load function being present.