Javascript / Node.js ] Handle Error for promise with await

Question

Written below is an example of my code I usually do. What I did is..

1. Add try-catch for a promise function with async-await.
2. Do no add try-catch for a promise function without async-await.

What I want to know is my code is alright, and it is anti-pattern or not.

Thank you in advance.

const readStatusAll = data => {
    return new Promise( async (resolve, reject) => {
        try {
            const category = await CoreStatus.findAll()
            resolve(category)
        } catch(err) {
            reject(err)
        }
    })
}


// Promise without Await
const readStatusAll = data => {
    return new Promise( async (resolve, reject) => {
        CoreStatus.findAll()
            resolve(category)
        }
    })
}

Answer 1

What I want to know is my code is alright, and it is anti-pattern or not.

The first one will work properly, but is an anti-pattern.

The second one will not work properly.

Neither of the code blocks you show is the recommended way to do things because they are both needlessly wrapping an existing promise in another manually created promise. This is referred to as an anti-pattern. This first version will actually work properly, but it contains a bunch of useless code (thus making it an anti-pattern) and with a little more complexity in the function, it's very easy to make coding mistakes (which is why, in addition to the useless code it contains, its an anti-pattern).

const readStatusAll = data => {
    return new Promise( async (resolve, reject) => {
        try {
            const category = await CoreStatus.findAll()
            resolve(category)
        } catch(err) {
            reject(err)
        }
    })
}

It can instead be this:

const readStatusAll = data => {
    return CoreStatus.findAll();
}

The caller will receive the promise as the return value and can then use either .then() and .catch() or await and try/catch. Since you aren't doing anything with the error, other than propagating it, you don't need to catch the error locally - you can just let it propagate back to the caller.

---

Your second version is just not correct at all:

// Promise without Await
const readStatusAll = data => {
    return new Promise( async (resolve, reject) => {
        CoreStatus.findAll()
            resolve(category)
        }
    })
}

Because you're not paying any attention to any asynchronous return from CoreStatus.findAll() so you will resolve this manual wrapper promise long before the database call is actually done. In fact, this isn't even legal code as you have improper bracing.

Perhaps you meant to call resolve(category) in some callback or .then() handler associated with CoreStatus.findAll(). But, even if you did that, you still wouldn't be propagating errors back to the caller. This is not the way to do things.