3 ms·
Unless it changed how NodeJS handles this you shouldn't use Promise.all(). Because if more than one promise rejects then the second rejection will emit a unhand
by panzi 1y ago
Unless it changed how NodeJS handles this you shouldn't use Promise.all(). Because if more than one promise rejects then the second rejection will emit a unhandledRejection event and per default that crashes your server. Use Promise.allSettled() instead.
- vinnymac 1y agoPromise.all() itself doesn't inherently cause unhandledRejection events. Any rejected promise that is left unhandled will throw an unhandledRejection, allSettled just collects all rejections, as well as fulfillments for you. There are still legitimate use cases for Promise.all, as there are ones for Promise.allSettled, Promise.race, Promise.any, etc. They each serve a different need. Try it for yourself: > node > Promise.all([Promise.reject()]) > Promise.reject() > Promise.allSettled([Promise.reject()]) Promise.allSettled never results in an unhandledRejection, because it never rejects under any circumstance.
- mijkal 1y agoWhen using Promise.all(), it won't fail entirely if individual promises have their own .catch() handlers.
- andrewmcwatters 1y agoSubtle enough you’ll learn once to not do that again if you’re not looking for that behavior.
- kaoD 1y agoThis didn't feel right so I went and tested. process.on("uncaughException", (e) => { console.log("uncaughException", e); }); try { const r = await Promise.all([ Promise.reject(new Error('1')), new Promise((resolve, reject) => { setTimeout(() => reject(new Error('2'), 1000)); }), ]); console.log("r", r); } catch (e) { console.log("catch", e); } setTimeout(() => { console.log("setTimeout"); }, 2000); Produces: alvaro@DESKTOP ~/Projects/tests $ node -v v22.12.0 alvaro@DESKTOP ~/Projects/tests $ node index.js catch Error: 1 at file:///C:/Users/kaoD/Projects/tests/index.js:7:22 at ModuleJob.run (node:internal/modules/esm/module_job:271:25) at async onImport.tracePromise.__proto__ (node:internal/modules/esm/loader:547:26) at async asyncRunEntryPointWithESMLoader (node:internal/modules/run_main:116:5) setTimeout So, nope. The promises are just ignored.
- panzi 1y agoSo they did change it! Good. I definitely had a crash like that a long time ago, and you can find multiple articles describing that behavior. It was existing for quite a time, so I didn't think that is something they would fix so I didn't keep track of it.
- kaoD 1y agoPerhaps you're confusing it with something else? I tried down to Node 8 (2017) and the behavior is still the same. Maybe a bug in userspace promises like Bluebird? Or an older Node where promises were still experimental? I love a good mystery!
- panzi 1y agoWeird. Also just tried it with v8 and it doesn't behave like I remember and also not like certain descriptions that I can find online. I remember it because I was so extremely flabbergasted when I found out about this behavior and it made me go over all of my code replacing any Promise.all() with Promise.allSettled(). And it's not just me, this blog post talks about that behavior: https://chrysanthos.xyz/article/dont-ever-use-promise-all/ https://chrysanthos.xyz/article/dont-ever-use-promise-all/ Maybe my bug was something else back then and I found a source claiming that behavior, so I changed my code and as a side effect my bug happened to go away coincidentally?
- kaoD 1y agoIt could be this: https://stackoverflow.com/questions/67789309/why-do-i-get-an-unhandled-promise-rejection-with-await-promise-all https://stackoverflow.com/questions/67789309/why-do-i-get-an... If you did something like: const p1 = makeP1(); const p2 = makeP2(); return await Promise.all([p1, p2]); It's possible that the heuristic didn't trigger?
- cluckindan 1y agoTypo? ”uncaughException”