search

celluloid / reel

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

Websockets.rb references non-existant Response#render #165

Closed tinco closed 9 years ago

tinco commented 9 years ago

Hi,

I'm trying to use your websockets class, and it's throwing an error that Response#render doesn't exist. I checked, and it's been removed from Response somewhere after stable-3-0.

Not sure if this has consequences for the reel server, or why your test suite hasn't caught it. I'll investigate further if I have time :)

tarcieri commented 9 years ago

That'd be great. It's possible there was something broken by changes that weren't covered by the test suite, although I'm surprised it hasn't been seen until now.

Do you have a reproduction of the problem?

tinco commented 9 years ago

Unfortunately I don't, I'd make a repro but unfortunately something else is eating my time at the moment.

You can follow my progress here: https://github.com/d-snp/celluloid-websocket-rack

I ran into some other incompatibilities so I decided to just extract your websocket class and try to make it work in the other app servers. At the moment I have some Rack specific code in there, so it won't work with Reel as it is, but I'm planning on removing the rack dependency so you could use it for Reel as well if you like. (it's a bit awkward that reel isn't rack compatible btw)

This celluloid stuff is really nice! I got the celluloid-websocket thing to work on Puma. On Passenger there's still a weird bug. I have a Rack object (basically an object with a call function that takes an env) that includes Celluloid, but the call function never gets called, something is going wrong in the proxy. Weird thing is that the initialize function does get properly proxied. The traces show that the call is properly put in a mailbox, but immediately after the Mailbox#<< the Mailbox#next_message return nil and @messages is 0 length.

Could it be a problem if Passenger calls #initialize in one thread and then #call in another? Would they land in different mailboxes, possibly one that never gets checked or something?

Anyway, that's irrelevant to reel. Sorry for ranting, I'm probably stuck with this problem for a while. If you think it's better to close this issue and wait for a repro case feel free to close the issue :)

tarcieri commented 9 years ago

If you're really interested in doing that, I'd suggest basing it off of jcoglan's work:

https://github.com/faye/websocket-driver-ruby

Here's a PR to switch Reel to use that:

https://github.com/celluloid/reel/pull/154

tinco commented 9 years ago

Oh.. that confuses me a little. I'm actually doing this so in the future I can move a project I'm working on off the faye websocket driver as it uses eventmachine and I don't really like it. I'll have to look at the PR but I'm surprised you can just integrate an EM project like that.

edit: Ohh I got confused, it's actually completely decoupled from EM. Very nice indeed :)

tarcieri commented 9 years ago

websocket-driver isn't tied to EventMachine. It's a sort of agnostic core protocol driver

On Sunday, February 1, 2015, Tinco Andringa notifications@github.com wrote:

Oh.. that confuses me a little. I'm actually doing this so in the future I can move a project I'm working on off the faye websocket driver as it uses eventmachine and I don't really like it. I'll have to look at the PR but I'm surprised you can just integrate an EM project like that.

— Reply to this email directly or view it on GitHub https://github.com/celluloid/reel/issues/165#issuecomment-72391448.

Tony Arcieri

tinco commented 9 years ago

Alright, I fixed it up, now it runs in Passenger as well. I had to make the Rack object not be an Actor itself. I think the process got forked, and new messages were delivered to a mailbox that wasn't being checked. It's much better now, as it correctly creates an actor for every new connection as well. I'm getting the hang of the celluloid way I think.

I will port the changes of #154 tomorrow if I have time. If I get it to work I'll give feedback in that thread so you can get either the pull request merged or use any changes I make to it.

I'll close this issue as I don't think it's worth thinking about the old websocket.rb if there's a brand spanking new one in a PR :)

digitalextremist commented 9 years ago

Are you going to complete #154 @d-snp?

tinco commented 9 years ago

I intend to, it's not (directly) work related so I will do it in the evening, sometime this week, hopefully tonight.