Proposal: DOMException.ignoreAborts(promise)
Ported from https://github.com/whatwg/dom/issues/881.
Consider the following code we're adding as an example to the Streams Standard in https://github.com/whatwg/streams/pull/1059:
const controller = new AbortController();
const pipePromise = readable1.pipeTo(writable, { preventAbort: true, signal: controller.signal });
// ... some time later ...
controller.abort();
// Wait for the pipe to complete before starting a new one:
try {
await pipePromise;
} catch (e) {
// Swallow "AbortError" DOMExceptions as expected, but rethrow any unexpected failures.
if (e.name !== "AbortError") {
throw e;
}
}
// Start the new pipe!
readable2.pipeTo(writable);
Wouldn't it be nice if we could do this instead?
const controller = new AbortController();
const pipePromise = readable1.pipeTo(writable, { preventAbort: true, signal: controller.signal });
// ... some time later ...
controller.abort();
await DOMException.ignoreAborts(pipePromise);
// Start the new pipe!
readable2.pipeTo(writable);
The semantics here are roughly
DOMException.ignoreAborts = function (promise) {
promise = Promise.resolve(promise);
return promise.catch(e => {
if (isDOMException(e) && e.name === "AbortError") {
// swallow exception
} else {
// rethrow
throw e;
}
});
};
This is small enough that I'd be willing to implement it in Chromium, if given the binding team's blessing. (@yuki3, thoughts?)
Any thoughts from other implementers?
From a point of view of Blink-V8 bindings, IIUC, you're going to implement a new (static?) IDL operation (which takes a promise and returns a promise) and no new mechanism seems necessary. Then, no objection of course. (The prototype chain of DOMException is a bit special, but I don't think it matters in this case.)
I'm sorry that I'm not familiar with this sort of code and honestly I don't understand its usefulness. Probably it's very common, I guess? Only my question is that AbortError is so special? Do we want to ignore other kinds of errors? Like ignoreErrors(promise, [error_name1, error_name2, ...])? I have no strong feeling.
From a point of view of Blink-V8 bindings, IIUC, you're going to implement a new (static?) IDL operation (which takes a promise and returns a promise) and no new mechanism seems necessary.
Yep! I just wanted to check, since you all are really the best "owners" for DOMException.
I'm sorry that I'm not familiar with this sort of code and honestly I don't understand its usefulness. Probably it's very common, I guess? Only my question is that AbortError is so special
Yeah, "AbortError" is quite special, because it's a type of error that's often expected by web developers who have aborted something, and they want to ignore it. (Things they could abort would be, e.g., a fetch, or a stream pipe.)
Some of the original designs for promise cancelation (what is now AbortSignal) actually used a new type of completion value, not an exception and not a return value, to represent cancelations. Instead we ended up reusing the exception channel, but this leads to awkward code like the above where you want to treat most exceptions as exceptions, but ignore "AbortError"s since they represent a cancelation.
Shouldn't throw new TypeError(); be return Promise.reject(new TypeError()) or some such? I wonder if this static should be on AbortSignal or AbortController instead as it directly relates to them.
@annevk you're right, it should; will update the OP.Actually, per Web IDL rules, it should Promise.resolve() its argument. I've updated the OP in that direction.
I think a reasonable argument could be made for any of DOMException, Promise, AbortSignal, or AbortController. Just in terms of what code the author writes, DOMException.ignoreAborts() seems pretty clean to me. Also it's a bit nicer in reducing coupling; right now AbortSignal doesn't "know" about DOMException.
I think this is a great idea, though I realized after looking at some places in our codebase where we’re doing “isAbortError” checks that it seems like it might be a more niche thing than I initially supposed (or maybe we’re just weird) and that it may not be helpful in our case.
It seemed like the most common places where we have these checks is in async functions where control flow should move to the catch block if an abort occurs. If I’m not mistaken, if we tried to leverage this API, we’d end up having to move the error handling that occurs within these catch blocks elsewhere in each such case — and it might end up being a net loss for removing boilerplate / improving clarity.
async butterBiscuits({ signal }) {
try {
this.biscuits.butter = await fetch('https://butternet/the-good-stuff?lots', { signal });
} catch (err) {
if (!isAbortError(err)) {
alerts.push('a TERRIBLE thing happened');
throw err;
}
}
}
Note that if the fetch here aborts, we wouldn’t want this.biscuits.butter to be set to undefined. Sometimes there are sequentially or simultaneously awaited-things; sometimes there’s more complex handling based on e.g. HTTP status codes, etc. But in all cases I found, we’d never want DOMException.ignoreAborts(aSpecificPromiseWithinTheOverallLogic) inside the try block portion — it’s only the “top level” async logic that wants to ignore aborts.
Is there a pattern I’m missing where ignoreAborts would provide a benefit for cases like that? (Of course, there doesn’t have to be; it could be that this API is tailored specifically for cases like the initial example where you want to continue after a “child” promise gets aborted, which is still useful.)
@bathos, your example would be rewritten as follows:
async butterBiscuits({ signal }) {
try {
const result = await DOMException.ignoreAborts(fetch('https://butternet/the-good-stuff?lots', { signal }));
if (result) {
this.biscuits.butter = result;
}
} catch (err) {
alerts.push('a TERRIBLE thing happened');
throw err;
}
}
Right — so the if-statement “artifact” gets moved up, but hasn’t actually gone away. And although that example doesn’t illustrate it cause it was 1:1, it multiplies it, too — an additional new if statement for each abortable-awaited-thing, instead of just one for all of them.
I personally think it would be most natural as a method of Promise. This would have an added benefit of integrating really nicely with the await.ops proposal e.g.:
await Promise.ignoreAbort(somePromise);
// with the proposal
await.ignoreAbort somePromise;
And as an added benefit it would be a less-awkward thing to perhaps standardize later in the TC39 than "DOMException" which is really weird legacy name that confers little meaning (especially in this context).
This doesn't make much sense on Promise since it's specifically about ignoring "AbortError" DOMExceptions.
Here're my thoughts. Can this be a userland function instead? I can imagine people asking for other related functions, like say DOMException.ignoreNotFound meaning "only bubble up errors if they are not not-found errors". In any case, the function is probably much easier to write in JavaScript than C++.
@domenic @MattiasBuelens how has the thinking evolved here now that AbortSignal is essentially an "ExceptionSignal"?
You're right, it's no longer sufficient to only ignore AbortError DOMExceptions. Following the suggestion in https://github.com/whatwg/dom/pull/1027#pullrequestreview-782289936, we would need something along the lines of:
AbortSignal.prototype.ignoreAborts = function (promise) {
promise = Promise.resolve(promise);
return promise.catch(e => {
if (this.aborted && e === this.reason) {
// swallow exception
} else {
// rethrow
throw e;
}
});
};
which authors could use as:
async function butterBiscuits({ signal }) {
try {
const result = await signal.ignoreAborts(fetch('https://butternet/the-good-stuff?lots', { signal }));
if (result) {
this.biscuits.butter = result;
}
} catch (err) {
alerts.push('a TERRIBLE thing happened');
throw err;
}
}
It's kind of weird that you have to use signal twice, though. 😕
It's kind of weird that you have to use
signaltwice, though. 😕
I think in most cases you would only bother with it at the same level you create the AbortSignal, i.e. if you're just using a delegated signal you didn't create you'd usually just want to delegate error handling to the caller (as they raised the error).
@MattiasBuelens thanks, though it seems such an approach would still place special value on aborting. Also, e === this.reason would only work if there was no cloning involved.
Perhaps @yuki3's https://github.com/whatwg/webidl/issues/933#issuecomment-718896274 is more applicable now, especially if we also introduce AbortSignal.timeout() (which would result in a "TimeoutError" DOMException). In essence it seems you want the ability to filter out certain exceptions. The tricky aspect to something like that would be bridging JS exceptions and DOMException somehow. Or perhaps each can have its own API and they can work together through chaining.
I still think DOMException.ignoreAborts(), which only ignores "AbortError" DOMExceptions, is a good idea. As I've stated over in https://github.com/whatwg/dom/issues/1033#issuecomment-974869224, I don't think the fact that you can change the reason, changes how consumers should treat the result. Most cases don't want a general "ignore any abort reason"; they want "ignore "AbortError" DOMException".
In particular, given the motivating case for branching out, i.e. "TimeoutError" DOMException, you generally don't want to ignore timeouts for a fetch. You want to tell the user: "the operation timed out". Whereas you do want to ignore aborts: the user is probably the one that caused an abort.
you generally don't want to ignore timeouts for a fetch
That's consistent with our experience. Timeout is a result.
We do actually have checks like this in other cases, including one spot where there are three DEX types that are expected and not considered exceptional:
let shouldIgnore =
check.isAbortError(err) ||
check.isNotAllowedError(err) ||
check.isNotSupportedError(err);
But this example only makes sense in its very local context. The way AbortError models a type of completion instead of an abrupt result seems special / a general semantic related to lower level concerns than the others.
It's like break forced to wear a humiliating throw costume that makes every catch bouncer stop it at the door for an ID check. Or something? This justifies privileged API vis a vis other DOMException types.
Hmm, Note that this was quite the fuss in the dotnet world a bit back https://github.com/dotnet/runtime/issues/21965