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

Adding Websocket support to core #19308

Open
MylesBorins opened this issue Mar 13, 2018 · 113 comments
Open

Adding Websocket support to core #19308

MylesBorins opened this issue Mar 13, 2018 · 113 comments
Labels
feature request

Comments

@MylesBorins
Copy link
Member

MylesBorins commented Mar 13, 2018

The original thread where adding this was discussed #1010 was closed with a decision by the iojs TC to rather implement lower level buffer methods, but that was abandoned.

There is an open EPS to add the feature, but we have since abandoned the process.

Some of the people who originally were -1 changed their opinions in #1010 more recently. In fact, we already ship a partial implementation of ws in the inspector.

I think it might be worth us revisiting adding WS to core.

/cc @eugeneo @rauchg

@MylesBorins MylesBorins added the feature request label Mar 13, 2018
@eugeneo
Copy link
Contributor

eugeneo commented Mar 13, 2018

Re: Inspector.

  1. Inspector WS implementation is not complete, e.g. there is no support for binary frames.
  2. Inspector would still need a C++ implementation that can run on a separate thread. Both JS execution and main libuv loop are suspended when the application hits a breakpoint.

@devsnek
Copy link
Member

devsnek commented Mar 13, 2018

i'm not wholly against this but i would like to factor in how much stuff we put into all our release binaries. if we can come up with more creative ways for shipping stuff like this i'm totally a +1 (#19307)

@mscdex
Copy link
Contributor

mscdex commented Mar 13, 2018

I am still -1 on this.

@devsnek
Copy link
Member

devsnek commented Mar 13, 2018

@mscdex can you be explicit in your reasoning?

@mscdex
Copy link
Contributor

mscdex commented Mar 13, 2018

@devsnek for the same reasons I gave in the linked issue.

@lpinca
Copy link
Member

lpinca commented Mar 13, 2018

As discussed in #1010 and as a maintainer of ws I'm +1 on adding WebSocket to core.

@MylesBorins
Copy link
Member Author

MylesBorins commented Mar 13, 2018

@mscdex in #1010

-1 WebSockets is still something better suited for userland IMHO. Sure it's something that many people do use, but there are also many other widely used standardized protocols/APIs that could be argued as "core" to the "web" that are not currently included in node/io.js core. For example: multipart parsing/generation, Server-sent events, HTTP/2, ICMP, SSH/SFTP, FTP, SOCKS, VNC/RFB, SMTP/IMAP/POP3, SOAP, Web Workers (as an API), XHR/XHR2 (as an API), etc.

Since this original post we've added http2. While you listed a bunch of protocols not all of them are supported natively by the browser.

https://developer.mozilla.org/en-US/docs/Web/API/WebSockets_API

https://caniuse.com/#search=Websockets

@mscdex
Copy link
Contributor

mscdex commented Mar 13, 2018

@MylesBorins I don't think node should aim to become a (DOM-less) browser. Anyway, my "vote" and reasoning still stands.

@targos
Copy link
Member

targos commented Mar 13, 2018

It's more about communicating with browsers, not becoming one.

@bnoordhuis
Copy link
Member

bnoordhuis commented Mar 13, 2018

http2 is an argument against being too eager to absorb protocols into core. Neither its API nor its implementation are all that great; it would have benefited from iterating outside core for a while.

I guess you could construe that as an argument in favor of websockets: third-party modules have existed for years and their APIs and implementations have pretty much crystallized by now.

@brianleroux
Copy link

brianleroux commented Mar 13, 2018

This is a great idea. While today Node is already an indispensable tool for web developers it does not enjoy the same full seat at the table of web browser tech advancement despite being held completely captive by it. Few agree on the controversial edicts like Promise and esmodules but we can definitely all agree these transition moments could have been handled better with Node being an a fully active participant instead of recipient of these challenges.

Node has a big opportunity to become a full fledged user agent (web browser) and first class support for web features will be a part of that. +1!

@jasnell
Copy link
Member

jasnell commented Mar 13, 2018

http2 is an argument against being too eager to absorb protocols into core. Neither its API nor its implementation are all that great;

PRs welcome.

Re: websockets

I'm still -1 for the time being. This is something that has been done quite well by userland and there are still unanswered open standards questions about http2+ws that require more thought and experimentation.

@watson
Copy link
Member

watson commented Mar 13, 2018

When http2 was added, I think one of the considerations were that we could do a lot of low level stuff in C++ land that was hard or even impossible(?) to do efficiently in user-land. Is there a similar reason for wanting to bring WS into core, or is it just to have more features?

@MylesBorins
Copy link
Member Author

MylesBorins commented Mar 13, 2018

@jasnell can you point me towards the open standards discussion regarding h2 + ws?

@jasnell
Copy link
Member

jasnell commented Mar 13, 2018

The ietf httpbis working group is working on this .... https://tools.ietf.org/html/draft-ietf-httpbis-h2-websockets-00

It's still super early in the process tho there is some early implementation happening.

@kof
Copy link
Contributor

kof commented Mar 13, 2018

Ideally there should be a clear general framework on how to decide whether something belongs to the core or not. I would consider points like

  • is it hard to be done right?
  • is it widely used?
  • does it have a clear spec?
  • is it likely to become obsolete in the near future?
  • does it need optimizations from the core?
  • will the entire community benefit from it being part of the core?

@addaleax
Copy link
Member

addaleax commented Mar 13, 2018

is it hard to be done right?

@kof I fully agree with all your points except this one – there is no reason to believe that code in Node core is better-written or better-maintained than userland code in general.

I think a better question in its place would be “Is it hard to do effeciently without native addons?”.

@kof
Copy link
Contributor

kof commented Mar 13, 2018

@addaleax The idea behind that point is: if it is in userland, it is likely there will be multiple competing implementations, which will result in attention not being fully focused on a single code base and as a result lower quality, just because less people are working on it or using it. I have no data to back it up.

Userland is good when a feature needs experimentation.

@lpinca
Copy link
Member

lpinca commented Mar 13, 2018

@kof

is it hard to be done right?

No.

is it widely used?

Yes it is.

does it have a clear spec?

Yes.

is it likely to become obsolete in the near future?

No.

does it need optimizations from the core?

Not necessarily but optimizations in core may help. In ws we have two optional binary addons. One for the masking/unmasking of frames (bufferutil) and one for the UTF-8 validation (utf-8-validate).

will the entire community benefit from it being part of the core?

Yes I think the community will benefit from WebSocket in core.

There are a lot of userland implementations. The most popular are ws, faye-websocket, and websocket. Combining the the best parts of each one of them to create a single module in core would be great imho.

@mcollina
Copy link
Member

mcollina commented Mar 13, 2018

+1, mainly because @lpinca has been doing a great job in maintaining ws the last few years.

@MylesBorins
Copy link
Member Author

MylesBorins commented Mar 13, 2018

pinging @jcoglan and @theturtle32

@watson
Copy link
Member

watson commented Mar 13, 2018

So far I haven't seen any concrete comments on why implementing WS in core would be better than having it in user-land. I'm sure @lpinca have done a great job maintaining ws, but that hardly qualifies as a reason to bring it into core right?

I just think there need to be concrete examples on how core can provide a better WS implementation than user land 😃

@devsnek
Copy link
Member

devsnek commented Mar 13, 2018

i don't think there's anyone saying we would do a better job. i think the argument is better integration and since we can ship it with a native backing users will get a perf boost without having to install anything.

@watson
Copy link
Member

watson commented Mar 13, 2018

@devsnek Could you provide an example of how the integration would be better if WS was in core?

Regarding native backing then n-api and pre-compiled binaries should remove the requirement of having users compile stuff (if I understood your argument correctly).

@TimothyGu
Copy link
Member

TimothyGu commented Mar 14, 2018

It seems the main argument for adding ws to Core is to reduce binary dependencies. Since both native dependencies are buffer-based, I think they may be good candidates for adoption of WebAssembly. @lpinca has that been seen as a possibility?

@MylesBorins
Copy link
Member Author

MylesBorins commented Mar 14, 2018

For an API, I'd like to propose the WhatWG WebSocket API, as much as we can implement the standard (I know there will be some inconsistencies around events).

If we run into any serious problems with the API design we can attempt to work with the whatwg to update the standard. It seems like @TimothyGu has participated in this document along with @domenic.

re: http/2 + ws, if we get an implementation going we can participate in trying to solve this problem!

will the entire community benefit from it being part of the core?

I think implementing standards in core, especially ones implemented in the webplatform, can help the ecosystem focus on creating better abstractions on top of a protocol like WS, and avoid having to implement the base protocol themselves. If the various ecosystem modules share a kernel it allows there to be collaboration across projects with a shared interest, and potentially brings more people to help maintain core itself.

@lpinca
Copy link
Member

lpinca commented Mar 14, 2018

@watson userland implementations work and I'm a big supporter of small core but HTTP(s) and many other modules are in core because they are so popular that it made sense to create a core module for them. The same is valid for WebSocket in my opinion. Quoting @domenic from nodejs/NG#10 (comment)

I think the conclusion of this line of thought leads us to a process wherein we say "yes, core is interested in supporting feature X." Then, someone---maybe an io.js collaborator, or maybe not!---goes off and builds a prototype of feature X as a user-land module. Along the way, they might need to ask for more low-level APIs from core, and those will get rolled in. But over time, this external implementation of feature X matures, and io.js collaborators start commenting on it, and the ecosystem starts using it, until we decide: yes, we should roll this in to core, and start shipping with it.

Is core interested in supporting WebSocket? That's an open question. I think it should.

@TimothyGu we didn't explore that possibility but that's definitely a good idea. We already have prebuilt and n-api based binaries for native addons. Removing binary dependencies is not the reason why I would like to see WebSocket in core though. The question is, why is HTTP and HTTP2 in core and WebSocket isn't? They all are in the same league and they all can live in userland if wanted. On top of that a good part of the WebSocket implementation is already in core as the upgrade mechanism is already available and supported in core.

@MylesBorins If we ever decide to experiment with WebSocket in core, API need to be discussed. I think we want to support piping data around so a websocket should be implemented as a Duplex stream like faye-websocket does. I guess we also want to support fragmented messages and permessage-deflate. In this case we should augment the standard send() to add the ability to specify if a message is the last one in a fragmented message and to specify if a message should be compressed or not.

@M3kH
Copy link

M3kH commented Mar 14, 2018

I can't establish a vote for this, although would be a nice to have.

I found that WebSocket implementation isn't that solid neither in the Browser.

Is high cpu consuming and so far I couldn't see any implementation in a big scale (which I would have guess they would be using it); eg: Facebook, Twitter and Gmail, they rather relay on polling.

I guess is mainly related on the cost created on the Server side due being really hard to scale it (eg. Load balancing). Not sure Node wants to make his capacity in supporting it.

@nicolasnoble
Copy link
Contributor

nicolasnoble commented Mar 27, 2018

Cross referencing bugs here: introducing websockets into the core could potentially help with issues like socketio/socket.io#3212 in the long run, where people are relying on binary extensions to improve websocket speed - which is evidently not a good idea.

@bertolo1988
Copy link

bertolo1988 commented Mar 30, 2018

In the long run aren't users going to prefer http2 streaming capabilities over websockets?

@josephg
Copy link
Contributor

josephg commented Mar 30, 2018

Huh? I haven't read anything to suggest that http2 will supersede websockets. http2's streaming is bidirectional but not symmetric - only the client can initiate requests. Unless something changed recently, server side push doesn't bubble responses up to JS code. Also (afaik) just like http, http2 doesn't make any guarantees about preserving the order of client->server messages.

You could implement your own message-oriented streaming protocol on top of http2 using POST messages for C->S and server-sent events for S->C. But you will have to implement your own message ordering code. And you'll have load balancer problems. Websockets are sticky-by-default. That is to say, if you have a cluster of frontend servers with a load balancer, a websocket connection will stay connected to a single frontend machine for its lifetime. Configuring load balancers to replicate that behaviour on top of normal http requests is a black art. Corner me in a bar sometime and I'll tell you all about the nightmares we had getting sticky sessions working correctly through ELB a few years ago. (I understand some libraries try to silently migrate sessions. This introduces its own concurrency problems, and it doesn't work for all use cases.)

So, no. In the long run I hope websockets are reimplemented on top of http2. Although to be honest most of http2's benefits won't make much difference for websockets anyway. Over http2 the initial WS connection & handshake will be faster (since it can reuse the TCP connection it loaded the page on), but once its established it won't really matter. When websockets are implemented in http2, it will still make sense to have websocket code inside nodejs.

@arxpoetica
Copy link

arxpoetica commented Apr 21, 2018

[@theturtle32 said:] I don't feel it's anywhere near as widely used or fundamental as HTTP is, and it would end up just being more for the core team to have to maintain.

As others have pointed out, adoption is actually high.

[@josephg said:] will the entire community benefit from it being part of the core? (no - websockets are used widely but still only in a minority of projects)

This is a false negative; it may be a chicken and egg scenario where adoption seems lower (it's not) because the tooling is not easy to use and natively available. I personally believe this to be true. Adoption will be much higher with the right low-level API.

[@josephg said:] Personally I think there's a middle-ground that I'd like even better: Have an official list of recommended solid, default modules for specific tasks in nodejs. These could be decided on however we like. The official documentation should list these modules.

Off topic, but it could easily solve the "should we" paradigm. Too much to say on that here. Can it be picked up elsewhere?

[@mikeal said:] Node.js adopting new core protocols of the Web is perfectly in-line with its original vision and only a dogmatic originalism could argue otherwise. There may be perfectly good technical reasons not to include Web Sockets but the argument that it is somehow out of scope doesn't match the history or vision the project has had since day one.

EXACTLY this.

[@rauchg said:] My opinion is that WebSocket should be in core. Node.js was designed and has thrived as a platform for writing network services.

As someone who has had to implement WS on a few different projects, I am certain of two things: usage will increase dramatically over the coming years by particular verticals (storytelling engineers, for one). As well, WS are never easy to use. As @mikeal said, one of the things Node.js has had going for it from the beginning was the sense that it was built to be interoperable with the browser. Java, Ruby, Python, Go, and all the other low-level languages have never been easy to work with in terms of browser interoperability, and one of the explicit reasons is the difficulty in interacting with basic protocols such as http. Userland has built up a nice ecosystem around WS, but the tooling and ease-of-use factor is still a hell-a-difficult.

Treat WS as the first-class citizen it deserves to be treated. This should have been done 5 years ago.

@galvez
Copy link

galvez commented Jul 18, 2018

Treat WS as the first-class citizen it deserves to be treated. This should have been done 5 years ago.

Especially with the growing popularity of WAMP-based apps.

+1 on this.

@GrosSacASac
Copy link
Contributor

GrosSacASac commented Jul 26, 2018

From the about page

About Node.js®
As an asynchronous event driven JavaScript runtime, Node is designed to build scalable network applications.

Node should definitely have a WebSocket support and Server-sent Events, TPC, UDP, HTTP, HTTP2.

@dipser
Copy link

dipser commented Sep 11, 2018

A lot of IT-student in germany have to write websocket-applications. From this standpoint alone, it should be native. nodejs feels incomplete, if it isnt. You just use it natively in a browser, but in node.js you first have to find out, its not supported, then you have to decide which userland implementation is ok and future proof? At this point, i would be annoyed. Because you will feel like nodejs is not the wonderland of server-communication you thought. The next thing as student is, you have to deal with a callback hell. How many give up at this point? Let alone, that javascript wasnt very loved in the past, but is developing (ES6) very fast and has the big advantage of learning only one language in front- and backend. If that wouldnt be the case, nodejs would have died long ago - but thats just my impression.

@damianobarbati
Copy link

damianobarbati commented Oct 20, 2018

Treat WS as the first-class citizen it deserves to be treated. This should have been done 5 years ago.

Definitely.

@bradisbell
Copy link

bradisbell commented Oct 25, 2018

I have strong opposition to this. Node.js is used for far more than web applications. Adding bloat to the core hurts these other types of applications and their portability, while providing no significant benefit to those who want it. Keep Node.js light and keep WebSockets in solid modules like ws.

@galvez
Copy link

galvez commented Oct 25, 2018

@bradisbell you have a valid point -- I just think WebSockets can be considered a core protocol of the web, very much like http2.

@arxpoetica
Copy link

arxpoetica commented Oct 25, 2018

@bradisbell this is probably the most valid point I've heard on this.

I would favor, alternately, some sort of tree-shaking methodology native to Node in that case. I.e., only import what's actually imported. Obviously waaaaaay beyond the scope of this ticket (how would such a thing even be possible), but that would be my 2 cents. Best of both worlds, actually. Significantly lighten Node core, but still have core/official requires available on demand.

@josephg
Copy link
Contributor

josephg commented Nov 7, 2018

What is the actual decision making process for a big change like this?

It'll be controversial no matter what the decision is. But it would be nice to have a clear official resolution on which way node will move forward with this. Can we bump this to the TC or something?

@devsnek
Copy link
Member

devsnek commented Nov 7, 2018

there's nothing stopping someone from making a pr. if that pr was blocked for a non-technical reason (like "ws shouldn't be in core") it could be elevated to the TSC

@gengjiawen
Copy link
Member

gengjiawen commented Jul 9, 2019

@lpinca Mind create a PR ?

Just found go has websocket too https://godoc.org/golang.org/x/net/websocket.

In js ecosystem,new api System like Graphql subscription rely on this: https://github.com/apollographql/apollo-server/blob/678714293337be19163e7ae0f973c6d873cf3656/packages/apollo-server-core/package.json#L46.

@feross
Copy link
Contributor

feross commented Aug 4, 2019

FYI everyone: There's a new spec WebSocketStream that is intended to supersede WebSocket in the same way that fetch superseded xhr. The new spec supports backpressure.

If the decision is to add ws to Node.js, then perhaps we should skip the current API and just implement this new one, since backpressure is quite important. https://groups.google.com/a/chromium.org/forum/m/#!msg/blink-dev/X7rWpAkMCyg/j6K7mEEwAgAJ

(We should also wait until this API stabilizes and there is buy-in from other browsers besides Chrome)

@lpinca
Copy link
Member

lpinca commented Aug 4, 2019

FWIW I've recently added in ws a utility function to wrap a ws WebSocket in a Duplex stream with full backpressure support.

@karimhm
Copy link

karimhm commented Jun 4, 2020

+1 on this. So no one would forget it.

@aral
Copy link

aral commented Feb 15, 2021

A new year, a new +1.

(As someone whose rollup bundle is currently breaking due to use of optional binaries in – the most excellent and wonderful – ws. goes to figure out a hack)

@galvez
Copy link

galvez commented Feb 15, 2021

@aral FWIW I'm on the Fastify stack now and this is pretty solid: https://github.com/fastify/fastify-websocket

@reyadussalahin
Copy link

reyadussalahin commented Apr 2, 2021

And another +1. No one should forget.

@64J0
Copy link

64J0 commented Jun 11, 2021

I'm not sure if I can vote on this issue because I'm only a Node.js user but if I can there's another +1.

@buzzy
Copy link

buzzy commented Oct 10, 2021

+100 ;)

@Assassin-1234
Copy link

Assassin-1234 commented Oct 10, 2021

+69 :DD

@shellscape
Copy link

shellscape commented Oct 10, 2021

BT

@CodeBradley
Copy link

CodeBradley commented Oct 11, 2021

+1

1 similar comment
@alanxp
Copy link

alanxp commented Oct 29, 2021

+1

@arxpoetica
Copy link

arxpoetica commented Nov 30, 2021

Interesting development in the world of deno: https://deno.com/blog/deploy-streams#websockets

@PodaruDragos
Copy link

PodaruDragos commented Aug 5, 2022

Hello guys
Is the any progress regarding a consensus on this particular feature ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request
Projects
Status: Pending Triage
Development

No branches or pull requests