honojs / node-server

Node.js Server for Hono
https://hono.dev
307 stars 41 forks source link

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

Open alexiglesias93 opened 9 months ago

alexiglesias93 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

+1

yusukebe commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

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

yusukebe commented 9 months ago

@alexiglesias93 Only when deploying.

yusukebe commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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

alexiglesias93 commented 9 months ago

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

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

yusukebe commented 9 months ago

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

As noted here https://github.com/honojs/hono/issues/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 commented 9 months ago

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 commented 9 months ago

@alexiglesias93

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

alexiglesias93 commented 9 months ago

@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? https://github.com/vercel/vercel/issues/10564

yusukebe commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

@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 commented 9 months ago

@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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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 commented 7 months ago

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 commented 4 months ago

Should this be mentioned in the docs?

cogoo commented 3 months ago

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"
prashantchothani commented 5 days ago

@cogoo what is the kind of framework you chose while publishing the app on Vercel ?

cogoo commented 4 days ago

@cogoo what is the kind of framework you chose while publishing the app on Vercel ? @prashantchothani

We use Hono, a lightweight app framework.

For deploying on Vercel, we wrap the app in a vercel handler:

import { Hono } from 'hono';
import { handle } from '@hono/node-server/vercel';
//.. some other code

const app = new Hono().basePath('/api');
export default handle(app);
prashantchothani commented 4 days ago

@cogoo I did the same thing. How do you then have the middleware working with each route having its own function ?

cogoo commented 4 days ago

@cogoo I did the same thing. How do you then have the middleware working with each route having its own function ?

@prashantchothani do you want to create a question on stackoverflow and send me a link, I'll be happy to help. Github issues are not a place for support requests 🙏🏾

prashantchothani commented 4 days ago

Ok will do that. Appreciate your help.