celluloid / reel

UNMAINTAINED: See celluloid/celluloid#779 - Celluloid::IO-powered web server
https://celluloid.io
MIT License
596 stars 87 forks source link

Switched to websocket/driver and improved websocket handling #110

Closed robertjpayne closed 9 years ago

robertjpayne commented 10 years ago

This pull request should be considered a WIP, I just wanted to open it for some feedback and sharing.

In general I found WebSocket support in Reel to be a tad flaky and hard to get working since there are not many good examples of keeping a long running persistent WebSocket open in Reel.

This pull request does the following:

What I would still like to achieve with websocket support in reel:

robertjpayne commented 10 years ago

For anyone wanting to potentially use this without integrating the pull request I have a standalone handler here:

https://gist.github.com/robertjpayne/6961149

Unfortunately this will not integrate with webmachine nicely.

tarcieri commented 10 years ago

Nice! Thanks for doing this! This is something we have planned on for awhile but never had time to do.

It looks like all the tests are failing though?

NameError:
       uninitialized constant WebSocket::ClientHandshake
robertjpayne commented 10 years ago

I'll have a look at the tests, this pull request is very much a work in progress too. The actor that captures the WebSocket is pretty much locked onto the socket so I'm not entirely sure how useful it will be without some mechanism to feed messages back to the client or if that is something we should let others solve!

I'd love to port FAYE to use Celluloid/Reel instead of EventMachine but that may take a bit more effort than I have time for right now too.

tarcieri commented 10 years ago

@robertjpayne having connections locked to a specific actor is intended behavior. It should be possible to send messages when callbacks fire within that specific actor, as well as set timers, although timers are presently broken with Celluloid::IO (see https://github.com/celluloid/celluloid-io/issues/56)

robertjpayne commented 10 years ago

Just for anyone that may be watching this I'm struggling to get a spec setup suitable for testing the websocket driver as it blocks the calling thread.

@tarcieri I'm not sure if you could help out here, maybe we can hash it out over IRC but I'm not having any luck bending rspec to my will to avoid things blowing up like mad and getting proper testing out of the specs.

Asmod4n commented 10 years ago

blocking the calling thread means Reel can only handle one connection? Solved: https://gist.github.com/Asmod4n/8654523 :)

tarcieri commented 10 years ago

@Asmod4n that makes a separate thread per Websocket connection. Ideally they could all use the same thread.

@robertjpayne I'd be happy to help pair with you over IRC to get this finished. It'd be nice for Reel to have solid Websocket support

Sen commented 10 years ago

hi @robertjpayne @tarcieri , any progress about this feature?

tarcieri commented 10 years ago

Nope :|

Asmod4n commented 10 years ago

Have made a small gist on how to use the websocket-driver lib with celluloid-io: https://gist.github.com/Asmod4n/241c84ef31df921b31e9

@tarcieri @robertjpayne

tarcieri commented 9 years ago

Fixed in #166