chore (client): use async event emitter in Satellite client

[kevin-dp]

Problem

The Satellite client uses an EventEmitter to emit events to which the Satellite process is subscribed. However, the Satellite process registers asynchronous event listeners but the EventEmitter calls them synchronously. Hence, async listeners for an event may run concurrently and a new event may be emitted while async listeners of the previous event are still running.

An example in NodeJS:

const EventEmitter = require('node:events');
const emitter = new EventEmitter();
emitter.on('transaction', (tx) => {
  console.log('listener 1 start: ' + tx);
  return new Promise((resolve) => {
    setTimeout(() => {
      console.log('listener 1 end: ' + tx);
      resolve()
    }, 10)
  })
})

emitter.emit('transaction', 'tx1');
emitter.emit('transaction', 'tx2');
/*
   Prints:
     listener 1 start: tx1
     listener 1 start: tx2
     listener 1 end: tx1
     listener 1 end: tx2
 */

emitter.on('transaction', (tx) => {
  console.log('listener 2 start: ' + tx);
})

emitter.emit('transaction');

/*
   Prints:
     listener 1 start: tx3
     listener 2 start: tx3
     listener 1 end: tx3
 */

The first example above shows interleaving of async listeners on an event.
The second example shows interleaving of listeners for distinct events.

Solution

Implemented an AsyncEventEmitter that awaits all listeners before processing the next event. Modified the Satellite client to use the AsyncEventEmitter instead of the regular EventEmitter. In addition, the AsyncEventEmitter is fully typed, inspired by the typings of the typed-emitter package.

Note that, we start all listeners for an event in order without awaiting. Hence, async listeners registered for an event may run concurrently. However, we await all listeners before processing the next event.

With the new AsyncEventEmitter we get this behavior:

const emitter = new AsyncEventEmitter();
emitter.on('transaction', (tx) => {
  console.log('listener 1 start: ' + tx);
  return new Promise((resolve) => {
    setTimeout(() => {
      console.log('listener 1 end: ' + tx);
      resolve()
    }, 10)
  })
})

emitter.enqueueEmit('transaction', 'tx1');
emitter.enqueueEmit('transaction', 'tx2');
/*
   Prints:
     listener 1 start: tx1
     listener 1 end: tx1
     listener 1 start: tx2
     listener 1 end: tx2
 */

emitter.on('transaction', (tx) => {
  console.log('listener 2 start: ' + tx);
})

emitter.enqueueEmit('transaction');

/*
   Prints:
     listener 1 start: tx3
     listener 2 start: tx3
     listener 1 end: tx3
 */

The first example above shows that events are processed in order only after all listeners have finished.
The second example shows that listeners for a given event are started in order one after the other without awaiting.

Note: we renamed emit to enqueueEmit to avoid confusion with the classical EventEmitter's emit function which has different semantics as it does not await all listeners before processing the next event.

[linear]

VAX-937 Fix client to process events that are async functions

Look for this comment ```// FIXME: calling an async function in an event emitter```

[balegas]

Needs more discussion

[balegas]

This looks much better than the code above, but I'd prefer to reuse the defined the callbacks (TransactionCallback, etc) than declaring them again

[kevin-dp]

I'm not sure what you mean here. Do you mean to use TransactionCallback as the type for the ackCb parameter for the transaction event below?
TransactionCallback is not the same type as () => void.

[icehaunter]

LGTM, nice work. A couple of questions but nothing blocking -- address at your discretion. Only thing I would ask you to acknowledge explicitly is the error handling comment

[icehaunter]

Curious as to why this is extends here but a separate private instance on the real client? I'd generally argue for exposing typed on methods on the client itself and doing away with subscribeToSmth method pattern, however that might be out of scope here.

[kevin-dp]

It was already like that before so i left it how it was, only changed the EventEmitter to use AsyncEventEmitter. Your proposed refactoring would indeed make the code cleaner and more idiomatic but should probably be addressed in a separate PR as you suggest.

[balegas]

I can shed some light on why mock still extends event emitter. The client used to be like that, but I refactored it to not extend event emitter anymore. I might have overseen the mock.

[icehaunter]

Why is this method even here if all we're doing with it is testing it, not even using it in other tests?

[kevin-dp]

Because AsyncEventEmitter implements an event emitter interface which typically provides an eventNames function. So i implemented it in case we need it later.