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

Experimental rewriting of POST request does not work (reopened) #11247

Closed
gersomvg opened this issue Jun 13, 2024 · 6 comments · Fixed by #11258
Closed

Experimental rewriting of POST request does not work (reopened) #11247

gersomvg opened this issue Jun 13, 2024 · 6 comments · Fixed by #11258
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@gersomvg
Copy link

gersomvg commented Jun 13, 2024

Astro Info

Astro                    v4.10.2
Node                     v20.12.2
System                   macOS (arm64)
Package Manager          pnpm
Output                   server
Adapter                  @astrojs/node
Integrations             @astrojs/tailwind

If this issue only occurs in one browser, which browser is a problem?

Not specific to a browser

Describe the Bug

Duplicate of this previous issue that I can't reopen: #11127

It is supposed to be fixed in Astro 4.10 but the exact same stackblitz repro still doesn't work.

I get the error Response body object should not be disturbed or locked

What's the expected result?

I'd expect /page-b to render and receive the email field in FormData

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-oyssmw?file=src%2Fpages%2Findex.astro

@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jun 13, 2024
@gersomvg gersomvg changed the title Experimental rewriting of POST request does not work Experimental rewriting of POST request does not work (reopened) Jun 13, 2024
@ascorbic
Copy link
Contributor

ascorbic commented Jun 14, 2024

The issue here is that even though the body is copied, you have already read it. When you call request.formData() it consumes the body, so when you try to rewrite it it can't read the body again. The fix is to change the code to: const data = await Astro.request.clone().formData(), so the original request body is not disturbed. This is what Astro Actions do. I'm not sure if this is something that Astro should do automatically for everyone, because it's a rare case, but maybe it could at least handle the error better. Perhaps if Astro.rewrite is called when the body has been consumed we could explain that you should clone the body if you intend to rewrite the request afterwards.

@ascorbic ascorbic self-assigned this Jun 14, 2024
@ascorbic ascorbic added the - P2: has workaround Bug, but has workaround (priority) label Jun 14, 2024
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Jun 14, 2024
@ematipico
Copy link
Member

ematipico commented Jun 14, 2024

I think this must be done in userland. Using a body after it's been consumed (or read) is most of the time invalid.

@ascorbic
Copy link
Contributor

I'll see if I can make it give a more helpful error

@gersomvg
Copy link
Author

gersomvg commented Jun 15, 2024

@ascorbic good catch. I didn't realise that something that's a read action – in my mind at least –, could cause something like this. Thanks for also providing the proper way to do this.

@ascorbic
Copy link
Contributor

ascorbic commented Jun 17, 2024

@gersomvg yeah, it's because all read actions on a request (or response) are actually stream methods, and once you've consumed a stream you can't "unstream" it. The clone() method is neat, because it splits the stream (using tee()), so the original stream is then piped to both the original and new body streams. You have to clone it before you read the original body though.

@gersomvg
Copy link
Author

@ascorbic Learned something new today 🎊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants