Closed domenic closed 8 years ago
It won't throw, it'll just return false.
Can you create a practical example using CancelToken.any where this is an issue?
I meant that assert
throws, not .requested
.
I'm not sure I can come up with an example where this behaviour causes an error (usually it's just unnecessary work being done despite cancel
already being called), but I would consider this result to be quite unexpected. What good is .requested
if it doesn't let us synchronously determine whether cancellation was requested?
It seems that this synchronous behaviour would also match the .net
implementation that any
was modeled after.
It does let you synchronously determine when cancelation is requested. Cancelation simply hadn't been requested yet for the returned token.
Yes, but only because cancellation is an asynchronous process when you restrict it to promise callbacks. I understand that you do it because it simplifies your spec, but does that match the user expectations? If I call cancel()
, I would expect it to take effect immediately on anything that is testing the token, not to be delayed arbitrarily by a sequence of promise jobs.
I've asked you to demonstrate a case where it violates user expectations, and am still waiting...
I hope it doesn't look too contrived:
function a(x, token) {
if (token.requested) throw Cancel.getReason(token);
return complicatedUncancellablePromise(x)
.then(…); // more `token` usage
}
const whole = CancelToken.source()
const resource = load("…").then(t => t.split("\n"));
const version = resource.then(r => Date.parse(r[0]));
const rest = resource.then(r => JSON.parse(r[1]));
version.then(v => {
if (v > startOfOperation)
whole.cancel("there's a newer version that we are working on");
});
rest.then(obj => {
const bFaster = CancelToken.source();
const combined = CancelToken.any([bFaster.token, whole.token]);
const bPromise = b(obj.y);
bPromise.then(bFaster.cancel);
return Promise.race([a(obj.x, combined), bPromise]);
})
(all this being part of a larger codebase that actually uses every single of the created objects, possibly in even more convoluted ways, with other sources cancelling whole
and other operations using it as well).
You might be able to make out my simple race
example in the code.
In this example, when we are working on an outdated version, the whole
thing gets cancelled, but a
is called nonetheless with token.requested
not yet being true
, so it unnecessarily starts the complicatedUncancellablePromise
operation.
Of course, this probably could be written in a cleaner way that makes token.requested
more predictable, but you surely know that real code bases aren't always like that.
That's why I oppose this "we already cancelled but it's not yet .requested
" limbo state somewhere between cancellation and getting to know that it happened. The .requested
property was introduced for being able to inspect cancellation state without needing to wait for an asynchronous cancellation notification callback, so why add the asynchrony back here? Why not just have clean semantics that make a cancellation request immediately known to all who are looking for it?
OK, here's a much better example:
function doA(token, cancelB) {
doSomething(X, token)
.then(doSomethingElse)
.then(resultA => {
if (token.requested) throw Cancel.getReason(token);
else cancelB();
document.getElementById("a").textContent = resultA;
}
}
function doB(token, cancelA) {
doSomething(Y, token)
.then(doSomethingElse)
.then(resultB => {
if (token.requested) throw Cancel.getReason(token);
else cancelA();
document.getElementById("b").textContent = resultB;
}
}
function doEither(whole) {
const a = CancelToken.source();
const b = CancelToken.source();
return Promise.race([
doA(CancelToken.any([a.token, whole]), b.cancel),
doB(CancelToken.any([b.token, whole]), a.cancel)
]);
}
doEither(new CancelToken(cancel => {
awaitSomethingElse.then(() => {
cancel("you're not needed any more");
document.body.innerHTML = ""; // removes #a and #b
})
});
It seems that either A or B finishes first and cancels the other, so it is expected that at most one of the two results shows up, right? Surely you've spotted the bugs already:
doSomethingElse
is synchronous and doSomething
has a caching layer. Now if X == Y
, it might return the same promise from both calls - and the a and b callbacks get scheduled together. Boom! Both results will show up because cancellation was not immediatedoSomethingElse
and awaitSomethingElse
are entangled somehow so that they both resolve in the same tick. Cancellation is requested, the document is emptied, and the a or b callbacks run - but their token.requested
is still false and they will try to access non-existing elements!
So we don't get immediate
.requested
here? This is kinda unexpected: