Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for AbortController #20

Open
ad-m opened this issue Jun 14, 2021 · 2 comments
Open

Support for AbortController #20

ad-m opened this issue Jun 14, 2021 · 2 comments

Comments

@ad-m
Copy link

ad-m commented Jun 14, 2021

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
Copy link
Owner

rolftimmermans commented Jun 15, 2021

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
Copy link
Author

ad-m commented Jun 15, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants