Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate size option #1438

Open
jimmywarting opened this issue Jan 1, 2022 · 1 comment
Open

Deprecate size option #1438

jimmywarting opened this issue Jan 1, 2022 · 1 comment
Labels
Milestone

Comments

@jimmywarting
Copy link
Collaborator

jimmywarting commented Jan 1, 2022

The size option have always been a node-fetch only option. It's not in the spec.
So I would like to deprecate/remove it also.

Describe the solution you'd like

In the attempts to align with the spec more correctly and having code that works the same way across undici, deno, browsers and node-fetch i would like for the size option to go away into a own separate user-land module.

Limiting the size can be tricky

  • it should really be using AbortController / AbortSignal to abort the request.
  • it needs to check content-length and take compression into consideration.
  • when no content-length is available it should stream & calculate & abort when exceeding.
  • weather or not it should handle redirects manually.

Describe alternatives you've considered

  • a own seperate module for handling size (similar to how we did it with timeout and provided a timer-signal)
  • Another possible solution is if we where to have fetchObserver and/or fetchEvent then one could also use AbortController to abort request in another way

Additional context

No other fetch impl have size limit

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Jan 2, 2022

we shipped size option way early before when there was not any possible way to abort a fetch... that may have been the biggest reason why it was added in the first place...

for now anybody would do:

const ctrl = new AbortController()
const { signal } = ctrl

const res = await fetch(url, { signal })
const len = res.headers.get('content-length')

if (len && Number(len) > 1024) {
  // to large, abort the response
  ctrl.abort()
}

await res.text()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant