rolftimmermans / event-iterator

Convert event emitters and event targets to ES async iterators
90 stars 10 forks source link

Support for AbortController #20

Open ad-m opened 3 years ago

ad-m commented 3 years ago

Hello,

I am porting code from NodeJS to browser. In NodeJS I use require('events').on (do not confuse with require('events').EventEmitter.on).

events.on is available since NodeJS v13.6.0 and returns <AsyncIterator> that iterates eventName events emitted by the emitter. It support option signal which can be used to cancel awaiting events. See details: https://nodejs.org/docs/latest-v16.x/api/events.html#events_events_on_emitter_eventname_options

I would like replace events.on with EventIterator, but it lack support signal aborting eg. via AbortController. AbortController is standard interface in browser and have raising deployment in NodeJS too. In NodeJS class AbortController is added in NodeJS v15.0.0 and since v15.4.0 is no longer experimental. See details: https://nodejs.org/api/globals.html#globals_class_abortcontroller

Is AbortController support in your area of interest?

rolftimmermans commented 3 years ago

I expect you can write a simple wrapper function for your use case that accepts the .signal property of the AbortContrller (AbortSignal):

function on(ee, event, {signal} = {}) {
  return new EventIterator(({push, stop}) => {
    ee.addListener(event, push)
    if (signal) {
      signal.addEventListener("abort", stop)
    }

    return () => {
      ee.removeListener(event, push)
      if (signal) {
        signal.removeEventListener("abort", stop)
      }
    }
  })
}

If this sort of thing is useful feel free to open a PR to include it in the documentation, and if this is a frequently occurring pattern I guess it can make sense to include it as a separate utility function?

ad-m commented 3 years ago

Thank for you comments. It's very useful.

I would like notice that NodeJS throw error on abort:

const main = async () => {
    const ee = new events.EventEmitter();
    const ac = new AbortController();

    setTimeout(() => ac.abort(), 1000);
    for await (const message of events.on(ee, 'message', { signal: ac.signal })) {
        console.log(message);
    }
};
$ nodejs a.js 
AbortError: The operation was aborted
    at abortListener (node:events:866:18)
    at EventTarget.<anonymous> (node:events:768:47)
    at EventTarget.[nodejs.internal.kHybridDispatch] (node:internal/event_target:459:20)
    at EventTarget.dispatchEvent (node:internal/event_target:407:26)
    at abortSignal (node:internal/abort_controller:98:10)
    at AbortController.abort (node:internal/abort_controller:123:5)
    at Timeout._onTimeout (/home/adas/Devel/rbx-api-caas/framework/lib/transport/a.js:36:25)
    at listOnTimeout (node:internal/timers:557:17)
    at processTimers (node:internal/timers:500:7) {
  code: 'ABORT_ERR'
}

See also https://github.com/nodejs/node/blob/5ce015ec72be98f064041d1bf5c3527a89c276cc/lib/events.js#L982-L984 .

I believe to mimic that behavior following changes is required:

class AbortError extends Error {
    constructor() {
        super('The operation was aborted');
        this.type = 'ABORT_ERR';
    }
}

const on = (ee, event, { signal } = {}) => {
    return new EventIterator(({ push, fail }) => {
        ee.addListener(event, push);
        const abort = () => fail(new AbortError());
        if (signal) {
            signal.addEventListener('abort', abort, { once: true });
        }

        return () => {
            ee.removeListener(event, push);
            if (signal) {
                signal.removeEventListener('abort', abort);
            }
        };
    });
};

The second difference I notice is that events.on passes an array of events. I have to consider how to manage it most effectively in order to be able to effectively maintain compatibility also in terms of such details.