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

500 responses when including body payload in any request using @hono/node-server/vercel #84

Open
alexiglesias93 opened this issue Sep 21, 2023 · 27 comments

Comments

@alexiglesias93
Copy link

alexiglesias93 commented Sep 21, 2023

Whenever we include a payload to a request using @hono/node-server/vercel, Hono silently fails and just returns a 500 response.
Here's a minimal repro:
https://github.com/alexiglesias93/hono-vercel-bug/blob/master/api/index.ts

Deployed:
https://hono-vercel-bug.vercel.app/api

NodeJS version in Vercel: 18.x
NodeJS version in local: 18.16.1

Error happens in both Vercel and local (using vercel dev).

I've been able to pinpoint the issue to these lines in listener.ts, after adding a console log in the catch statement I was able to find out that Vercel is failing with this error:

TypeError: Response body object should not be disturbed or locked
at extractBody (node:internal/deps/undici/undici:6342:17)
at new Request (node:internal/deps/undici/undici:7216:48)
at file:///C:/Development/hono-vercel-bug/node_modules/.pnpm/@HONO+node-server@1.2.0/node_modules/@hono/node-server/dist/listener.mjs:26:33
at Server. (file:///C:/Development/hono-vercel-bug/node_modules/.pnpm/@vercel+node@2.14.3/node_modules/@vercel/node/dist/serverless-functions/serverless-handler.mjs:14:16)
at processTicksAndRejections (node:internal/process/task_queues:95:5)

I also tried to update vercel from 29.4.0 to 32.2.5 but still getting a similar error.

Update: seems that this line in particular is the one causing the error. If I comment it out, the request goes through without errors.

Any thoughts? This is quite a blocker to implement Hono in Vercel ☹️
Let me know if I can do anything to help!

@alexiglesias93
Copy link
Author

alexiglesias93 commented Sep 21, 2023

Update: seems that this line in particular is the one causing the error. If I comment it out, the request goes through without errors.

@ErickLuis00
Copy link

+1

@yusukebe
Copy link
Member

Hi!

I started working on fixing this isse, but when I deploy, I get the following error. Are you getting the same error?

Screenshot 2023-09-27 at 18 04 50
Error: Cannot find module '/var/task/node_modules/hono/dist/index.js' imported from /var/task/api/index.js
Did you mean to import hono/dist/cjs/index.js?
    at new NodeError (node:internal/errors:405:5)
    at finalizeResolution (node:internal/modules/esm/resolve:329:11)
    at moduleResolve (node:internal/modules/esm/resolve:992:10)
    at moduleResolveWithNodePath (node:internal/modules/esm/resolve:936:12)
    at defaultResolve (node:internal/modules/esm/resolve:1178:79)
    at nextResolve (node:internal/modules/esm/loader:163:28)
    at ESMLoader.resolve (node:internal/modules/esm/loader:835:30)
    at ESMLoader.getModuleJob (node:internal/modules/esm/loader:424:18)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:77:40)
    at link (node:internal/modules/esm/module_job:76:36) {
  code: 'ERR_MODULE_NOT_FOUND'
}
Error: Runtime exited with error: exit status 1
Runtime.ExitError

The project is the same as @alexiglesias93 's .

@ErickLuis00
Copy link

@yusukebe I had the same error, I'll try to replicate it here because I forgot the reason. I'll get back to you soon. I think it was an issue with the tsconfig file; I'll check.

@ErickLuis00
Copy link

ErickLuis00 commented Sep 27, 2023

Remove "type": "module" from the package.json
Add to tsconfig.json:

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "CommonJS",
    "moduleResolution": "node",
    "esModuleInterop": true,
  },
}

I don't know what the right configuration for tsconfig is, but this one is working correctly.
If you leave tsconfig.json completely empty, with just the defaults, it works too.

@alexiglesias93
Copy link
Author

alexiglesias93 commented Sep 27, 2023

Seems weird though, I've never had any issues using "type": "module" or using the tsconfig from Hono's quickstart template 🤔
It's always worked fine for me.

@yusukebe
Copy link
Member

@ErickLuis00 Thanks, I'll try it later.

@alexiglesias93 It's weird. When I run it with the edge runtime using the Hono starter, it succeeds, but when I try to run it with Node.js, I get an error.

Anyway, I'll try again.

@alexiglesias93
Copy link
Author

alexiglesias93 commented Sep 27, 2023

I'll try to reproduce the issue too.
Do you get the error when running vercel dev or when deploying?

@yusukebe
Copy link
Member

@alexiglesias93 Only when deploying.

@yusukebe
Copy link
Member

The main issue, the error occurs when the body doesn't include a payload, is the incoming message stream is closed when it comes to the Node.js Adapter. I think this is specific to the Vercel environment and we are looking for a workaround.

@alexiglesias93
Copy link
Author

alexiglesias93 commented Sep 27, 2023

I've been able to reproduce the FUNCTION_INVOCATION_FAILED error, seems like something in Vercel has changed because I just triggered a redeploy of https://hono-vercel-bug.vercel.app/api and now it's failing, but before it was working fine (except for the bug mentioned in this Issue)

@yusukebe
Copy link
Member

Hmm, maybe Vercel changed something in the environment. I'll try again tomorrow.

@alexiglesias93
Copy link
Author

Hmm, maybe Vercel changed something in the environment. I'll try again tomorrow.

Thank you for taking the time to look at this! 🙏🏻

@yusukebe
Copy link
Member

Hmmm. I'm still getting errors in the Vercel environment.

As noted here honojs/hono#1256, I think we can get it working with a custom build, but I want to use Verbal CLI, you too right? But maybe not. Hmm.

@alexiglesias93
Copy link
Author

Yeah,, ideally we'd be able to use Vercel's CLI to get the same experience as when working with edge functions 🤔
Shall I raise an issue in Vercel's repo? Maybe they can provide some insights.

@yusukebe
Copy link
Member

yusukebe commented Oct 1, 2023

@alexiglesias93

If you are able I would like you to do that!

@alexiglesias93
Copy link
Author

@yusukebe seems like the errors magically disappeared?
I triggered a new deploy of my repro (https://github.com/alexiglesias93/hono-vercel-bug/blob/master/api/index.ts and
https://hono-vercel-bug.vercel.app/api) and now looks like the deploy works correctly and POST requests work fine when including a payload or headers.

I didn't update any package. Current versions in my repro are:

  "dependencies": {
    "@hono/node-server": "^1.2.0",
    "hono": "^3.6.0"
  },
  "devDependencies": {
    "vercel": "^32.2.5"
  }

I've been inspecting and maybe this issue is related to it? vercel/vercel#10564

@yusukebe
Copy link
Member

yusukebe commented Oct 3, 2023

@alexiglesias93

Ah, yes! the errors are gone, and it works with POST requests including a body! This might be a Vercel issue.

Though, in the vercel dev local environment, I'm still getting 500 on POST that contain a body. What about you?

@alexiglesias93
Copy link
Author

alexiglesias93 commented Oct 3, 2023

Yes, I also am getting the error when using vercel dev.

Interestingly, I tried adding the NODEJS_HELPERS=0 environment variable like suggested here and now it seems to work fine when using both vercel dev and when deploying.
I wonder if this approach has any unwanted side effects? Does Hono rely on Vercel's NodeJS helpers?

@yusukebe
Copy link
Member

yusukebe commented Oct 3, 2023

@alexiglesias93

Ah, I think so! Since the error is caused by the body being used before it reaches the Hono app handler. Can you able to disable the helper locally?

@alexiglesias93
Copy link
Author

alexiglesias93 commented Oct 3, 2023

@alexiglesias93

Ah, I think so! Since the error is caused by the body being used before it reaches the Hono app handler. Can you able to disable the helper locally?

Yes, we can select what environments a variable must be added to:

image

I'm curious to know why is it only needed in Development (local) but not in Production (deployed)? Should we add it everywhere just to be safe or is it better to only have it in Development?

@yusukebe
Copy link
Member

yusukebe commented Oct 3, 2023

Woooow, it works well! Thanks!

I'm curious to know why is it only needed in Development (local) but not in Production (deployed)? Should be add it everywhere just to be safe or is it better to only have it in Development?

I don't know, but I think we should add it everywhere.

@alexiglesias93
Copy link
Author

alexiglesias93 commented Oct 3, 2023

Great, then I guess we can close this issue? I'll re-open it if something starts failing again.
Thank you very much for all the insights @yusukebe 🙏🏻

@ccssmnn
Copy link

ccssmnn commented Oct 29, 2023

I am using Hono in a Next.js API Route. There I got the same behavior: silently failing Hono on POST requests. Configuring the API Route to avoid body parsing helped.

export const config = {
  api: {
    bodyParser: false,
  },
};

@arronKler
Copy link

the problem is probably caused by consuming the Request stream twice.

I'm using vercel dev to start my project in my local machine.
when I call ctx.req.json in a post request, it always report error with Response body object should not be disturbed or locked. I looked the call stack and I found that the json call in honojs/node-server will execute new Request(), and this operation will extracting body (this operation will check the stream is consumed or not).
image

And I found another interesting thing is vercel dev have an addHelpers function which will consume the body stream.
image

I comment this line and It worked, no error report now.
Accordding to vercel's guide, add NODEJS_HELPER=0 in your project dashboard will solve it.
https://vercel.com/docs/functions/serverless-functions/runtimes/node-js#disabling-helpers-for-node.js

So, the final solution for this issue is just add NODEJS_HELPER=0 in your vercel project.

@ramiel
Copy link

ramiel commented Mar 5, 2024

Should this be mentioned in the docs?

@cogoo
Copy link

cogoo commented Apr 2, 2024

I've investigated the issue with running vercel dev for local development and found a solution that might help others facing the same problem.

The issue can be resolved by setting the following environment variable in your local environment file:

NODEJS_HELPERS="0"

Armster15 added a commit to Armster15/honojs-website that referenced this issue Apr 7, 2024
See honojs/node-server#84
Based on manual testing, this only affects the Node js runtime on the pages router: the app router as a whole and edge API routes in the pages router are unaffected.
yusukebe pushed a commit to honojs/website that referenced this issue Apr 7, 2024
* add typescript type to next api route `config` object

* disable bodyParser in next api route when running on node js


See honojs/node-server#84
Based on manual testing, this only affects the Node js runtime on the pages router: the app router as a whole and edge API routes in the pages router are unaffected.
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

7 participants