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

Aborting a fetch #95

Closed
jimmywarting opened this issue Apr 1, 2016 · 23 comments
Closed

Aborting a fetch #95

jimmywarting opened this issue Apr 1, 2016 · 23 comments

Comments

@jimmywarting
Copy link
Collaborator

jimmywarting commented Apr 1, 2016

Implement AbortController so we can abort a request

@TimothyGu
Copy link
Collaborator

The cancelable promises proposal was recently withdrawn (see tc39/proposal-cancelable-promises#70 and whatwg/fetch#27).

New tracking issue: whatwg/fetch#447.

@SimenB
Copy link

SimenB commented Jul 19, 2017

@TimothyGu
Copy link
Collaborator

Everyone, AbortController and AbortSignal interfaces are now in the DOM Standard. Once the corresponding change has been passed in Fetch Standard we will be able to implement it in node-fetch 🎊

@bitinn
Copy link
Collaborator

bitinn commented Jul 20, 2017

@TimothyGu Thx for the heads-up, I feel sorry for not being able to help much as I am fighting on multiple fronts. Time to see if there are any takers on this issue (aka #252).

@SimenB
Copy link

SimenB commented Aug 23, 2017

Here are the tests for fetch ran in browser (if you want to check on browser implementation progress) http://wpt.fyi/fetch/api/abort

@TimothyGu
Copy link
Collaborator

The Web Platform Tests were added in web-platform-tests/wpt#6484. An overview of WHATWG's design of aborting fetch is available at web-platform-tests/wpt#6484 (comment). If it's not too hard we should look at integrating WPT into this module.

IMO we should try to implement (or endorse) a version of AbortController that can be used independent of node-fetch, like https://www.npmjs.com/package/abortcontroller-polyfill (/cc @jimmywarting).

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Aug 23, 2017

That polyfill is depending on browser due to extending DOM's event emiter.
One thing i don't like about that polyfill is that it tries to monkey patch fetch - i think it would be useful to have a independent module that other libs also can use as well.

I could release a independent module that mostly look like this:

const EventEmitter = require('events')

class AbortSignal extends EventEmitter {
  constructor() {
    super()
    this.aborted = false
  }
}

class AbortController {
  constructor() {
    this.signal = new AbortSignal()
  }
  abort() {
    this.signal.aborted = true
    this.signal.emit('abort')
  }
}

It would feel a bit more nicer to have a node emitter also

@TimothyGu
Copy link
Collaborator

@jimmywarting Yeah it would feel more natural given node-fetch's server-side context. I've already filed mo/abortcontroller-polyfill#2. Regarding monkey patching fetch, that could be lifted into a separate file to be required.

@jimmywarting
Copy link
Collaborator Author

I will make a such package now and probably call it abortcontroller

@jimmywarting
Copy link
Collaborator Author

the abortcontroller package don't have to do anything with regards to fetch, it's just node-fetch that has to use it correctly

@bradisbell
Copy link

For what it's worth, one way to cancel a Fetch in-browser is to consume the body as a stream and then close the stream. Calling .terminate() on the response.body stream from this Fetch module doesn't have the same effect.

@jimmywarting
Copy link
Collaborator Author

Good to know, although you would only be able to abort the download, not the upload

@joaovieira
Copy link

While working on the browser fetch polyfill (JakeChampion/fetch#592) I made abortcontroller-polyfill universal, so it can also work within web workers and on Node.js:

mo/abortcontroller-polyfill#12
mo/abortcontroller-polyfill#13
mo/abortcontroller-polyfill#14

While you could have nicer implementations for browser (using document) and node (using EventEmitter) separately, it would still not work for web workers. Having a universal shared module seems 👍

I'm also of the opinion this should only polyfill AbortController and not patch fetch - the latter being done by both fetch polyfills (browser and Node).

@TimothyGu
Copy link
Collaborator

A WIP is now available in #437.

@mika-fischer
Copy link

I can confirm that #437 fixes my issue in #438. However, a few questions:

  1. AFAICT, I always have to pass a signal and keep the controller, there's no way to use the one that's in the Request object by default to abort the request or response. This might be by design and is fine, just making sure I'm not missing anything.
  2. If I use a global AbortController for all requests, it seems that every fetch adds a listener to that controller, which is never removed. This will create problems in the long run, no? See my comment in the PR.

@heri16
Copy link

heri16 commented Jun 14, 2018

Does anyone have sample code for the WIP? Is this the API?

const controller = new AbortController();
const signal = controller.signal;
controller.abort();
fetch(Request("http://api.github.com", {signal}))
  .then(r => r.json())
  .then(j => console.log(j))
  .catch(err => {
      if (err.name === 'AbortError') {
          console.log('aborted');
      }
  })

@heri16
Copy link

heri16 commented Jun 14, 2018

Alright, found the http.request.abort() line that should also abort the response stream. https://github.com/bitinn/node-fetch/blob/a823a1ef74a34b9b6b25ed6cb790de8c40e4ef38/src/index.js#L68

@heri16
Copy link

heri16 commented Jun 22, 2018

Hi @TimothyGu,

I am trying to abort a slow res.json().
I cannot use timeouts as my use-case requires Promise.race between multiple fetch responses.
These lines do not work for me.

const controller = new AbortController()
const res = await fetch(url, {
    signal: controller.signal,
}).then((res) => {
    res.abortController = controller
    return res
})
setImmediate(() => res.abortController.abort())
res.body.on('error') # nothing is caught
res.body.on('abort') # nothing is caught
await res.json()

@bitinn
Copy link
Collaborator

bitinn commented Nov 5, 2018

A shoutout to people watching this thread, we are closed to merging AbortSignal support, it's a bit different from previous WIP, so let us know if you have any feedback on it: #539

@bitinn
Copy link
Collaborator

bitinn commented Nov 13, 2018

AbortSignal support is now merged and released as v2.3.0. You can use any valid implementation of AbortController to create the signal.

See README for details and examples.

@muscaiu
Copy link

muscaiu commented Jun 23, 2020

What about Pausing/Resuimg a download?

@ml054
Copy link

ml054 commented Jun 23, 2020

maybe through2 might be helpful for doing that?

@muscaiu
Copy link

muscaiu commented Jun 23, 2020

Just looked at though2 and can't think how i could use it.

I have a question here about it pausing/resuming a download.
If you have any example, i would really apreciate it, thanks!

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

Successfully merging a pull request may close this issue.

10 participants