[BUG] New http return types always use chunked transfer encoding

[caiges]

Is there an existing issue for this?

• I have searched the existing issues

What version of workers-rs are you using?

0.0.23

What version of wrangler are you using?

3.39.0

Describe the bug

After experimenting with the new http feature, I noticed that transfer-encoding: chunked is returned for all responses, even when manually specifying a content-length header. I'm not sure this is a problem for everyone but it's a change from earlier releases.

Steps To Reproduce

Prior release returning Content-Length: 2:

#[event(fetch)]
async fn fetch(_req: Request, _env: Env, _ctx: Context) -> Result<worker::Response> {
    let mut headers = Headers::new();
    let _ = headers.set("content-type", "application/json");
    let response = worker::Response::from_bytes("{}".into())
        .unwrap()
        .with_headers(headers);
    Ok(response)
}

HTTP/1.1 200 OK
Content-Length: 2
Content-Type: application/json

0.0.23 release returning Transfer-Encoding: chunked:

#[event(fetch)]
async fn fetch(req: HttpRequest, _env: Env, _ctx: Context) -> Result<http::Response<axum::body::Body>> {
    Ok(http::Response::builder()
    .header("content-type", "application/json")
    .header("content-length", "2")
    .body("{}".into())
    .unwrap())
}

HTTP/1.1 200 OK
Transfer-Encoding: chunked
Content-Type: application/json

[kflansburg]

I think this is probably due to our focus on supporting streaming bodies. There are probably some semantics that I missed when converting between different body types. I will try to figure out where this is coming from.

[kflansburg]

I've created a test to reproduce the issue. #516

I'm not really sure how this can be solved. The return type that we accept is http::Response<B> where B: http_body::Body<Data=Bytes>. This really just exposes a Stream-like interface.

I think that in order to have JavaScript Response return an un-chunked body, it should be constructed from new_with_opt_buffer_source_and_init (unless there is some undocumented logic about transfer-encoding header and ReadableStream). To do this, we would need to somehow detect that the body is not chunked and resolve it into a buffer. I'm not exactly sure how to do this. There is a size_hint method, but the upper bound is best effort and seems to be None in most cases.

I could special case our Body type to hold a fixed-length buffer like the legacy ResponseBody type, but I can't do anything about axum's body type.

Would appreciate any suggestions.

[kflansburg]

Ok, I think I have something sort of working. If the Body has a defined upper boundary in it's size_hint, I poll it a single time and create a web_sys::Response from that buffer.

This has a couple of limitations:

• upper is best-effort and this information is probably lost if you make a round trip through JavaScript for whatever reason (we do this to make conversion between legacy and http types easier). So this really only works if you are directly returning an http_body::Body with defined size_hint (such as an axum::Body created from &str).
• I think this involves fully copying the data at least once.
• My poll of the Body is hacky and if there are fixed-length bodies that don't resolve in a single poll, then this breaks. I think there are some helpers in http_body_util that could fix this if we are willing to make a few more things async.

Overall this doesn't feel great, but it does address the specific case described above.

[dakom]

Would appreciate any suggestions

I've been thinking about this a lot lately, diving into Workers (and loving it!) still not sure I have a strong enough grasp of the whole ecosystem to make strong suggestions...

... but one thing that feels like "the right thing to do" is to not be more restrictive than the JS API, but rather allow using Rust idioms within the JS API.

So for example, it should be possible to use a Rust Stream and have it turn into ReadableStream under the hood. Rust Option::None or Unit type should turn into plain Response constructor under the hood, etc.

I'm not sure that's achievable with the http type, they seem to be at odds at first glance- since the http type is generic and to convert into web_sys means knowing what that concrete type is. I don't think forcing it into a stream everywhere is the ultimate best approach, though it does seem like a great approach if one concrete type must be chosen.

As an alternative, what do you think about having a trait that requires into/from web_sys types? I am not sure if it can literally be bound by From/Into or just have those as methods, but either way- having it trait-based means downstream can impl it for anything. Features can be enabled to add impls for it on various http types, axum, etc. if workers really needs access to something like headers, in the library code, that can be on the trait too

I may not be seeing all the problems, but I think this also might be a way to let people transition over to http without any backwards compatibility issues, so it could be easier to get to a stable API.

Just sharing some thoughts though, no pressure to go this route!

[dakom]

opened a PR to demonstrate: #530

[caiges]

@dakom I think you make convincing arguments 👏 and it feels similar to Axum's Response trait and impl Response pattern. I'm not sure what other worker related concerns there might be with this approach so I'll let those folks chime in.