[BUG] .size option is broken

[Pizzacus]

What

It seems that when the .size option is used, minipass-fetch expects the body size to be exactly equal to it, whereas node-fetch expects the body size to be lower.

This makes the option quite useless as it cannot be used to limit size. You can only use it if you know the exact body size in advance.

When

• Whenever .size is used in the options

Where

This is my config:

• Node.js v12.13.0
• minipass-fetch v1.2.1
• I use Windows, didn't test on Linux.

How

Steps to Reproduce

const fetch = require('minipass-fetch');

fetch('https://example.org', {
    size: 10000000
})
    .then(res => res.text())
    .then(console.log)
    .catch(console.error);

Current Behavior

FetchError: Invalid response body while trying to fetch https://example.org/: Bad data size: expected 10000000 bytes, but got 1256
    at C:\[...]\node_modules\minipass-fetch\lib\body.js:156:15
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  code: 'EBADSIZE',
  expect: 10000000,
  found: 1256,
  errno: 'EBADSIZE',
  type: 'system'
}

Expected Behavior

If node-fetch is required instead, the source code of https://example.org is printed.

.size should be a size limit. minipass-fetch should accept smaller sizes and abort the download as soon as the limit is crossed.

[isaacs]

Hm... I do actually (in npm) have some use cases where I want to fetch something, and know the exact size ahead of time, so anything larger or smaller than that size is an error.

It doesn't look like size is in the WhatWG fetch() spec, so both node-fetch and minipass-fetch are sort of forging their own path here.

Would you be happy if minipass-fetch provided a maxSize option to set a maximum, and left size to be "it must be this exact size"?

[Pizzacus]

Hmm... Not sure if that would be the best, because node-fetch uses size as the max size, and this package is meant to be a reimplementation of node-fetch.

Maybe it would be better if the current "exact size" option was moved to another option. Or maybe a new option could let the user decide how it should behave. (A .strictSize boolean?) 🤔

[isaacs]

Well, this is a reimplementation/fork of node-fetch, but I think in terms of intended API, they're both aiming to be an implementation of the WhatWG fetch() spec for Node.js. And WhatWG fetch() doesn't have a size option at all, so this is an extension of the spec either way. We're not strictly committed to matching node-fetch in every point. For example, minipass-fetch supports a trailers promise, which was dropped from the spec due to a lack of browser commitment, and never made it into node-fetch. (I might opt to drop it from minipass-fetch, since it's pretty niche anyway, but for now it's not hurting anyone.)

But you do make a good point, it would be simpler to switch back and forth between them if the options that do exist in both had the same semantics. And node-fetch came first. Ok, I'm convinced. In the use case where I'm passing size, it also passes size to cacache (where it does mean "exact size") so any errors on that will get caught, just in a different spot.

[jadbox]

I also just hit the same issue as I was expecting that size would limit the maximum size allowed to be returned.