Use proposed new WebSockets.jl

[EricForgy]

I modified the original WebSockets.jl package to use HTTP.jl.

JuliaWeb/WebSockets.jl#87

This PR brings those changes here so that HTTP.jl uses the core - slightly modified / generalized -
functionality from the original and brings some proposed alignment.

One issue is this does not yet support SSLContext because the original WebSockets.jl didn't, but that should be straightforward to fix in both packages. Update: Support SSLContext now 😊

Note: I actually don't think we need a WebSockets.jl in this package since we can just use standalone package (assuming we can get it merged).

[EricForgy]

Hi @samoconnor and @quinnj 👋

It will take some time to get the other WebSockets package merged given I suspect more people are using that one. In the meantime, any chance we can merge / tag this one so I can use it in my package? 🙏

Is there anything you need that got lost?

[quinnj]

I don't think I fully understand the plan/reasons here. Are we just copy-pasting WebSockets.jl into this file? Ultimately, how much websockets code do we need in HTTP.jl? My gut says that the WebSocket code should all live in WebSockets.jl and HTTP would add a dependency on it to handle upgrade requests/etc. Does that sound right to anyone?

[quinnj]

why is this block all commented out?

[quinnj]

might as well just take out this definition if it's just commented out?

[EricForgy]

Hi @quinnj and @samoconnor ,

I am happy to briefly share my story and why I even care about this which leads to plans / reasons.

TL;DR

In a nutshell, given the faster-than-expected progress with the WebSockets.jl and HTTP.jl integration, I am in favor of withdrawing this PR AND removing HTTP.WebSockets.jl all together and just focusing on improving the original WebSockets.jl package now that it is integrated with HTTP.jl (at least on a dev branch).

Longer Version

I am cofounder of a 28-person (and growing) fintech startup in Asia. So far, I have 5 awesome .NET developers for back-end work and 5 awesome Node / React developers for front-end work and over the past 2 years we've developed an awesome microservices architecture on Azure (Service Fabric). I would now like to introduce Julia as a microservice for some of the mathematical modeling we need to do. Almost 2 years ago to the day, I started work on Pages.jl. It is a very simple package, but allowed me to easily create API endpoints and interact with the browser via WebSockets for experimentation. I've been busy building the business since then, so serious work on Pages.jl hasn't happened yet and may not happen as good alternatives (including functionality in HTTP.jl) have appeared. As I was dusting off my Julia, I noticed you guys working on HTTP.jl. The original Pages.jl was based on HttpServer.jl, but I did not have a lot of confidence that it would perform on par with the rest of our awesome tech. I could tell that you @quinnj had made some serious improvements and @samoconnor was working hard to make HTTP.jl production worthy. So... I decided that it would be wise to hitch myself to your efforts. I think HTTP.jl is a good basis for the future of serious Julia web apps.

BUT... I need WebSockets. The original WebSockets.jl package only worked with HttpServer.jl. Around the time I started getting back into things, I noticed the issue JuliaWeb/WebSockets.jl#84 and decided to try to help out. I wasn't sure how to proceed. I have no clue about building WebSocket libraries. The initial idea was to integrate the original WebSockets.jl with HTTP.jl BUT @samoconnor had already built a fairly complete WebSockets submodule. Now we have two WebSockets modules 🤔

Rather than integrate the old WebSockets.jl with HTTP,jl, I proposed keeping the two modules separate and we would just focus on the WS module contained in HTTP.jl. I spent A LOT of time staring at this code and learned a lot. By now I've read the IETF specs several times and can almost draw the WS byte frame by memory 😅 Not a skill I was setting out for, but it is all good.

I was making little tweaks here and there to HTTP.WebSockets, but the more I tweaked, the more it started looking like the original WebSockets.jl package, which was laid out in a way my poor old brain could comprehend slightly easier. After spending way more time than I'd like to admit, I had learned enough that I could easily see what is going on with the original WebSockets. It was clearly designed as a server-centric library and HTTP.WebSockets.jl was client-centric, e.g. the only test was a WS client connecting to an external server. By now, I've learned how to make a WS module that can act as both a client and a server.

I recently integrated the original WebSockets.jl with HTTP.jl, but in the process, I stripped out its dependence on HttpServer.jl. That is a breaking change that I thought was highly unlikely to get merged any time soon, but I need it and it has been merged to a development branch https://github.com/JuliaWeb/WebSockets.jl/tree/change_dependencies. Since I need WS and I thought the changes to WebSockets.jl wouldn't get merged any time soon, I decided to just replace the new WebSockets.jl that works with HTTP.jl into THIS package. The more I understand both WebSockets.jl and HTTP.WebSockets.jl, the less I see a need for two separate packages. We should take the good stuff from HTTP.WebSockets.jl and just incorporate into WebSockets.jl.

Now, I am actually optimistic that we CAN get the HTTP.jl integration with the original WebSockets.jl merged sooner than I thought. With this PR, JuliaWeb/WebSockets.jl#88, I have stripped both HTTP.jl and HttpServer.jl as dependencies from WebSockets and I load appropriate methods depending on whether you are using HttpServer.jl or HTTP.jl. For example,

julia> using HTTP

julia> using WebSockets
INFO: Loading HTTP methods...

or

julia> using HttpServer

julia> using WebSockets
INFO: Loading HttpServer methods...

Not only that, the new changes to Websockets.jl allow you to use WebSockets equally easy as either a server or a client.

I am REALLY happy with this solution. All WebSockets.jl tests are passing plus the new HTTP.WebSockets.jl tests are also passing all in the same WebSockets package! I have some minor clean up and there is likely going to be some discussion (mostly because I removed the field id from the WebSocket object after @samoconnor explained to me in a convincing way we don't need or want that).

This afternoon, I got the chat example working and in short order I should have everything cleaned up and ready to merge so that the independent WebSockets.jl will work out of the box with both HTTP.jl AND HttpServer.jl. That would be awesome.

At that point, it raises a question for you guys (@samoconnor and @quinnj ): Do we even need a WebSockets module as part of HTTP.jl if the separate WebSockets.jl already works with HTTP.jl? I think not, so I would actually recommend removing WebSockets completely from HTTP.jl and move any effort to WebSockets.HTTP, i.e. https://github.com/EricForgy/WebSockets.jl/blob/HTTP.jl/src/HTTP.jl, which will hopefully have a home in the official WebSockets.jl package once we get that merged.

Sorry for the long response, but I hope that makes some sense. In a nutshell, given the faster-than-expected progress with the WebSockets.jl and HTTP.jl integration, I am in favor of withdrawing this PR AND removing HTTP.WebSockets.jl all together and just focusing on improving the original WebSockets.jl package now that it is integrated with HTTP.jl (at least on a dev branch).

I understand @samoconnor has some good questions and those deserve good answers and I'll work on them, but probably over at WebSockets.jl. It would be great if we could get WebSockets.jl to be fully compliant with the IETF specs and I'll do my best to help see that happen.

[quinnj]

My preference would be for all the websockets code to live in WebSockets.jl and then include WebSockets.jl as a dependency in HTTP.jl, so that upgrade requests can happen automatically. i.e., when you load HTTP.jl, you're also loading full websockets functionality.

[samoconnor]

From JuliaWeb/WebSockets.jl#84 (comment) on 9th Jan:

...I've implemented a WebSockets client (and test) in my HTTP.jl PR HTTP.jl branch: #135. My motivation for doing this was that HTTP.jl did not have test coverage for the Upgrade: mechanism, and building a simple client for wss://echo.websocket.org seemed like the easiest way to understand how it is supposed to work and test how it interacts with connection pooling and pipelining.

As for division of functionality into packages I would prefer to see more separation rather than less. HTTP.jl currently aggregates code from a number of previous packages, which I think @quinnj would agree could eventually be split out again. Having now spent some time on the internals of HTTP.jl I can see why @quinnj found it necessary to combine the older packages before starting his mammoth refactoring and integration of his pure-julia parser.

My take at this point is that it is essential for HTTP.jl to include higher level protocols so that real-world tests can be implemented. For example my PR branch implements AWS authorisation to enable a [test that does lots of concurrent GET/PUT against AWS S3] (https://github.com/samoconnor/HTTP.jl/blob/simplify_parser_branch/test/async.jl#L19). This enabled me to find intermittent bugs in HTTP.jl, MbedTLS.jl (JuliaLang/MbedTLS.jl#117, JuliaLang/MbedTLS.jl#114) and Base JuliaLang/julia#25314.

[...]

I think HTTP.jl should try to build up and maintain a test suite that includes stress testing against real-wold servers as well as unit tests. This probably means having a test/REQUIRE dependancy on the relevant client packages after everything has been split up.

My motivation for writing HTTP.WebSockets.jl was to learn enough to design a test a good "upgrade" API. Having come this far, I'm reluctant to leave a remaining few issues on the table and say "WebSockets.jl works better, lets just use that", I would worry that the root cause of the remaining issues might be an undiscovered issue in the design of the HTTP.jl "upgrade" API.

So, my preference would be for Eric to forge ahead getting JuliaWeb/WebSockets.jl#88 merged; and to leave HTTP.WebSockets.jl as is for the moment. I would like to subject HTTP.WebSockets.jl to whatever tests Eric comes up with for WebSockets.jl in an effort to find any remaining corner cases (after those fixed in 2cea2f7).
When both are working, it would be good to do some stress tests, performance tests, memory usage tests etc and see if any benefits of one implementation can be shared with the other.
When it comes time to split things up, we obviously don't need to keep both implementations around.

[samoconnor]

... so that upgrade requests can happen automatically.

I'm not sure I follow how this would look from an API point of view. It seems to me that WebSockets is a layer above HTTP, so a WebSockets user would not need to interact with the HTTP layer. e.g. the websockets user writes:

using WebSockets
WebSockets.open("ws://echo.websocket.org") do io
    ...

i.e. use of WebSockets is user-driven, I don't see how HTTP.jl could automatically upgrade requests. Can you explain the use case you have in mind?

[EricForgy]

Good morning from Manila (the country I find myself in today - who knows where I will be tomorrow 😅 ).

Awesome to have some thoughts on the table 💪

There is one clear consensus that emerged. This PR has got to go 😊

I will close this now and maybe it is better to continue the discussion in an issue 👍

[Hugo-Trentesaux]

@EricForgy @samoconnor after one year, were are we?