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

response.Body stream behaviour #3256

Closed
nstott opened this issue Nov 2, 2019 · 2 comments
Closed

response.Body stream behaviour #3256

nstott opened this issue Nov 2, 2019 · 2 comments
Labels

Comments

@nstott
Copy link
Contributor

nstott commented Nov 2, 2019

In fetch, the response body is a ReadableStream with the data being pumped into the stream via the start method here: https://github.com/denoland/deno/blob/master/cli/js/fetch.ts#L25-L38

This is fine, but it has certain behavioural implications. we enqueue all the data as quickly as we can read it. there's no queue mgmt. this could cause memory problems if we have slow consumers of the data from these queues.

As an alternate implementation, we could pull the data by implementing

pull(controller: ReadableStreamController): Promise<void> {

rather then the current pump implementation. this means we get to apply a high watermark and have more queue management options
https://developer.mozilla.org/en-US/docs/Web/API/ReadableStream/ReadableStream

I'm not sure which is preferred at this point

@ry
Copy link
Member

ry commented Nov 4, 2019

Your alternative is preferable. As we stream data only a fixed amount of memory should ever be allocated - as is done with Deno.copy.

@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2021
@stale stale bot closed this as completed Jan 13, 2021
mmastrac added a commit that referenced this issue Jul 31, 2023
Includes a lightly-modified version of hyper-util's `TokioIo` utility. 

Hyper changes:

v1.0.0-rc.4 (2023-07-10)
Bug Fixes

    http1:
http1 server graceful shutdown fix (#3261)
([f4b51300](hyperium/hyper@f4b5130))
send error on Incoming body when connection errors (#3256)
([52f19259](hyperium/hyper@52f1925),
closes hyperium/hyper#3253)
properly end chunked bodies when it was known to be empty (#3254)
([fec64cf0](hyperium/hyper@fec64cf),
closes hyperium/hyper#3252)

Features

client: Make clients able to use non-Send executor (#3184)
([d977f209](hyperium/hyper@d977f20),
closes hyperium/hyper#3017)
    rt:
replace IO traits with hyper::rt ones (#3230)
([f9f65b7a](hyperium/hyper@f9f65b7),
closes hyperium/hyper#3110)
add downcast on Sleep trait (#3125)
([d92d3917](hyperium/hyper@d92d391),
closes hyperium/hyper#3027)
service: change Service::call to take &self (#3223)
([d894439e](hyperium/hyper@d894439),
closes hyperium/hyper#3040)

Breaking Changes

Any IO transport type provided must not implement hyper::rt::{Read,
Write} instead of tokio::io traits. You can grab a helper type from
hyper-util to wrap Tokio types, or implement the traits yourself, if
it's a custom type.
([f9f65b7a](hyperium/hyper@f9f65b7))
client::conn::http2 types now use another generic for an Executor. Code
that names Connection needs to include the additional generic parameter.
([d977f209](hyperium/hyper@d977f20))
The Service::call function no longer takes a mutable reference to self.
The FnMut trait bound on the service::util::service_fn function and the
trait bound on the impl for the ServiceFn struct were changed from FnMut
to Fn.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants