search

solidjs / solid-start

SolidStart, the Solid app framework
https://start.solidjs.com
MIT License
5.22k stars 372 forks source link

[Bug]: Actions with latest `netlify` preset return 500 due to `Response body object should not be disturbed or locked` #1673

Open serhalp opened 3 weeks ago

serhalp commented 3 weeks ago

Duplicates

Latest version

Current behavior 😯

The page renders "⚠️ Oops, something went wrong".

This is because the Action's POST /_server returns a 500.

This error is logged on the server:

[nitro] [request error] [unhandled] Response body object should not be disturbed or locked

Expected behavior 🤔

The page should render "✅ Correct!" because the Action returns true

This works as expected with:

Steps to reproduce 🕹

Steps:

with https://github.com/serhalp/repro-solidstart-nitro-response-body-disturbed (pnpm i) and netlify-cli (npm i -g netlify-cli)

  1. ntl serve
  2. click "click me"

or

  1. create a Netlify site and link it (e.g. ntl init)
  2. ntl deploy --build
  3. click "click me"

I've also deployed it here: https://672c1abc039334ad67383d7e--repro-solidstart-response-disturbed.netlify.app/.

Context 🔦

Seems to be similar to https://github.com/solidjs/solid-start/pull/1577...?

If I add some logging here (necessary because the error handling in that function strips the stack trace and all details outside of dev but this bug can only be reproduced in "real" environments) I can see that this is throwing from here.

Your environment 🌎

macOS Monterey 12.7.1 (21G920) Firefox 131.0.3

$ uname -a
Darwin [...] 21.6.0 Darwin Kernel Version 21.6.0: Wed Oct  4 23:55:28 PDT 2023;
$ node -v
v22.9.0
$ pnpm -v
9.6.0
serhalp commented 3 weeks ago

@pi0 I see that in many other presets we're reading in the whole body stream and then passing it back to Nitro as a Buffer:

I suspect this is related. In both cases I can see this being added specifically to fix something unspecified.

Do we need to do something like this in the new netlify preset? If so, I'm happy to open a PR but can you point me to the underlying issue or reason? and I wonder if there's some day we can fix this at the source (is that what https://github.com/nitrojs/nitro/issues/1721#issuecomment-2425419583 is trying to do?) or at least make it harder to regress.

pi0 commented 3 weeks ago

Considering Nitro is moving to web standards, we should fix the root cause (other presets will be updated progressively).

Can we reproduce it with (unmodified) Nitro alone? (if not it seems a bug with solid/vinxi)

serhalp commented 2 weeks ago

Whew, finally found the time to try to create a Nitro repro of this, but it seems to work just fine:

So it seems to be specific to SolidStart. Any ideas @ryansolid? or ideas for workarounds?

I could maybe look into fixing myself if either of you could give me a little guidance here as there's, uhh, quite a lot going on here I'm a bit wary of diving in without understanding the stories behind all these workarounds. Thanks!

ryansolid commented 1 week ago

I admit I have no idea. All the hacks in there were for getting around previous issues we had in various envs and I wasn't involved in implementing any of them. I verified them on various platforms but for the most part it was due to gaps in adapters for those envs. It's possible some of them are no longer needed. If you look one of the comments sounds almost like it was to fix an issue similar to what you are facing by cloning the request rather than using a locked one. It is interesting that no longer works?...

serhalp commented 1 week ago

@ryansolid Thanks, Ryan.

If you look one of the comments sounds almost like it was to fix an issue similar to what you are facing by cloning the request rather than using a locked one. It is interesting that no longer works?...

Yeah, I'd have to check again but when I poked around with some added logging a few weeks ago it seemed like this instanceof was false even though it seemed to be a ReadableStream. So maybe there are two copies of that class defined making it not pass the instanceof check... 🤷🏼

I guess I'll try to dig further when I can find some time.