-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add support for request cancellation using AbortController #47
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect!!
Do you think it is better to add support Example from axios const CancelToken = axios.CancelToken;
const source = CancelToken.source();
axios.get('/user/12345', {
cancelToken: source.token
}).catch(function (thrown) {
if (axios.isCancel(thrown)) {
console.log('Request canceled', thrown.message);
} else {
// handle error
}
});
axios.post('/user/12345', {
name: 'new name'
}, {
cancelToken: source.token
})
// cancel the request (the message parameter is optional)
source.cancel('Operation canceled by the user.'); We can create a CancelToken function that wrap around the AbortController and return signal as a cancel token, what do you think? |
@developit @n3tr To support the axios API I think we'll have to implement the function CancelToken(fn) {
const ac = new AbortController();
fn && fn(() => ac.abort());
return ac.signal;
}
CancelToken.source = () => {
const ac = new AbortController();
return {
token: ac.signal,
cancel: () => ac.abort()
};
}; By implementing const CancelToken = axios.CancelToken;
let cancel;
axios.get('/user/12345', {
cancelToken: new CancelToken(function executor(c) {
// An executor function receives a cancel function as a parameter
cancel = c;
})
});
// cancel the request
cancel(); Use Case 2 const CancelToken = axios.CancelToken;
const source = CancelToken.source();
axios.get('/user/12345', {
cancelToken: source.token
}).catch(function (thrown) {
if (axios.isCancel(thrown)) {
console.log('Request canceled', thrown.message);
} else {
// handle error
}
});
axios.post('/user/12345', {
name: 'new name'
}, {
cancelToken: source.token
})
// cancel the request (the message parameter is optional)
source.cancel('Operation canceled by the user.'); I have tested this locally So if you want I can push this implementation of |
Looks good. I will have to golf it down a lot, but it's nice to see what the API is supposed to be. |
@developit I pushed the changes, thought a lot about golfing it down, but no breakthrough... |
Merged master and fixed conflicts |
Thanks for putting in the effort to keep this moving forward @melwyn95, I'll take another look! |
Some great work here - will this be merged into main branch soon? |
@developit Any idea when this will be merged? Or is there something we can help with in order to get this ready for merge? |
Merges developit#47
Any plans to merge this? |
@developit @melwyn95 bumping this |
1 similar comment
@developit @melwyn95 bumping this |
As per #11 added support for request cancellation using the abort controller exposed as CancelToken.
Added test for the same.
@developit I don't know if this the correct way to do this, please let me know what to if I'm wrong