Ky: `Promise.race()` breaks debugging

[Splact]

I generally develop frontend having "Pause on exceptions" functionality of chrome dev tools enabled. Here Ky doesn't work so good and forced me to disable that feature.
What happens is that determining if the fetch request goes well or it goes in timeout, Ky puts the actual request in race against a promise that on timeout (delay) throws an error. It's a nice looking piece of code and works fine, but when the fetch resolves correctly (in time) the other promise continues on its way and launches an uncontrolled exception that is caught by "Pause on exceptions".

A solution can be removing the throw statement form the timeout promise and chaining the result of the promises race with another function that checks which promise won (fetch or timeout), if the latter then throw the error.

[sholladay]

Promises are supposed to be immutable. Once they are resolved or rejected, they stay that way. That's fundamentally why the Promise.race() approach works. Seems to me like this is a bug in Chrome DevTools, or at least a debugging feature that shouldn't be on by default.

To make sure I'm 100% understanding correctly, though, a reproducible snippet might be useful.

Speaking for myself, I'm open to the idea of short-circuiting the timeout throw if the request already succeeded. It's pretty easy to do and if it aids debugging, seems worth it. Network calls are hard enough to debug as it is, just generally, unrelated to Ky.

[szmarczak]

I've reproduced the issue with this piece of code:

const delay = ms => new Promise(resolve => setTimeout(resolve), ms);

const fn = async () => {
    await delay(100);
    throw new Error('Hello there.');
};

Promise.race([fn(), 'd']);

Indeed, it pauses on these exceptions. Seems like a bug in Chrome.

[sindresorhus]

Someone should open a Chromium issue → https://chromiumbugs.appspot.com/?token=wrdLx4uRUNV9omU7L9nyKToxNTM5MjE2MDAw&role=&continue=https%3A//bugs.chromium.org/p/chromium/issues/entry.do

[arty-name]

I am quite sure that this is not a Chrome issue, as it behaves the same way in Firefox, and it’s also logical behavior. When a asynchronous operation is started it shouldn’t be cancelled just because the resulting promise was given as a parameter to Promise.race.

Code to reproduce in Firefox:

const script = document.createElement('script')
script.src = 'https://cdn.jsdelivr.net/npm/[email protected]/umd.js'
document.head.appendChild(script)
// after a delay...
ky.default('/').then(console.log)

The proper fix would be to set some flag to true on successful response and to check the value of this flag before proceeding with TimeoutError.

[sholladay]

It's a strange behavior for the browser DevTools to have because the Promise is not rejected. Promise.race() effectively acts like a catch () {} in this case. I can imagine some scenarios where it might be nice to inspect Promise races and see all errors in case they were accidentally ignored, but by default the DevTools should probably not break in this case because the Promise is successfully resolved.

[arty-name]

You are right and actually browser developers agree with you: by default this feature is not activated. And indeed we developers use it sometimes to detect all errors, including those that were accidentally or willingly ignored.

However it is nice not to have such false positives, and Szymon has already suggested a nice solution in my PR #122.

[arty-name]

Thanks everyone!