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

Cors issue in node 20 #3

Closed
jean343 opened this issue Nov 9, 2023 · 7 comments
Closed

Cors issue in node 20 #3

jean343 opened this issue Nov 9, 2023 · 7 comments

Comments

@jean343
Copy link

jean343 commented Nov 9, 2023

When using node 20 with Elysia, @elysiajs/cors and @bogeychan/elysia-polyfills/node/index.js, we get an error with the 204 response.

Response constructor: Invalid response status code 204

I have tracked it down to the Elysia plugin, changing the Response from

            return new Response('', {
                status: 204
            });

to the following fixes it.

            return new Response(null, {
                status: 204
            });

I am opening the issue in this repo as it probably should be fixed in the polyfills, and not in the Bun only repo.

@jean343
Copy link
Author

jean343 commented Nov 9, 2023

If it helps, I did polyfill it this way

global.Response = class extends Response {
  constructor(body: any, init: any) {
    super(body === "" && init.status === 204 ? null : body, init);
  }
};

@bogeychan
Copy link
Owner

Hi 👋

thanks for reporting!

Can you send me a small code snippet to reproduce this issue, and the version number used for the following packages, pls:

  • elysia
  • @elysiajs/cors
  • @bogeychan/elysia-polyfills

@jean343
Copy link
Author

jean343 commented Nov 13, 2023

For sure :)

Here you go https://github.com/jean343/elysia-polyfills-3

bogeychan added a commit that referenced this issue Jan 7, 2024
@BLucky-gh
Copy link

I pnmp patch-ed a8a0b15 in over v0.6.3 since it hasn't been released yet

diff --git a/src/env/headers.ts b/src/env/headers.ts
index 819e6234b17ee890c0f82526f3e86dae73e8628c..013eec21d0a9fd791b8dc34a83595344dc523825 100644
--- a/src/env/headers.ts
+++ b/src/env/headers.ts
@@ -27,6 +27,10 @@ globalThis.Request = class Request extends globalThis.Request {
 };
 
 globalThis.Response = class Response extends globalThis.Response {
+  constructor(body?: Bun.BodyInit | null, init?: Bun.ResponseInit) {
+    super(init?.status === 204 ? null : body, init);
+  }
+
   // @ts-expect-error
   get headers() {
     return new globalThis.Headers(

but I'm still getting the same error, at least on aws lambda's node 20.x

Judging by the error message, the issue might not be that there's a body included, but with trying to make a 204 response at all, which is weird (or alternatively the error message is wrong/dumb/unhelpful, which wouldn't be the first time with node)

@BLucky-gh
Copy link

BLucky-gh commented Jan 11, 2024

Nevermind it works, for some reason the patch just didn't get bundled into the lambda bundle, sorry for the misinfo

@bogeychan
Copy link
Owner

Should be fixed with 0.6.4. Let me know if the issue persists

@jean343
Copy link
Author

jean343 commented Jan 12, 2024

I confirm that the fix works, thank you.

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