cloudflare / cloudflare/workerd

🐛 Bug Report — unused stream warning on a tee()d tee

Open
#754 6 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.