cloudflare / cloudflare/workerd
🐛 Bug Report — unused stream warning on a tee()d tee
- Dominant language
- C++
- Stars
- 8.7k
- Forks
- 739
- Avg merge
- 2d 20h
- Merged PRs (30d)
- 174
Description
Consider the following code, as part of a worker's `fetch` handler. It calls `tee()` on the request body in order to get 3 ReadableStreams, and cancels the first two:
```ts
const [a, tee] = request.body.tee();
const [b, c] = tee.tee();
await a.cancel();
await b.cancel();
return new Response(c, {
headers
});
```
This works (in that the request body is returned to the client as expected), but the following warning is printed:
> Your worker called response.clone(), but did not read the body of both clones. This is wasteful, as it forces the system to buffer the entire response body in memory, rather than streaming it through. This may cause your worker to be unexpectedly terminated for going over the memory limit. If you only meant to copy the response headers and metadata (e.g. in order to be able to modify them), use `new Response(response.body, response)` instead.
I don't think this warning is accurate, since all tee branches were either fully read (sent back as a response) or cancelled.
Looking at the `WarnIfUnusedStream` class, I wonder if the call to [`tryTee`](https://github.com/cloudflare/workerd/blob/39be22e7423d69578f0122d49506f76737329990/src/workerd/api/streams/internal.c%2B%2B#L277) should set `wasRead = true` :
```cpp
// No special behavior, just forward these verbatim.
kj::Maybe tryTee(uint64_t limit) override { return inner->tryTee(limit); }
```
Interestingly, a body that is only `tee()`d once doesn't issue any warning:
```ts
const [a, b] = request.body.tee();
await a.cancel();
return new Response(b, {
headers
});
```
-----------
Using wrangler v3.1.0
Contributor guide
Assessment
This issue has not been assessed yet.