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

Clarify behaviour when listening for 'error' event using once() #31244

Closed
aalexgabi opened this issue Jan 7, 2020 · 4 comments
Closed

Clarify behaviour when listening for 'error' event using once() #31244

aalexgabi opened this issue Jan 7, 2020 · 4 comments
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.

Comments

@aalexgabi
Copy link

The documentation for once states:

Creates a Promise that is fulfilled when the EventEmitter emits the given event or that is rejected when the EventEmitter emits 'error'. The Promise will resolve with an array of all the arguments emitted to the given event.

It's not clear from the documentation what happens when once is used to listen for error event. In my case this is the desired behaviors but I was surprised when my tests were not failing because an error event was emitted.

This:

const { EventEmitter, once } = require('events');

const ee = new EventEmitter();

once(ee, 'error').then(([err]) => {
    console.log('resolves', err);
}).catch((err) => {
    console.log('rejects', err);
})

ee.emit('error', new Error('Hi'))

Prints:

resolves Error: Hi
    at Object.<anonymous> ...

Can you clarify this behaviour in the documentation please?

@addaleax addaleax added doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter. labels Jan 7, 2020
@sam-github
Copy link
Contributor

Its documented here: https://nodejs.org/api/events.html#events_error_events

This issue isn't specific to the promisified methods. Perhaps every method that listens for an event needs to be linked to that section? Can you do a PR?

@aalexgabi
Copy link
Author

@sam-github I don't understand your comment. The section of the documentation you linked does not mention the behavior of once() but the behavior of an EventEmitter.

My issue is that when I read this:

Creates a Promise that is fulfilled when the EventEmitter emits the given event or that is rejected when the EventEmitter emits 'error'. The Promise will resolve with an array of all the arguments emitted to the given event.

I don't know what's going to happen when I listen for an error event using once() because both are true:

  • is fulfilled when the EventEmitter emits the given event
  • is rejected when the EventEmitter emits 'error'

@simonkcleung
Copy link

It is fulfilled if 'error' is emitted, ie the fulfilled handler will be called. The 2nd statement is ignored in this case.

@SowmyaGrama
Copy link

Can i work on this?

jasnell added a commit to jasnell/node that referenced this issue Jul 6, 2020
@jasnell jasnell closed this as completed in 949d1c1 Jul 9, 2020
MylesBorins pushed a commit that referenced this issue Jul 14, 2020
Fixes: #31244

PR-URL: #34225
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Jul 16, 2020
Fixes: #31244

PR-URL: #34225
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
codebytere pushed a commit that referenced this issue Sep 27, 2020
Fixes: #31244

PR-URL: #34225
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
addaleax pushed a commit that referenced this issue Sep 28, 2020
Fixes: #31244

PR-URL: #34225
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. events Issues and PRs related to the events subsystem / EventEmitter.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants