ampproject / ampproject/amp-toolbox
oneBehindFetch does not always fetch
- Dominant language
- HTML
- Stars
- 459
- Forks
- 242
- PR merge metrics
- No merged PRs in 30d
Description
This is quite bizarre. For large files, `oneBehindFetch()` does not return any response:
```js
const fetch = require("@ampproject/toolbox-core").oneBehindFetch;
fetch("https://amp.dev/manifest.json")
.then((res) => res.text())
.then((text) => console.log(`length of manifest.json = ${text.length}`));
fetch("https://amp.dev/")
.then((res) => res.text())
.then((text) => console.log(`length of index.html = ${text.length}`));
```
On node 12.16.1, this outputs:
```sh
length of manifest.json = 1136
```
(That is, the length of index.html is not output.) Removing and reordering requests, etc. does not help.
The problem here seems to be that `Response.clone()` (as indirectly used by `oneBehindFetch`) is [essentially broken](https://github.com/node-fetch/node-fetch/issues/151) for rather complicated reasons).
The recommended way of fixing this is to [resolve the cloned responses in parallel](https://github.com/node-fetch/node-fetch/blob/master/README.md#custom-highwatermark), however this is not really suitable for oneBehindFetch's use case, since it endeavours to be a drop-in replacement for `fetch()`.
This is related to #712 and #378. It may also be a factor in #650, since one of the consequences of this is that responses are not cached as expected. (Although that involved a small file, so maybe not.)
Contributor guide
Research direction
Start by locating the oneBehindFetch entry point and reproduce the issue with the Node 12.16.1 example using both amp.dev/manifest.json and amp.dev/. Trace how cloned responses are handled, using the linked node-fetch issue and custom highWaterMark guidance as context. Done means both fetch calls return and their response bodies are readable, without breaking the drop-in fetch behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100