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?]: createResource fails if using SSR and an exception is thrown in the wrapped function #1520

Closed
2 tasks done
apatrida opened this issue May 29, 2024 · 4 comments
Closed
2 tasks done
Labels
bug Something isn't working upstream Issue waiting on dependencies

Comments

@apatrida
Copy link

apatrida commented May 29, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the latest version

Current behavior 😯

Using createResource after Solid Start 1.0 fails if ssr: true and an exception is thrown in the resource. The error does not come back in myResource.error nor does the resource state transition correctly, and the error boundary is hit instead. This breaks code with no work arounds other than changing all code to return error values instead of exceptions.

See note:
https://discord.com/channels/722131463138705510/910635844119982080/1245249872156954708

and stackblitz:
https://stackblitz.com/edit/github-hzq3du-1rmrvf?file=src%2Froutes%2Findex.tsx

Expected behavior 🤔

The exception thrown inside createResource shows in the resource error property and not hitting the error boundary.

Steps to reproduce 🕹

Steps:

  1. add a createResource in a component that throws an exception
  2. put error boundary in the component
  3. use the results and error state of the resource in the JSX
  4. log in an effect the state transition, result, and error state of the effect

note that the logging and value under error state are incorrect and error boundary is hit instead.

Context 🔦

Normal behavior that worked and is documented but now does not work.

Your environment 🌎

Solid-Start 1.0.0
Solid 1.8.17
@apatrida apatrida added the bug Something isn't working label May 29, 2024
@ryansolid
Copy link
Member

ryansolid commented May 29, 2024

This behavior for createResource seems correct. We always throw on read (and always have, the .error property was added later). You can guard the read with the .error property but reading the resource itself directly throws. So if the resource read was in the fallback or vice versa it would not get caught in the ErrorBoundary.

@apatrida
Copy link
Author

The resource goes into the wrong state regardless and ends up as state ready on the error with the result and error field undefined This is the wrong state. Only happens via SSR=true

@Brendonovich
Copy link
Contributor

I don't think resources handle failed serialized promises correctly, p.value isn't passed to the third argument of loadEnd anywhere.

if ("value" in p) {
  if ((p as any).status === "success") loadEnd(pr, p.value as T, undefined, lookup);
  else loadEnd(pr, undefined, undefined, lookup);
  return p;
}

I've opened a PR to Solid to add a status === "failure" branch, which makes the repro behave correctly - the resource's status logs "errored" instead of "ready".

@ryansolid ryansolid added the upstream Issue waiting on dependencies label Jun 18, 2024
@ryansolid
Copy link
Member

This should be fixed in Solid 1.8.18

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

No branches or pull requests

3 participants