Skip to content
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

[Bug?]: Unable to use ErrorBoundary when throwing Error from cached function in combination with load #399

Closed
2 tasks done
bondehagen opened this issue Apr 1, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@bondehagen
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Node process exits with unhandled exception

Expected behavior 🤔

Load function should ignore errors and ErrorBoundary should get triggered.

Steps to reproduce 🕹

Steps:

import { Show, ErrorBoundary } from "solid-js";
import { type RouteDefinition, type RouteSectionProps, cache, createAsync } from "@solidjs/router";
import { HttpStatusCode } from "@solidjs/start";

export const route = {
  load: ({ params }) => getHouse(params.house)
} satisfies RouteDefinition;

const getHouse = cache(async (house: string) => {
  "use server"
  if (house != "gryffindor") {
    throw new Error("House not found");
  }
  return house;
}, "house");

export default function House(props: RouteSectionProps) {
  const house = createAsync(() => getHouse(props.params.house), {
    deferStream: true,
  });
  return (
    <ErrorBoundary
      fallback={(e) => (
        <Show when={e.message === "House not found"}>
          <HttpStatusCode code={404} />
          <h1>House not found</h1>
        </Show>
      )}
    >
      <div>{house()}</div>
    </ErrorBoundary>
  );
}
  1. Run http://localhost:3000/house/gryffindor and make sure that page works
  2. Run http://localhost:3000/house/notgryffindor and see that node process fail with unhandlet exception

Context 🔦

It should be possible to handle errors in combination with preloading data.

Your environment 🌎

System:
 OS: Windows

Node: v21.1.0

Packages:
 vinxi: 0.3.11
 solid-js: 1.8.16
 @solidjs/router: 0.13.1
 @solidjs/start: 1.0.0-rc.0
@bondehagen bondehagen added the bug Something isn't working label Apr 1, 2024
@ryansolid
Copy link
Member

Yeah this looks like a router issue. I'm going to move it there.

@ryansolid ryansolid transferred this issue from solidjs/solid-start Apr 8, 2024
@ryansolid
Copy link
Member

Interesting.. The problem here is that unless the promises are awaited we can't catch them. They will be uncaught. Like I can't just wrap the load function in a try {} finally {}. It won't do anything. But it also doesn't make a ton of sense to await them here. In the browser this would be inconsequential but I'm gathering the node process is exiting.

@paularmstrong
Copy link

paularmstrong commented Apr 15, 2024

This just hit me as well. The node process is exiting on the uncaught exception. It does not require having a route load function for me:

node:internal/process/promises:289
            triggerUncaughtException(err, true /* fromPromise */);
            ^

Error
    at eval (/.../src/routes/path/to/file.tsx?pick=route:20:11)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

@indeyets
Copy link

Connecting .catch(…) to them (like we did with promises before async/await was invented) should probably still work.

@ryansolid
Copy link
Member

For the end user but not something we can do with the library. I mean hmm.. I suppose the cached functions could not throw under some circumstances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants