proxygen icon indicating copy to clipboard operation
proxygen copied to clipboard

Sample application combining Websocketpp and Proxygen

Open Dalzhim opened this issue 7 years ago • 8 comments

There were a number of features that didn't exist in other software (some still don't) that seemed quite difficult to integrate in existing projects but would be immediately useful for Facebook. Examples of features that we were interested in included SPDY, WebSockets, HTTP/1.1 (keep-alive), TLS false start, and Facebook-specific load-scheduling algorithms.

The quote above comes from the original blog post announcing Proxygen (emphasis mine), and it confused me when I started using it as I believed WebSocket support was builtin. It is only much later that I realized that it has never been made public.

I have worked on a proof of concept to bring together Websocketpp's support for WebSockets on top of Proxygen in order to try and drive forward real WebSocket support within Proxygen.

Here are a few highlights describing this contribution :

  • It is a proof of concept
  • It only supports server-side websockets
  • There is unnecessary overhead :
    • Websocketpp represents payloads as std::string and getting it to use folly::IOBuf was not trivial
    • The initial HTTP/1.1 request's headers are parsed twice overall. Once by proxygen, and once by Websocketpp.
    • An unnecessary copy of the headers is created to allow Websocketpp to read the headers
  • This sample has a dependency on the Websocketpp project, but I am not familiar enough with autotools to configure an optional dependency and avoid building this sample when the dependency is not present. At the moment I expect the build to fail in the absence of that library.

Even though this first draft is limited, I'm hoping it may drive forward full WebSocket support with proxygen eventually.

Dalzhim avatar May 17 '17 00:05 Dalzhim

Hi @Dalzhim thank you for taking the initiative to provide websocket support for open-sourced proxygen! It is towards our upcoming goal to extend the open-sourcing of proxygen modules and we should certainly consider the websocket part.

On the other hand if we want to create this support feature available for users right now, can we consider moving it to external?

Thanks, Lanxi

lanxi avatar May 30 '17 15:05 lanxi

Hi @lanxi ! I'm glad to hear there are still plans to extend the open-sourcing of proxygen modules such as WebSocket. That would completely deprecate the Websocketpp bridge I have implemented in this PR which isn't as efficient as it should be.

For the time being, moving this feature to external seems very appropriate! Thank you!

Dalzhim avatar May 30 '17 16:05 Dalzhim

I'd like to verify, were you asking me if you can move the new sample app I created to the external directory, or were you asking me to do it? Also I'm guessing it would be preferable if this sample application wasn't part of the binaries that are built when using the instructions out of the box. I think it should be built only when people explicitly want to experiment with it as it requires a third party library (websocketpp).

Dalzhim avatar Jun 09 '17 16:06 Dalzhim

@afrind has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot avatar Jun 15 '17 19:06 facebook-github-bot

Hi @Dalzhim, really happy to find your pull request for websocket. We are currently using proxygen for our server, and going to support websocket, this really help us a lot!

The WebSocketHandler is very simple to use, but I have a question, how can I support HTTP and websocket at the same time for the same port? Since we need to keep the compatibility for the original users.

Thanks.

kapcino avatar Jan 17 '18 09:01 kapcino

Hi @kapcino, I'm very happy this pull request was useful to you! Please be aware that there are at least 2 things that should really be improved about it before it is production ready:

  1. Search for the single const_cast I used to cheat around a problem I had and find a way to get rid of it
  2. Profile real WebSocket sessions to make sure the conversions from Proxygen's folly::IOBuf to Websocketpp's std::string are not a problem for you

Otherwise, the solution I have used to support both HTTP and WebSocket requests on the same port is to instantiate a different RequestHandler based on the GET request. Basically, some of my routes were HTTP only while others were WebSocket only. The RequestHandlerFactory is where I make that choice before I instantiate the appropriate RequestHandler instance.

I wish you good luck. Unfortunately, I have given up on this Proxygen+Websocketpp solution and migrated towards https://github.com/boostorg/beast so I don't plan any further contributions here.

Dalzhim avatar Jan 18 '18 04:01 Dalzhim

Thank you @Dalzhim I will check the items you mentioned. I am also curious about the reason you abandoned Proxygen solution, could you please share some of the considerations? We checked Boost beast before, and found the interface was low level and not as clean and simple as Proxygen.

kapcino avatar Jan 18 '18 15:01 kapcino

@kapcino: I moved away from proxygen for various reasons :

  • Build can sometimes be broken for multiple weeks
  • Lack of support for macOS
  • Lack of direct support for WebSockets
  • Large amount of dependencies my project doesn't need otherwise
  • Lastly: Facebook, as a company, doesn't seem strongly committed behind this project and I don't see most of my list changing in the near future

Dalzhim avatar Jan 23 '18 02:01 Dalzhim

Closing old pull request. While WebSocket support might be suboptimal, WebTransport (new hotness) is supported as a first class construct.

afrind avatar Sep 16 '24 21:09 afrind