No Clear way to abort XmlHttpRequest about axios HOT 38 CLOSED

is this still on the roadmap?

from axios.

I like the idea of it being ignored since you actively choose to abort the
request. No need to send an error that I have to do extra work to handle.
On Jul 25, 2015 3:46 PM, "Patrick Stoica" [email protected] wrote:

Also, maybe you'd want to return another promise as the result of an abort?

—
Reply to this email directly or view it on GitHub
#50 (comment).

from axios.

@pstoica what would you want to do with an aborted request?

function fetchListData() {
  // Abort request already in-flight, in case user hits "Refresh" repeatedly
  axios.abort('fetch-list-data');

  axios.get('/list/data', { requestId: 'fetch-list-data' })
    .then(function (response) {
      // Populate DOM with data, or whatever
    })
    .catch(function (response) {
      if (response instanceof AbortedRequestError) {
        // What would I even do with this?
      } else {
        // Handle 404, 500, et al, or some other Error
      }
    });
}

document.getElementById('refresh-list-data').onclick = fetchListData;

from axios.

BTW, my arguments against silent aborts:

• ES7 async functions will never continue over await calls. It is better to have an AbortedError or something so the async functions can complete execution.
• Some promise libraries tend to keep a registry of running promises for lifecycle management etc. Bluebird recently added such features, I'm not sure about the implementation details but forgotten promises might lead up to memory leaks.
• Easier to check and ignore an aborted exception than to work around that if you need the promise to resolve / reject.

I think .abort should just reject with an error.

from axios.

👍 User interactions on my app can cause requests, further action can make previous actions invalid and necessary. Right now all requests remain in flight and I have to have to track state to determine if I should use the response. Instead it would be nice to just abort the previous requests and start a new one. That way I only ever have one of a certain type of request in flight.

I recently switched from superagent to this library, and this is the main feature I am in serious need of before I can release the code that switches over. It is times like these I wish libraries just used callbacks instead of promises :(.

from axios.

How about providing 2 functions instead of an abort?

Something like:

let req = axios.post(...).then(...);

// Would abort the request in flight but resolve the promise to supplied object.
req.resolve({ foo: 'bar' });

// Would abort the request in flight and reject the promise if an argument is supplied
// Can be aliased as `abort`.
req.reject(new Error("Aborting Request"));

Or making it configurable?

req.abort({ resolve: {foo: 'bar'} });
req.abort({ reject: new Error("Foo") });
req.abort(); //silent

from axios.

This is a common problem with using Promises. There is no way to return the XHR object from axios since the Promise is already being returned. I have had a lot of requests for this behavior though, and have given it a lot of thought. I actually had an idea last night that I want to play with. I will let you know if it pans out.

from axios.

If you're interested in seeing my proposal, I have it in a Gist https://gist.github.com/mzabriskie/ec3a25dd45dfa0de92c2

from axios.

TL;DR How should aborting a request be handled? Enter an error flow, or act as if request wasn't made (https://www.thenpoll.com/#/jbeyad)

I have a working version of aborting requests ready to commit. Though I am debating a piece of the implementation. How should aborting a request be handled? Should it be treated as an error, or ignored completely as if the request hadn't even been made? I took a look at jQuery, and superagent to see how they are handling things, and found they are not unified in their approaches.

jQuery:

var xhr = $.ajax({
  url: 'https://api.github.com/users/mzabriskie',
  success: function (data, status) {
    console.log('success', status);
  },
  error: function (xhr, status) {
    console.log('error', status);
  },
  complete: function (xhr, status) {
    console.log('complete', status);
  }
});
xhr.abort();

// Logs 'error abort', 'complete abort'

superagent:

var request = require('superagent');
var req = request
  .get('https://api.github.com/users/mzabriskie')
  .end(function (err, res) {
    console.log(err);
    console.log(res);
  });
req.abort();

// Nothing is logged

What I'm trying to decide is which approach to follow. I think that superagent has the right idea. Aborting a request should be the same as if it had never happened. It's not an error; it was explicitly canceled.

from axios.

Since you're dealing with promises, I would still expect some kind of completion where I can choose to ignore the error if I'm expecting an abort. RxJS-DOM also throws an error with a type abort: https://github.com/Reactive-Extensions/RxJS-DOM/blob/master/doc/operators/ajax.md#returns

from axios.

Also, maybe you'd want to return another promise as the result of an abort?

from axios.

I had a hard time with this one. I definitely want to be able to respond to a request cancellation because I think that'd be valuable, but I don't want to have cancel checking logic all over my error handlers... I'm thinking that having the flexibility would be worth it though.

from axios.

I'm mostly thinking of abstractions here. In the case where I'm interacting with axios directly and making a request myself like that, then I don't care as much because I'm the one who canceled so I can handle it there. However, if I have some kind of abstraction and I want to respond to cancellations in some cases but not in others, then not having the flexibility might be a pain...

But I don't have a use case for this currently, so you can ignore my opinion :-) I'm just speculating here... I suppose that if a strong use case surfaced itself, a major version change could be made to add this...

Perhaps it could be configurable on the request...............

from axios.

@kentcdodds what do you gain by having an error flow? The only way that the request will get aborted is if you, as a developer explicitly abort it axios.abort('some-id'). In which case, entering the catch provides no value. If you want to do something at the time a request is canceled, why not do it when you call axios.abort?

For me catch is what it says it is: a way to catch an error that you would have otherwise missed. This is where I would have logic for handling a request not reaching the server, or an error due to an improperly formatted request, etc.

In the example above, just move any "error" handling logic from the catch up to the line after axios.abort.

from axios.

@mzabriskie, I think we just had a race condition in our comments :-)

from axios.

@kentcdodds :-)

I had considered that, for the case of abstractions. The trouble comes with having to always check if the error was from aborting in every catch, even if you don't want that behavior. As you mentioned this could be handled by making it configurable, but that adds extra complexity within axios.

from axios.

For some reason a promise that is never resolved seems much worse than an event never firing or a callback never being called. If it was an event or callback based API, it would be easier to ignore.

Even in an extra layer of abstraction though, if I abort an ongoing operation, it is because I no longer care about the result, so needing to handle it in all of my error handling is just superfluous work. If it is important in the new abstraction, it can expose its own abort method that aborts the request and resolves/rejects/emits/calls back as it prefers.

from axios.

FWIW, Bluebird rejects cancelled promises with a CancellationError (docs). That way you can still handle cancellations if needed.

from axios.

Before this ID-based (I know, I suggested it 😅) approach lands in the API, and maybe just useful for completeness, here are two other approaches I've seen while researching this:

Cancel/abort using the promise itself

var req = axios.get(/*...*/);
axios.abort(req);

The drawbacks are that axios would need to keep track of all requests and that it only would work with the original promise (so not if we returned another one in .then).

Return an object with a abort method

var req = axios.get(/*...*/);
// req.promise would be the actual promise
req.abort();

That would be a breaking change in axios' API. But it would make it a bit more flexible, and other things could be added, like xhr onprogress handlers.

from axios.

@thetalecrafter I don't see why a Promise never being settled is any different/worse than a callback not getting invoked. Would you mind elaborating?

from axios.

@herrstucki the drawback to both of these approaches is that it would require you to pass around an object in order to abort a request. Where using an ID you only need to know what the ID is.

function uploadFile(file) {
  var data = new FormData();
  data.append('file', file);
  axios.post('/upload', data, {
    requestId: 'file-upload'
  }).then(function (response) {
    alert('File has been uploaded');
  });
}

function cancelUpload() {
  axios.abort('file-upload');
}

This doesn't look too bad, but assume the cancel function is in a cancelable loading type component from a separate file. You would have to pass the object around, vs using a constant.

That said, I prefer the API of the object with an abort method that you suggested.

from axios.

I mostly don't care because if you synchronously aborted, then you're responsible for can deal with consequences. But, I could see a case for rejections once you've abstracted a bit.

renderLoading()
Promise
  .all(stuff.map(src => Lib.load))
  .then(render, renderFail)

Lib={
  reqs:[],
  load:src=>
    (this.reqs.push(let x=axios(src)) && x),
  stop:()=>
    this.reqs.map(req=>req.abort())
}

Event.on("user scrolled", () => Lib.stop())

from axios.

I'd really prefer an optional way to respond to a request being aborted. This would be helpful for optimistic updates, where you update your UI assuming the request succeeds, and need to roll it back if it doesn't go through. Since you might not always want this behavior though, an optional listener of some kind would be grand.

from axios.

@ergason,
No worries there, failed requests already go down the Promise error path. This is specifically when you, the developer, manually abort the request.

from axios.

@jergason ^

from axios.

@jergason in your use case, what would be causing the abort?

from axios.

i think @mzabriskie is right. The whole need of catching is when the developer cannot foresee all possible errors not for an event triggered manually by him.
@jergason even if we have a use case where the user cancels the upload, the developer still has the total country of UI updates...
like

      function uploadFile(file) {
   var data = new FormData();
  data.append('file', file);
  axios.post('/upload', data, {
requestId: 'file-upload'  }).then(function (response) {
alert('File has been uploaded');
});
    }

    function unCancelButtonClicked()
 {
     axios.abort('file-upload');
      UIUpdate(''Hey you just cancelled the update");
  }

thou @mzabriskie, how sure are you that the abort() action will be carried out and that it won't come back as an error.... (I'm new to the whole promise thing).

from axios.

Leaving promises unresolved when a request is aborted is a bad idea.

The examples I see here on why one might want to abort a request are a limited set. The code that wants to abort the request may not always be directly integrated with the code making a request. And saying code calling abort should be responsible for cleanup can end up actually making code messier and/or add duplication to code.

Here's an example for a different use case. Say this is part of an application is written to use the browser's pushState API. When a user navigates to a new page this app decides it's best to abort any API request that is related to the context of the previously open page.

Say this runs as part of an AJAX form submit.

var obj = createExpensiveObject();
button.pending(); // Disable submit button and display a pending indicator
Bluebird.cast(axios.get('/api/req', {requestId: 'api-req'}))
  .then((response) => {
    // Do something with the result data and expensive object
  })
  .finally(() => {
    // Disable the pending indicator when the request has either succeeded or failed
    button.pending(false);
    // Allow the expensive object to be garbage collected
    obj = null;
  });
apiRequestIds.push('api-req'); 

And the cleanup when navigating pages runs this.

apiRequestIds.forEach((id) => {
  axios.abort(id);
});

In this code a finally is used to turn off a pending button state if the request fails or succeeds and a separate piece of code wants to abort the request. Under this use case if an abort leaves a promise hanging in the pending state either the button will never leave that state (in this case not a big deal since the button will probably disappear, but not always the case). And saying that the code doing the aborting should be responsible for aborting should be responsible for handling it is saying two bad things. A) This disconnected code that only wants to abort a HTTP request should be responsible for digging into the state of every button, etc... related to the requests and cleaning them up. Which of course can be handled by making a more complex system where abort callbacks that can handle cleanup are passed. B) That code in the .finally needs to be duplicated for aborts to work.

Additionally the finally also cleans up a reference to an expensive object. In this example, depending on whether references belonging to a promise that is pending and not resolved but references to the promise itself have been lost are garbage collected the expensive object may be collected after all. However if the reference being cleaned up is global, then you're guaranteed a leak.

from axios.

@dantman 👍

from axios.

I think rejecting the promise on abort is better than leaving it hanging. It's better to have too much information than not enough. If I'm aborting the request, I can make sure the rejection is ignored. If something else aborted, I should be informed via rejection.

My initial thought on callbacks not getting called or events not triggered vs promise unresolved is probably mostly derived emotion from terminology. "You promised you'd do it, and never even told me when you decided not to."

from axios.

Jumping in as this is affecting my current project. Have you considered what Angular does with $http? They have a timeout parameter that takes milliseconds or a promise. If that timeout promise gets resolved, the request gets aborted. For axios, it might look something like:

var cancelRequest = Promise.defer();
axios.post('/upload', data, {
  timeout: cancelRequest.promise
});

cancelRequest.resolve(); // Aborts request

This solves a couple of problems -- you can differentiate between requests that were cancelled versus requests that were aborted. Furthermore, you don't have to worry about colliding "request IDs".

from axios.

Angular has an elegant way, where you pass in a promise to abort, meaning if that promise resolves it aborts it. The cool thing with this strategy is that you can do timeout + abort with the same abstraction

e.g. axios.post(..., {timeout:somePromise})...

timeout or manual abort = somePromiseDeffered.resolve()

*edit I just saw Ralph's post above with same suggestion :D

from axios.

+1 for the way Angular does it, via resolving a promise. It has worked very well for me in practice.

from axios.

Related: https://github.com/domenic/cancelable-promise and w3c/web-animations#141. Maybe it will inspire you :)

Also: http://stackoverflow.com/questions/21781434/status-of-cancellable-promises

from axios.

I believe the best way to handle aborting a request is to steal from the Context pattern in Go. In this case, the context would be another Promise.

Consider these example invocations using Q for brevity.

cancelationDeferred = Q.defer()
p = axios.get('/user?ID=12345', { context: cancelationDeferred.promise })
pp = axios({
  method: 'post',
  url: '/user/12345',
  data: {
    firstName: 'Fred',
    lastName: 'Flintstone'
  },
  context: cancelationDeferred.promise
})

To cancel either p or pp, we'd call cancelationDeferred.reject({ foo: 'bar' }). Doing this should abort the XHR and reject both p and pp with { foo: 'bar' }.

from axios.

@Plummat Yes, it is.

from axios.

New fetch API is also promise based and has the same problem, see discussion on whatwg/fetch#27

from axios.

I think we should close this issue and work on this only in #333.

from axios.