-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Add custom highWaterMark option to clone method #563
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
Conversation
|
Many thx for this PR, I will look into as soon as I am available, it’s a much needed improvement.
If anyone on the node-fetch team want to comment on this please feel free to. It’s not a part of the Fetch spec but browsers definitely have their ways of handling large streams.
(From my phone)
… On Jan 7, 2019, at 4:05, Radek Matěj ***@***.***> wrote:
Fixes #386.
I've spent enormous time investigating the conditions when backpressure occurs, yet I still have a lot of unanswered questions. Although I hate uncertainties, the formula I introduced is a verified hypothesis. I'm well prepared and open to any discussions about this topic. :)
I would appreciate any feedback on the code, especially tests.
You can view, comment on, or merge this pull request online at:
#563
Commit Summary
Test clone timeout
Mock response instead test-specific URL
Test resolved clone
Create chai helper for testing promise timeout
Move chai timeout helper to separate file
Lower timeout value to speed up test
Return mock response URL from mockResponse
Add highWaterMark param to clone method
Update information about clone parameter
Add more extensive highWaterMark documentation
Test described highWaterMark formula
File Changes
A CLONE-HIGHWATERMARK.md (108)
M LIMITS.md (11)
M src/body.js (10)
M src/response.js (5)
A test/chai-timeout.js (17)
M test/server.js (14)
M test/test.js (67)
Patch Links:
https://github.com/bitinn/node-fetch/pull/563.patch
https://github.com/bitinn/node-fetch/pull/563.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Hey @nikdo, I looked into it and while your documentation is very good. I am thinking about 2 things:
|
|
What's the status of this PR? What can I do to help advance it? It's a pretty serious problem considering a lot of users using |
|
@gutenye We are currently in process of preparing the I've just submitted a pull request, that fixes this problem, but in a preferred way 😄 |
|
Let's close this then. |
Fixes #386.
I've spent enormous time investigating the conditions when backpressure occurs, yet I still have a lot of unanswered questions. Although I hate uncertainties, the formula I introduced is a verified hypothesis. I'm well prepared and open to any discussions about this topic. :)
I would appreciate any feedback on the code, especially tests.