Using promises to await triggered events

Question

In the following code, thing is an external object that I don't control; I can't change how the event system of thing works. When fn is called, we return a promise whose executor listens to an event and then begins awaiting a series of functions which eventually result in that event being triggered:

function fn() {

  return new Promise(async function(resolve, reject) {

      // This handler must be attached before `c` is called
      thing.once('myEvent', function(e) {
        resolve(e.data); // done
      });

      // The order of these functions calls is important,
      // and they may produce errors that need to be handled.
      await a();
      await b();
      await c(); // this causes myEvent

  });

}

This works fine, but I've been told that this is a promise anti-pattern and that I should make fn an async function. How would I do this? If I made fn an async function, then how could I resolve e.data from within the event handler?

Edit:

I've accepted Bergi's answer because it is helpful in explaining the anti-pattern and how it applies to this scenario. Having said that, I think the code above is more readable and shows explicitly what's happening so I'm going to keep it as is. This isn't the noob denouncing the best practices, it's just that for my use-case, following the rules makes things more complicated than they need to be. Of course, this leaves me open to certain problems, but I'll just have to live with that until I find a better way to do this.

Answer 1

Don't do any awaiting inside the Promise constructor - you only should do the promisification of the asynchronous callback in there:

async function fn() {
  await a();
  await b();
  await c(); // this causes myEvent
  return new Promise(function(resolve, reject) {
    thing.once('myEvent', function(e) {
      resolve(e.data); // done
    });
  });
}

The thing that starts the process which eventually causes the event to be emitted is usually called inside the Promise executor callback as well (to catch synchronous exceptions), but usually it doesn't return a promise like your c function does.

Maybe this expresses the intent better:

async function fn() {
  await a();
  await b();
  const {data} = await new Promise(resolve => {
    thing.once('myEvent', resolve);
    thing.c(); // this causes myEvent
  });
  return data;
}

Of course this assumes that you only need to start listening to the event when you've called the other ones. If you expect the event to fire before that, you essentially have a race with parallel execution - I'd recommend to use Promise.all in that case:

async function fn() {
  await a();
  await b();
  const [{data}, cResult] = await Promise.all([
    new Promise(resolve => thing.once('myEvent', resolve)),
    c()
  ]);
  return data;
}

If you have node v11.13.0 or higher you can use the events.once method so that you don't have to build the promise yourself - and it also handles error events correctly:

import { once } from 'events';

async function fn () {
  await a()
  await b()
  await c()
  await once(thing, 'myEvent')
}