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

Fix server functions hanging on edge functions #1255

Closed
wants to merge 3 commits into from

Conversation

oscartbeaumont
Copy link
Contributor

@oscartbeaumont oscartbeaumont commented Jan 14, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • infrastructure changes
  • Other... Please describe:

What is the current behavior?

When you call a server function when Solid Start is deployed on Deno or Vercel Edge Functions the entire function hangs.

Example page at routes/index.ts to demonstrate the issue:

const demoAction = async (name: string) => {
  "use server";

  return `Hello, ${name}`;
};

export default function Page() {
  return (
    <main class="text-center mx-auto text-gray-700 p-4 flex flex-col">
      <button onClick={() => demoAction("Oscar").then(console.log)}>
        Demo
      </button>
    </main>
  );
}

To test locally you can install Deno and use the following:

NITRO_PRESET=deno_deploy pnpm build && cd ./.output && deno run -A ./server/index.ts

The issue is the call to await request.json() within handleServerFunction freezes forever causing the request to hang until the function runtime (Eg. Vercel) times out.

It's worth noting nothing is special about this call to request.json(), it's just the place where the body is used. Any usages of the request.body will hang (including .arrayBuffer(), .text(), etc).

SolidStart calls Vinix toWebRequest which in-turn calls H3's getRequestWebStream.

Following along with the implementation of thegetRequestWebStream function:

event.web?.request?.body is undefined so we go to the next condition,
event._requestBody is undefined so we go to the next condition,
We reach the "finally" condition and create a ReadableStream and call event.node.req.on to set up listeners to the body.

However, none of the .on(...) callbacks fire in these Edge environments so the promise hangs and the entire function freezes.

I noticed that event.node.req.body is an ArrayBuffer so we can bypass the ReadableStream and just use it as body property of the Request which is what this PR does.

This issue is related to:

What is the new behavior?

The request returns correctly like it does when run using Node.js.

@oscartbeaumont oscartbeaumont changed the title Fix server actions hanging on Edge functions Fix server functions hanging on Edge functions Jan 14, 2024
@oscartbeaumont oscartbeaumont changed the title Fix server functions hanging on Edge functions Fix server functions hanging on edge functions Jan 14, 2024
@lxsmnsyc
Copy link
Member

@oscartbeaumont also would be helpful if you can point out the location of the file where handleServerFunction is declared (just for better context)

@nksaraf
Copy link
Member

nksaraf commented Jan 14, 2024

This is awesome! Thanks for finding this. We should try to get this also upstream in h3 and before that in vinxi. We want these fixes to help all the vinxi users. I want to take this whole function upstream first, @edivados has done one bit with the request caching in nksaraf/vinxi#118

I want to get this ArrayBuffer check in there too.

@nksaraf
Copy link
Member

nksaraf commented Jan 14, 2024

Upstreaming this in nksaraf/vinxi#123

nksaraf added a commit to nksaraf/vinxi that referenced this pull request Jan 14, 2024
upstreams Fix server functions hanging on edge functions solidjs/solid-start#1255
@oscartbeaumont
Copy link
Contributor Author

Sorry for the lack of detail on the issue, was rushing a bit. I have updated the original issue with a full rundown of what's going on.

@nksaraf I was just about to ping you. You raise a good point this should probably be fixed upstream.

I am working on a Nitro/h3 PR, I'm honestly not sure who is at fault.

Thanks both, am really loving Solid Start!

@oscartbeaumont
Copy link
Contributor Author

oscartbeaumont commented Jan 14, 2024

I have opened an upstream issue & PR in H3:

I am closing this issue as a pnpm update --latest --recursive updates Vinxi and patches the issue.

Thanks both for your help!

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

Successfully merging this pull request may close these issues.

3 participants