bogeychan / elysia-polyfills

Collection of experimental Elysia.js polyfills
https://npmjs.com/package/@bogeychan/elysia-polyfills
MIT License
48 stars 2 forks source link

Cors issue in node 20 #3

Closed jean343 closed 9 months ago

jean343 commented 11 months ago

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

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

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:

jean343 commented 11 months ago

For sure :)

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

BLucky-gh commented 9 months ago

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

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

bogeychan commented 9 months ago

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

jean343 commented 9 months ago

I confirm that the fix works, thank you.