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

Crash when server function throws redirect and wrapped in createAsync #429

Open
apatrida opened this issue May 26, 2024 · 3 comments
Open

Comments

@apatrida
Copy link

apatrida commented May 26, 2024

Describe the bug

If you throw a redirect from a server function wrapped in createAsync it does not handle the redirect nor serialize it across. Server functions called as actions might want to throw redirects, and if used elsewhere it is likely they would as well. cache wrapping handles the redirect, but if you do not want caching, you do not have a noCache option and therefore throw or return a redirect causing what? currently a crash.

Example:

async function serverApiCallThrow(isIndirect?: boolean): Promise<string> {
    'use server';
    throw redirect(`/${isIndirect ? 'fromIndirect' : 'fromThrow'}/whateva?q=456`);
    return 'ok'
}

...
  const asyncThing = createAsync(()=>serverApiCallThrow())
  
...
  <Show when={asyncThing()}>
      <div>boo!</div>
  </Show>

results in exception:

Error: Unknown error at castError (...server bundle...)

and client shows:

Screenshot 2024-05-26 at 8 26 41 AM

Your Example Website or App

see above

Steps to Reproduce the Bug or Issue

  1. create a server function
  2. throw a redirect in that functino
  3. use in a component with createAsync
  4. boom

Expected behavior

The redirect happens or thrown as error, but not a serialization crash

Screenshots or Videos

No response

Platform

any

Additional context

No response

@Brendonovich
Copy link
Contributor

From what I can tell this is already being thrown as a normal error and not a crash - wrapping the <Show> in an ErrorBoundary functions as you'd expect.
Perhaps just a better error message is necessary?

@maksimsemenov
Copy link

I think the issue here is that instead of handling the redirect, and navigating user to the new url, it throws an error. I don't think wrapping <Show> in ErrorBoundary will help.

From the docs it seems that only cache and action functions know how to correctly handle redirects. But I think there are cases, when we don't want to cache this function. For me it seems like missing functionality. We can try to use some workarounds, like cache revalidation, or pass some dummy random argument, so it will always produce unique cache key, but that doesn't seem as right thing to do.

@Brendonovich
Copy link
Contributor

Brendonovich commented Jun 13, 2024

Yeah cache and action are the only wrappers that handle redirects, it's possible that a third neutral one is needed.
Even then a thrown response in a raw server function surfacing as an error is still expected behaviour, it could just be reported better.
Wrapping the <Show> in ErrorBoundary doesn't help, but it functions as you'd expect any other error to (repo) rather than causing a crash which would be more worrying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants