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

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

Open
2 tasks done
serhalp opened this issue Nov 7, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@serhalp
Copy link

serhalp commented Nov 7, 2024

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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:

  • vinxi dev (doesn't exercise the netlify preset)
  • ntl dev (doesn't exercise the netlify preset)
  • using the netlify-edge preset instead of netlify
  • removing this config or otherwise reverting to the netlify-legacy preset

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 #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 serhalp added the bug Something isn't working label Nov 7, 2024
@serhalp
Copy link
Author

serhalp commented Nov 7, 2024

@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 nitrojs/nitro#1721 (comment) is trying to do?) or at least make it harder to regress.

@pi0
Copy link

pi0 commented Nov 7, 2024

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
Copy link
Author

serhalp commented Nov 16, 2024

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
Copy link
Member

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
Copy link
Author

serhalp commented Nov 21, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants