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

indefinite behavior of nodejs/events.once() on WebSocket #1795

Closed
zimtsui opened this issue Sep 3, 2020 · 2 comments
Closed

indefinite behavior of nodejs/events.once() on WebSocket #1795

zimtsui opened this issue Sep 3, 2020 · 2 comments

Comments

@zimtsui
Copy link

zimtsui commented Sep 3, 2020

hello guys.

the function once of nodejs v12 LTS makes different behaviors between EventEmitter or EventTarget. REF

This method is intentionally generic and works with the web platform EventTarget interface, which has no special 'error' event semantics and does not listen to the 'error' event.

once makes judgement about whether an emitter is an EventEmitter or an EventTarget according to whether emitter.addEventListener is a function. REF

since WebSocket implements addEventListener in

const { addEventListener, removeEventListener } = require('./event-target');

once will misjudge the emitter to be an EventTarget and it will not listen to error event. thus, users cannot do:

async function connect(){
    ws = new WebSocket('xxx');
    await once(ws, 'open');
}

this will be waiting forever if it fails to connect.

since it's said in docs that

This class represents a WebSocket server. It extends the EventEmitter.

, this will mislead users.

but this problem does not exist with nodejs v14, because nodejs v14 makes that judgement via whether emitter.on is a function. REF

i don't think it's a bug, but it'd be better if this problem should be warning in docs

@lpinca
Copy link
Member

lpinca commented Sep 3, 2020

I think nodejs/node#34015 will be backported to Node.js 12 so the issue should only be temporary.

lpinca added a commit to lpinca/node that referenced this issue Oct 26, 2020
Assume that the `emitter` argument of `EventEmitter.once()` is an
`EventEmitter` if `emitter.on` is a function.

Refs: nodejs@4b3654e923e7c3c2
Refs: websockets/ws#1795
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 16, 2020
Assume that the `emitter` argument of `EventEmitter.once()` is an
`EventEmitter` if `emitter.on` is a function.

Refs: 4b3654e923e7c3c2
Refs: websockets/ws#1795

PR-URL: #35818
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@lpinca
Copy link
Member

lpinca commented Nov 25, 2020

I'm closing this as the issue should be fixed in Node.js v12.20.0

@lpinca lpinca closed this as completed Nov 25, 2020
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