Conversation
lpinca
force-pushed
the
support/blob
branch
7 times, most recently
from
223c6fc to
a83321b
Compare
|
@kettanaito I'm still not very happy with this because, other than compatibility with the WHAWG spec, it is pretty much useless. Anyway, here it is. I also didn't run any benchmarks to see the impact on performance. |
| function emitErrorAndClose(websocket, err) { | ||
| websocket._readyState = WebSocket.CLOSING; | ||
| // | ||
| // The following assignment is practically useless and is done only for |
There was a problem hiding this comment.
Which summarizes the entire change pretty flawlessly. I am still grateful for this consistency! Thank you!
|
Hi there @lpinca, I can't confirm exactly what's wrong here, but I did a minor version bump and this entirely broke our implementation of |
|
@titanism are you setting |
nikwen
left a comment
nikwen
left a comment
There was a problem hiding this comment.
Was just looking at the commit to make sure that upgrading ws won't break my app. Looks great overall. Just had a small suggestion. :)
| websocket._errorEmitted = true; | ||
| websocket.emit('error', err); |
There was a problem hiding this comment.
This seems worthwhile to pull out into a small function:
function emitError(err) {
websocket._errorEmitted = true;
websocket.emit('error', err);
}Seems like these lines should always be called together. Pulling them out into a function will make it less likely that someone will forget to do so in the future.
Introduced in v8.18.0 via websockets/ws#2229
7 participants
Closes #2206