JavaScript Promises confusion anti-pattern?

Question

I spent whole day trying to figure out weather I use Promises wrongly.

Is that anti-pattern ?

export const myExample = (payload) => {
  return new Promise((resolve, reject) => {})
}

Can I use async in promise like that ?

export const myExample = (payload) => {
  return new Promise(async (resolve, reject) => {})
}

Also is that wrong as well ? Assuming adding async makes it a promise by default,

export const myExample = async (payload) => {
  return new Promise((resolve, reject) => {})
}

also if that's the case, should I just return from function which will be same as resolve, and if I throw Error will be reject, so it would look like that ?

export const myExample = async (payload) => {
  if(payload) return true
  else throw new Error('Promise rejection?')
}

So is first and last the same ?

`new Promise(async (resolve, reject)` always is a "code smell" ... — Jaromanda X, Aug 14 '20 at 09:02
you should never be using async await when defining promises, those should be used when executing promises only, it's unclear what you wanna do and what your problem is — EugenSunic, Aug 14 '20 at 09:04
It's not a problem as such, I am making my own promises, for my app. but in those promises I want to use database calls, or fs async calls, e.g. await `fs.writeFile()` so you are saying I should never have `await` inside Promise ? so just use fs.writeFile().then().catch() ? — uneasy, Aug 14 '20 at 09:09
the second and third can be written as `async (payload) => {}`, not sure if there might be some differences in microtask world, ... async await uses try catch finally, also fs.promises ;) — Estradiaz, Aug 14 '20 at 09:11
[Never pass an `async function` as the executor to `new Promise`](https://stackoverflow.com/q/43036229/1048572), the others are mostly fine. (You're probably not really constructing a `new Promise` that never resolves, you actually do something inside there, right?) — Bergi, Aug 14 '20 at 09:57

score 3 · Accepted Answer · answered Aug 14 '20 at 09:12

export const myExample = (payload) => {
  return new Promise((resolve, reject) => {})
}

should only be used to convert code that is not promise based but returns the result asynchronously into a Promise.

export const myExample = (payload) => {
  return new Promise(async (resolve, reject) => {})
}

Is an anty pattern async already makes a function to return a Promise, and you break the promise chaining here.

export const myExample = async (payload) => {
  return new Promise((resolve, reject) => {})
}

Same as the first one new Promise should only be used to convert code that is not promise based but returns the result asynchronously into a Promise. If async can be committed depends on the other code within that function. But it would be better that if you need to use new Promise((resolve, reject) => {}) that the enclosing function only contains and returns that new Promise like in your first example.

also if that's the case, should I just return from function which will be same as resolve, and if I throw Error will be reject, so it would look like that ?

yes

Thanks, I think I get it now. Well I was using it wrong. So as my conclusion, in my case most of the time, I just want to wrap other await calls in my own promise. So the best for me would be just `export const myExample = async (payload) =>{}` and doing mu async calls, then return whatever if resolve and throw error if doesn't resolve ? — uneasy, Aug 14 '20 at 09:18
`So the best for me would be just export const myExample = async (payload) =>{} and doing mu async calls, then return whatever if resolve and throw error if doesn't resolve ` yes , if you mean something like that: `const myExample = async (payload) =>{ let res = await someTask(); if( res == "unexpected value") { throw new Error("something went wrong") } return res; }`, and if you call `myExample` it will return a Promise that is either resolved to `res` or be rejected with that error. — t.niese, Aug 14 '20 at 09:26

score 2 · Answer 2 · answered Aug 14 '20 at 09:22

It's a nice question, I was facing that kind of confusions as well and wanted to know where and what kind of structure to use. Came up with this:

async/await - I use it at a high level where mostly I write my handling part

async function asyncExample() {
  try {
    const sampleData = await otherFunction();
    // You code here
  } catch (err) {
    // Your error handling here
  }
}

It's always a good idea to use try/catch in async/await

Using new Promise(resolve, reject) concept. I mostly use it when I have to wrap a function that only supports callbacks.

function promiseExample() {
  return new Promise((resolve, reject) => {
    // your code to resolve()
    // otherwise to reject()
  });
}

But there is a nice module promisify which sometimes is a better solution then wrapping each function

score 0 · Answer 3 · answered Aug 14 '20 at 09:12

It's not an anti pattern, you need to do some logic with your payload argument inside the promise using the resolve function, for instance resolve(payload+'concatenate') , also you should implement the reject function based on the payload argument presumably

 export const myExample = (payload) => {
      return new Promise((resolve, reject) => {})
    }

You should not be using async await once defining a promise, those are to be used upon calling/executing promises only.

export const myExample = (payload) => {
  return new Promise(async (resolve, reject) => {})
}

The same goes for this code snippet

export const myExample = async (payload) => {
  return new Promise((resolve, reject) => {})
}

you should use await within your function since you are using the async keyword, also there is no promises defined in the snippet so make sure you define them before using async await, hence you'll be able to return some value or throw an error

export const myExample = async (payload) => {
  if(payload) return true
  else throw new Error('Promise rejection?')
}

score -1 · Answer 4 · answered Aug 14 '20 at 09:11

This is considered an 'anti-pattern' because there is a passability that errors can go uncaught. If an execution in the async function throws the error will be lost and won't cause the newly-constructed Promise to reject.

You can get around this problem by making sure you use try/catch although this necessary requirement can make your code more brittle.

How can I use this pattern correctly?

function promise() {
    return new Promise(async (resolve, reject) => {
        try {
            await iWillThrow() // throw err
        } catch (error) {
            reject(error) // need to reject here, error will not 'bubble'
        }
    });
}

How can this pattern catch me out?

function promise() {
    return new Promise(async (resolve, reject) => {
            await iWillThrow() // throw err, unhandled error ⚠
    });
}

I found the eslint explanation here usefull.

JavaScript Promises confusion anti-pattern?

4 Answers4