stream: fix eventNames() to not return not defined events

[IlyasShabi]

Fix issue #51302
As explained by @Qard, the root cause is that by default, we pre-allocate events here here

1. Since those events are initially set to undefined, we should check the value of the listener before returning eventNames().
2. I also believe that we are protected because we are not allowed to set a null or undefined listener.

I did different tests like:

const readableStream = createReadable();
readableStream.on('a', () => {});
readableStream.on('error', () => {});
console.log(readableStream.eventNames()); // ['a', 'error']

const readableStream = createReadable();
readableStream.on('a', () => {});
console.log(readableStream.eventNames()); // ['a']

I will add tests as soon as I can

[Qard]

Yep, that's basically what I had in mind. I'd maybe make it check if it's an array and non-empty though to be more accurate rather than just the simple check that it's not undefined. Can you add some tests? 🙂

[benjamingr]

LGTM, please add a test :]

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/56622/

[Qard]

LGTM, but a suggestion: the tests currently rely on the implementation detail of streams doing the _events pre-allocation. It may be a good idea to manually reproduce the behaviour on a plain event listener as streams could change in the future to not do that anymore. 🤔

[lpinca]

I wonder if it is better to override eventNames() only on classes where _events is pre-allocated. We don't do this with the base EventEmitter.

[IlyasShabi]

I wonder if it is better to override eventNames() only on classes where _events is pre-allocated. We don't do this with the base EventEmitter.

@lpinca I agree with you, this is related to Stream not EventEmitter, I pushed the changes, could you please check again ?
cc @Qard @benjamingr

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57205/

[IlyasShabi]

I think flaky tests 🤔 no ?
@lpinca

[lpinca]

Yes.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57216/

[IlyasShabi]

Failed on this one wasi/test-wasi # TODO : Fix flaky test
@lpinca should I do something to merge this one ?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57342/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57397/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57450/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57474/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/57477/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/51331
✔  Done loading data for nodejs/node/pull/51331
----------------------------------- PR info ------------------------------------
Title      stream: fix eventNames() to not return not defined events (#51331)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     IlyasShabi:stream-event-names -> nodejs:main
Labels     events, needs-ci
Commits    1
 - stream: fix eventNames() to not return not defined events
Committers 1
 - Ilyas Shabi 
PR-URL: https://github.com/nodejs/node/pull/51331
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Stephen Belanger 
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/51331
Reviewed-By: Benjamin Gruenbaum 
Reviewed-By: Stephen Belanger 
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last approving review:
   ⚠  - stream: fix eventNames() to not return not defined events
   ℹ  This PR was created on Mon, 01 Jan 2024 12:43:44 GMT
   ✔  Approvals: 3
   ✔  - Benjamin Gruenbaum (@benjamingr) (TSC): https://github.com/nodejs/node/pull/51331#pullrequestreview-1799869531
   ✔  - Stephen Belanger (@Qard): https://github.com/nodejs/node/pull/51331#pullrequestreview-1890807762
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/51331#pullrequestreview-1890926622
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2024-02-27T21:35:07Z: https://ci.nodejs.org/job/node-test-pull-request/57477/
- Querying data for job/node-test-pull-request/57477/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/8072630772

[nodejs-github-bot]

Landed in a51efa2