6 ms·

Awaiting a promise later on can lead to unhandled promise rejections, which will make Node.js crash in the future (currently prints a warning). Instead, Promis

by fathyb 5y ago

Awaiting a promise later on can lead to unhandled promise rejections, which will make Node.js crash in the future (currently prints a warning). Instead, Promise.all() and .then() should be used: let [result1, result2] = await Promise.all([ fetch('/request1').then(doSomeCalculation), fetch('/request2') ])

WolfOliver 5y ago
can you not use try catch blocks for this?
- nosianu 5y ago
  Unless you stop executing code inside the try{} block by using "await" the try/catch is over when the error is thrown asynchronously later on. Leading to an asynchronous promise rejection. Yes your code is located inside a try/catch - but the execution has gone well past it unless you "await" the promise inside the try{}. That is a serious little trick question/problem for people not deeply familiar with async/await and promises in Javascript, so most newbie and middlish programmers. Either you only use stock-standard code constructs, so you refrain from creating promises and not immediately await-ing them, but passing them around*, or you really need to know and deeply understand promises and async/await. Relying on async/await but also additionally passing some un-await-ed promises around poses danger and needs a "senior" level understanding. The try/catch tricks people to think any promise created within is caught - but the try/catch exists only at one specific point in time. You have an additional dimension, your lexical code structure shows an incomplete picture. Whenever you create a promise, if you don't immediately await it, you must make sure that before there is another "await" you must attach a catch handler, or you may get an unhandled promise rejection at some point. Which usually nobody tests for, because on the happy path the program works just fine. Either attach one with .catch(), of if you use try/catch you must "await" the promise inside the try block - it must not resolve before code execution progresses past that block.
  - munchbunny 5y ago
    Wait, isn’t awaiting inside the try{} exactly how you’re supposed to handle it in the first place? Assuming you’re using await and not .catch(). And if you await without the try/catch you just end up with an unhandled exception at the await, no? I’ve yet to see any junior devs make the mistake of try-ing an unawaited promise. They’ve all seemed to appreciate the “async” aspect of the semantics up front.
    - nosianu 5y ago
      Remember that the context of this discussion is achieving parallelism by not waiting - instead, you create promises in parallel, pass them on, and attach handlers at some later point, e.g. after collecting them in a Promise.all(). As I said, as long as you do standard stuff, which for try/catch in an async function and an "await" of all promises is the standard path. Sometimes people create a promise without awaiting inside a try/catch, thinking they got errors handled, because they don't want to wait but start another promise-producing function right away. But that other function needs its own catch handler so they write another one below the first one and don't "await" to get both asynchronous promise-returning functions to run simultaneously without the second one having to wait for the first one, because what they do is independent (so far the thoughts are correct and good). Since they use try/catch instead of the .then() and .catch() handler and they don't fully understand all the implications they run into this problem. It happened to me when I learned promises, after already having programmed with them for well over a year and feeling comfortable, and it just recently happened to some of my people (it no longer happens to me, it's the next generations turn...). Other issues that make this exact problem insidious are: - This problem can only be spotted when the promise is rejected. If your tests don't include that you will only see it when someone runs into this issue by chance, much later, maybe in production. - Often it may not get fixed even after there is a rejected promise showing the problem. What time-limited stressed middle-level developers will do is solve the rejected promise but not the way it isn't caught. The code construct that lead to "uncaught promise rejection" will remain unfixed though, because as soon as the promise no longer rejects the code appears to work fine.
      - 5y ago
- fathyb 5y ago
  Using a try/catch block won't attach an error handler at the right time (before the next event loop tick): // promise1 will resolve after one second let promise1 = new Promise(resolve => setTimeout(resolve, 1000)) // promise2 will wait for the next even loop tick to call its error handler let promise2 = Promise.reject('something bad') // At this point, you should await promise2, or set an error handler using .catch // But you can do some sync stuff if you want to: someExpensiveCPUStuff() // But if you do anything async, it'll print a warning in Node.js let result1 = await promise1 If you ever need to store a promise and its error over time (ie. an async cache), you can wrap the value, and unwrap it when needed: let wrapped = promise .then(result => ({ type: 'resolved', result })) .catch(error => ({ type: 'rejected', error })) await doSomeAsyncStuff() let result = await wrapped.then(data => { if(data.type === 'resolved') { return data.result } else { throw data.error } })
  - mfbx9da4 5y ago
    > will wait for the next even loop tick to call its error handler are you sure about that? Promise.reject is synchronous and won't wait for the next tick it just returns a rejected promise try/catch can be used fine async function f() { throw new Error("asdf"); } let result1 = f(); let result2 = f(); try { await result1; } catch (e) { console.log("caught the error"); } try { await result2; } catch (e) { console.log("caught the error"); }
    - fathyb 5y ago
      The error handler won't be called until the next tick: Promise .reject('error') .catch(error => console.log('Error handler called:', error)) console.log('After reject') This will print: After reject Error handler called: error
    - lhorie 5y ago
      I think the point is that try/catch in async code is tied to the presence of await. This will run happily without falling into the catch clause: const err = async () => { throw new Error('async error') } try { const rejected = Promise.reject(); const errored = err(); console.log('ok!'); // in real code, this might have returned a deferred to be handled elsewhere instead } catch (e) { console.log('not ok!'); // never gets here }
- lhorie 5y ago
  Depends on what you're doing w/ the promise variable. If at some point you start to return promises without awaiting them, then the try/catch might not catch unhandled rejections. This is a source of subtle non-happy-path bugs for code that deals w/ custom deferred objects, e.g. some flavors of promisified ChildProcess'es (promise objects with `stdout`/`stderr` properties). Naked promises are tricky enough that I've seen experienced developers use hungarian notation to denote that a value is a promise, in order to indicate the presence of non-trivial error propagation semantics in said snippet of code.
deschutes 5y ago
How can the runtime conclude that the rejection is unhandled when there are outstanding references to the Promise? In other words, as long as the promise might later be awaited or otherwise handled I don't see why the runtime should care. (That's the mental model I carry from other languages.)
- Footkerchief 5y ago
  Normal memory references aren't enough to prove that the promise's rejection is handled. It needs a reference in the form of one or more registered handlers -- which can be achieved by registering them immediately via the Promise.all in the above example.
  - slaymaker1907 5y ago
    Correct me if I'm wrong, but I think you can avoid this by doing: thingToUseLater.catch(() => {}) And just ignoring the result of .catch(). Promises are reusable so thingToUseLater can be waited on later when you want to block for it. Thanks for thoughts on this. I use promises a lot but this is an edge case I hadn't thought about. They are really pretty handy for build scripts since they make your dependencies explicit.
    - beardedetim 5y ago
      Yup. If you call catch, you've handled the promise. You would need to throw from within the callback in order for the promise itself to reject after that.

by fathyb 5y ago

WolfOliver 5y ago

can you not use try catch blocks for this?

nosianu 5y ago
Unless you stop executing code inside the try{} block by using "await" the try/catch is over when the error is thrown asynchronously later on. Leading to an asynchronous promise rejection. Yes your code is located inside a try/catch - but the execution has gone well past it unless you "await" the promise inside the try{}. That is a serious little trick question/problem for people not deeply familiar with async/await and promises in Javascript, so most newbie and middlish programmers. Either you only use stock-standard code constructs, so you refrain from creating promises and not immediately await-ing them, but passing them around*, or you really need to know and deeply understand promises and async/await. Relying on async/await but also additionally passing some un-await-ed promises around poses danger and needs a "senior" level understanding. The try/catch tricks people to think any promise created within is caught - but the try/catch exists only at one specific point in time. You have an additional dimension, your lexical code structure shows an incomplete picture. Whenever you create a promise, if you don't immediately await it, you must make sure that before there is another "await" you must attach a catch handler, or you may get an unhandled promise rejection at some point. Which usually nobody tests for, because on the happy path the program works just fine. Either attach one with .catch(), of if you use try/catch you must "await" the promise inside the try block - it must not resolve before code execution progresses past that block.
- munchbunny 5y ago
  Wait, isn’t awaiting inside the try{} exactly how you’re supposed to handle it in the first place? Assuming you’re using await and not .catch(). And if you await without the try/catch you just end up with an unhandled exception at the await, no? I’ve yet to see any junior devs make the mistake of try-ing an unawaited promise. They’ve all seemed to appreciate the “async” aspect of the semantics up front.
  - nosianu 5y ago
    Remember that the context of this discussion is achieving parallelism by not waiting - instead, you create promises in parallel, pass them on, and attach handlers at some later point, e.g. after collecting them in a Promise.all(). As I said, as long as you do standard stuff, which for try/catch in an async function and an "await" of all promises is the standard path. Sometimes people create a promise without awaiting inside a try/catch, thinking they got errors handled, because they don't want to wait but start another promise-producing function right away. But that other function needs its own catch handler so they write another one below the first one and don't "await" to get both asynchronous promise-returning functions to run simultaneously without the second one having to wait for the first one, because what they do is independent (so far the thoughts are correct and good). Since they use try/catch instead of the .then() and .catch() handler and they don't fully understand all the implications they run into this problem. It happened to me when I learned promises, after already having programmed with them for well over a year and feeling comfortable, and it just recently happened to some of my people (it no longer happens to me, it's the next generations turn...). Other issues that make this exact problem insidious are: - This problem can only be spotted when the promise is rejected. If your tests don't include that you will only see it when someone runs into this issue by chance, much later, maybe in production. - Often it may not get fixed even after there is a rejected promise showing the problem. What time-limited stressed middle-level developers will do is solve the rejected promise but not the way it isn't caught. The code construct that lead to "uncaught promise rejection" will remain unfixed though, because as soon as the promise no longer rejects the code appears to work fine.
    - 5y ago
fathyb 5y ago
Using a try/catch block won't attach an error handler at the right time (before the next event loop tick): // promise1 will resolve after one second let promise1 = new Promise(resolve => setTimeout(resolve, 1000)) // promise2 will wait for the next even loop tick to call its error handler let promise2 = Promise.reject('something bad') // At this point, you should await promise2, or set an error handler using .catch // But you can do some sync stuff if you want to: someExpensiveCPUStuff() // But if you do anything async, it'll print a warning in Node.js let result1 = await promise1 If you ever need to store a promise and its error over time (ie. an async cache), you can wrap the value, and unwrap it when needed: let wrapped = promise .then(result => ({ type: 'resolved', result })) .catch(error => ({ type: 'rejected', error })) await doSomeAsyncStuff() let result = await wrapped.then(data => { if(data.type === 'resolved') { return data.result } else { throw data.error } })
- mfbx9da4 5y ago
  > will wait for the next even loop tick to call its error handler are you sure about that? Promise.reject is synchronous and won't wait for the next tick it just returns a rejected promise try/catch can be used fine async function f() { throw new Error("asdf"); } let result1 = f(); let result2 = f(); try { await result1; } catch (e) { console.log("caught the error"); } try { await result2; } catch (e) { console.log("caught the error"); }
  - fathyb 5y ago
    The error handler won't be called until the next tick: Promise .reject('error') .catch(error => console.log('Error handler called:', error)) console.log('After reject') This will print: After reject Error handler called: error
  - lhorie 5y ago
    I think the point is that try/catch in async code is tied to the presence of await. This will run happily without falling into the catch clause: const err = async () => { throw new Error('async error') } try { const rejected = Promise.reject(); const errored = err(); console.log('ok!'); // in real code, this might have returned a deferred to be handled elsewhere instead } catch (e) { console.log('not ok!'); // never gets here }
lhorie 5y ago
Depends on what you're doing w/ the promise variable. If at some point you start to return promises without awaiting them, then the try/catch might not catch unhandled rejections. This is a source of subtle non-happy-path bugs for code that deals w/ custom deferred objects, e.g. some flavors of promisified ChildProcess'es (promise objects with `stdout`/`stderr` properties). Naked promises are tricky enough that I've seen experienced developers use hungarian notation to denote that a value is a promise, in order to indicate the presence of non-trivial error propagation semantics in said snippet of code.

deschutes 5y ago

How can the runtime conclude that the rejection is unhandled when there are outstanding references to the Promise? In other words, as long as the promise might later be awaited or otherwise handled I don't see why the runtime should care. (That's the mental model I carry from other languages.)

Footkerchief 5y ago
Normal memory references aren't enough to prove that the promise's rejection is handled. It needs a reference in the form of one or more registered handlers -- which can be achieved by registering them immediately via the Promise.all in the above example.
- slaymaker1907 5y ago
  Correct me if I'm wrong, but I think you can avoid this by doing: thingToUseLater.catch(() => {}) And just ignoring the result of .catch(). Promises are reusable so thingToUseLater can be waited on later when you want to block for it. Thanks for thoughts on this. I use promises a lot but this is an edge case I hadn't thought about. They are really pretty handy for build scripts since they make your dependencies explicit.
  - beardedetim 5y ago
    Yup. If you call catch, you've handled the promise. You would need to throw from within the callback in order for the promise itself to reject after that.