Skip to content
This repository has been archived by the owner on Dec 7, 2018. It is now read-only.

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

Closed
tinco opened this issue Feb 1, 2015 · 8 comments
Closed

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

tinco opened this issue Feb 1, 2015 · 8 comments

Comments

@tinco
Copy link
Contributor

tinco commented Feb 1, 2015

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
Copy link
Member

tarcieri commented Feb 1, 2015

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
Copy link
Contributor Author

tinco commented Feb 1, 2015

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
Copy link
Member

tarcieri commented Feb 1, 2015

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:

#154

@tinco
Copy link
Contributor Author

tinco commented Feb 1, 2015

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
Copy link
Member

tarcieri commented Feb 2, 2015

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
#165 (comment).

Tony Arcieri

@tinco
Copy link
Contributor Author

tinco commented Feb 2, 2015

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 :)

@tinco tinco closed this as completed Feb 2, 2015
@digitalextremist
Copy link
Member

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

@tinco
Copy link
Contributor Author

tinco commented Feb 2, 2015

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants