Why is code inside then block being evaluated before the code inside catch block completes

Question

Here is the code:

function f1(id) {
    return new Promise((resolve, reject) => {
        if (id === 123132) {
            resolve([{id:1,name:"John"}, {id:10, name:"Chris"}])

        } else {
            reject(new Error("f1 fails - can't find info for id"))
        }


    })

}

function f2(gender) {
    return new Promise((resolve, reject) => {
        if (gender === "FEMALE") {
            resolve([{id:6, name:"Stacy"}, {id:1, name:"John"}, {id:13, name:"Veronica"}])
        } else {
            reject(new Error("f2 Fails"))
        }
    })

}



 function Test(User, N){
    return new Promise((resolve, reject)=>{
        f1(User.id).catch(err=>{

            console.log(err.message)
            //this api returns an error, so call the other one
            f2(User.gender).catch(err=>{
                console.log(err.message)
                reject(new Error("Both api calls have failed"))
            }).then(res=>{
                if (res.length<N){
                  reject(new Error("Not Enough Data..."))
                } else {
                    console.log("We are here..")
                    console.log(res)
                    resolve(res.slice(0,N).map(item=>item.name))
                }
            })
        })
        .then(res1=>{
            console.log("We should not be here but we are...")
            console.log(res1)
          if (res1.length<N){
              f2(User.gender).catch(err=>{
                    console.log(err.message)
                //   reject(new Error("Not Enough Data"))
                  throw new Error("Not Enough Data")
              }).then(res2=>{
                  res3 = filterDups2(res1, res2)
                  res3.length>=N ? resolve(res3.slice(0,N).map(item=>item.name)) :
                  reject(new Error("Not Enough Data"))
              })
          } else {
              console.log("Why are we not here...")
              resolve(res1.slice(0,N).map(item=>item.name))
          }
        })
    })
}


function filterDups2(list1, list2){
    jointRes = [...list1, ...list2]
    ans = Array.from(new Set(jointRes.map(item=>JSON.stringify(item)))).map(item=>JSON.parse(item))  
    return ans
  }

let user = { id: 123, gender: "FEMALE" }
let result = Test(user, 2)


result.then(res=> console.log(res)).catch(err=> {
    console.log("Inside result...")
    console.log(err.message)
})

Here is the output on the console:

f1 fails - can't find info for id
We should not be here but we are...
undefined
We are here..
[
  { id: 6, name: 'Stacy' },
  { id: 1, name: 'John' },
  { id: 13, name: 'Veronica' }
]

[ 'Stacy', 'John' ]

(node:2968) UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'length' of undefined
    at /Users/daanishraj/Desktop/Front-End-Work/WDB-Feb-2021/JavaScript/Practice/interviews/zalando/forStackOverflow.js:50:20
(Use `node --trace-warnings ...` to show where the warning was created)
(node:2968) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)
(node:2968) [DEP0018] 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.

I don't understand why we have the following two things on the console

We should not be here but we are...
undefined

showing up before these outputs:

We are here..
[
  { id: 6, name: 'Stacy' },
  { id: 1, name: 'John' },
  { id: 13, name: 'Veronica' }
]
[ 'Stacy', 'John' ]

I understand that because the outermost catch block i.e. f1(U.id).catch() is resolved, this would make us step into the outermost then block i.e.f1(U.id).then(). But the sequence of these outputs suggest that we are stepping into the then block before the catch block is resolved.

1. Why is this the case?
2. Since we are indeed stepping into the then block, once res1 is deemed undefined, why doesn't the control move to the bottom of the then block where we would output Why are we not here...?

In general, if you could throw some light on the sequence of execution here, that would be great!

Answer 1

Why is this the case?

Because you haven't instructed the promise chain to wait for an asynchronous result from the catch handler. You'd need to return a promise from there for that.
Stepping into the then handler doesn't happen "before the catch block is resolved", the catch handler did already execute and did return undefined - that's what the promise chain continues with.

Why doesn't the control move to the bottom of the then block where we would output "Why are we not here..."?

Because right after logging undefined, you access res1.length, which causes the exception TypeError: Cannot read property 'length' of undefined. This rejects the promise, which is not handled anywhere, leading to the warning.

---

Now onto the real question: how to do this properly? You should avoid the Promise constructor antipattern! The .then() and .catch() calls already return a promise for the result of the handler, which you can and should use - no need to manually resolve or reject a promise, and no unhandled promise rejections because some inner promise is rejected but you fail to propagate this failure outside (notice the "Inside result..." handler should have been called when res1.length errored). Also I recommend to use .then(…, …) instead of .then(…).catch(…) (or .catch(…).then(…)) for conditional success/error handling.

function Test(User, N) {
    return f1(User.id).then(res1 => {
//  ^^^^^^
        if (res1.length < N) {
            return f2(User.gender).then(res2 => {
//          ^^^^^^
                const res3 = filterDups2(res1, res2)
                if (res3.length >= N)
                    return res3.slice(0,N).map(item => item.name)
//                  ^^^^^^
                else
                    throw new Error("Not Enough Data")
//                  ^^^^^
            }, err => {
                console.log(err.message)
                throw new Error("Not Enough Data")
//              ^^^^^
            })
        } else {
            return res1.slice(0,N).map(item => item.name)
//          ^^^^^^
        }
    }, err => {
        console.log(err.message)
        return f2(User.gender).then(res => {
//      ^^^^^^
            if (res.length < N) {
                throw new Error("Not Enough Data...")
//              ^^^^^
            } else {
                return res.slice(0,N).map(item => item.name);
//              ^^^^^^
            }
        }, err => {
            console.log(err.message)
            throw new Error("Both api calls have failed")
//          ^^^^^
        })
    })
}

Avoiding some code duplication:

function Test(User, N) {
    return f1(User.id).then(res1 => {
        if (res1.length >= N) return res1;
        return f2(User.gender).then(res2 => {
            return filterDups2(res1, res2)
        }, err => {
            console.log(err.message)
            return res1;
        })
    }, err => {
        console.log(err.message)
        return f2(User.gender).catch(err => {
            console.log(err.message)
            throw new Error("Both api calls have failed")
        })
    }).then(res => {
        if (res.length >= N)
            return res.slice(0,N).map(item => item.name)
        else
            throw new Error("Not Enough Data")
    })
}