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

feat(proxy): stream request body with streamRequest option #374

Merged
merged 10 commits into from
Jul 25, 2023

Conversation

iainsproat
Copy link
Contributor

Example of problem and roughly-sketched implementation for #373

This PR amends the behaviour of proxyRequest to stream the proxied request to the upstream target.

Note that this PR is a rough hack and doesn't represent a polished solution.

@Hebilicious Hebilicious added the proxy label Jun 30, 2023 — with Volta.net
@Hebilicious Hebilicious added the enhancement New feature or request label Jul 4, 2023
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #374 (a6e8c65) into main (78aec63) will increase coverage by 0.00%.
The diff coverage is 84.61%.

@@           Coverage Diff           @@
##             main     #374   +/-   ##
=======================================
  Coverage   78.37%   78.38%           
=======================================
  Files          26       26           
  Lines        2793     2803   +10     
  Branches      407      408    +1     
=======================================
+ Hits         2189     2197    +8     
- Misses        604      606    +2     
Files Changed Coverage Δ
src/utils/proxy.ts 81.44% <84.61%> (-0.07%) ⬇️

@pi0 pi0 changed the title feat(proxy): stream requests feat(proxy): stream request body with streamRequest option Jul 25, 2023
@pi0 pi0 marked this pull request as ready for review July 25, 2023 17:06
@pi0
Copy link
Member

pi0 commented Jul 25, 2023

Thanks for the nice PR! With recently introduced (experimental) event.body that returns a readable stream. Based on this, i think introducing this enhancement at least behind an option flag (streamRequest) would be a nice idea. I have made some refactors to do this.

@pi0 pi0 merged commit 8c73933 into unjs:main Jul 25, 2023
6 checks passed
@iainsproat
Copy link
Contributor Author

Thanks for picking this up @pi0 - great work in adding the missing parts and getting it over the line! I'm looking forward to using this very soon :)

@pi0 pi0 mentioned this pull request Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request proxy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants