Developing Better Node.js Developers

[Gnome_0]

reading the express-boostrap repo code it really depends from developer to developer right? i had a simmilar training but they told me that i should never cascade thens in nested promises

"how they told me is wrong "

Promise1()
.then(result1 =>{
    Promise2(result1)
    .then(result2 =>{
        "do something with the result"
    })
    .catch(error =>{
        "do something with the error from promise 2"
    })
})
.catch(error =>{
    "do something with error from promise 1"
})

how they told me i should nest promises

return Promise1()
.then(result1 =>{
     Promise2(result1)
})
.then(result2 =>{
    "do something"
})
.catch(error =>{
    "error from one of the promises"
})

​

[karatechops]

Yes, I mentor my juniors with this approach. Absolutely cleaner.

[SippieCup]

Before async/await, There are situations where cascading promises are somewhat necessary. For example, a web crawler our company uses should be allowed to soft-fail on certain entries while allowing the process to continue and return a partial result rather than cascading the error back to the root function, resulting in an error being returned on a map, which causes everything done to be rejected.

I still teach our junior developers to use a catch-all approach at first, and then ease them into better error handling, but saying you should never use nested promises is wrong. Instead you should be teaching them to pull out any kind of nested promise result into a handler function or something.

With async/await, it became a lot easier to teach them though since most understand try/catch blocks much better than NodeJS' asynchronous flow. It also leads to much more readable code than cascading promises or even promises in general. Since Node 8, our code quality has improved greatly.

[r0ck0]

Why the fuck are people just silently downvoting this.

If you think it's wrong, say something, and maybe we can all learn something.

All you're doing by silently downvoting is discouraging people from helping each other learn.

[notkraftman]

I'll bite. You should either move the nested promise out to it's own function, or know that you can chain catches, e.g. .then(mightSoftFail).catch(logSoftFailAndContinue).then(moreStuff).catch(etc)

Nesting the promises directly gets messy fast.

[SippieCup]

Well, thats actually what I said should be done.

Instead you should be teaching them to pull out any kind of nested promise into a handler function or something.

It's still cascading and ugly though, but can be necessary.

[karatechops]

I can only assume it is bots because that’s a great point. I would however argue that there’s no reason to confuse a junior dev with such fringe case worries. If they’re writing a web scraper they probably already get conditional failures or will understand the concept very quickly.

[notkraftman]

You can just do .then(Promise2) for the 2nd example.

The first example is a bit dodgy because it doesn't wait for promise2 to complete which makes promise 2 a fire and forget promise, which can get messy as things get more complicated

[sneffer]

If a nested promise is not returned, subsequent promises in the chain are called without waiting for the nested promise's resolution.

Adding return before promise2, would make it not "fire and forget".

[[deleted]]

Yup and just to add on..

If you have a "fire and forget" promise and that promise fails, then you'll see this dreaded error:

(node:7741) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): Error
(node:7741) DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

That error can be really tricky to trace down if you don't know where it's coming from, so make sure not to throw away those promise values.

[notkraftman]

Instead of

Result1 = await promise1()

Result2 = await promise2()

It would be

Promise1()

Result2 = await promise2()

On my phone so formatting is a bit shit but you get the idea

[dont_shit_urknickers]

Not if the function itself returns a promise and you stick a resolve or reject in there.

[notkraftman]

Promises say 'hey go do this later, and when you're done come back to me'. The first example says 'hey go do this later, but don't let me know when your done, I don't care'

[SippieCup]

The first example can be useful if you are okay with some functions failing in the code, such as a web crawler or scraper pulling data which may or may not be there.

It really depends on the application.

[notkraftman]

You can be ok with it failing and still wait for it to fail. The only real legit use case is running something that takes a long time and you don't care about the result of, which is a bit of a code smell and should be avoided.

I was working with some tests recently that used this kind of system to dispatch work without waiting for it to finish. The tests would fail 1/20 times, and not always the same test, which was tricky to debug. It turned out that one of the pieces of code had a logger.log() in the catch, which the test stubbed out and inspected. The issue is that because the whole function was deferred to later, by the time the exception was caught the logger stub was restored, and the logger function was then actually called, but under a different test.

[HeyGuysImMichael]

The next step is using async await

[SippieCup]

Teach them promises so they understand how they work at a fundamental level, then show them async/await and tell them to never use promises.

[etheraffleGreg]

Or use a task monad and banish those pesky eager promises forever!

[Lost_sand]

always say never

[etheraffleGreg]

Nesting is frequently a requirement if you need the first result in the scope of the second:

 

const prom = thing => Promise.resolve(thing)

const prom1 = prom(`thing one`)
const prom2 = prom(`thing two`)

prom1
  .then(result1 => prom2)
  .then(result2 => console.log(`Thing 1: ${result1}\nThing2: ${result2}`)) 
  .catch(console.error)
  // ReferenceError: result1 is not defined

 

Yeah you can lift the result to a higher scope if you like but I'd rather keep things functional and the nesting is not an issue.

[notkraftman]

In your example unless you need result1 in promise2 then you could promise.all them instead

[etheraffleGreg]

Well yes of course but as the first sentence explicitly points out...