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

Support the Rack hijack API #42

Closed
wants to merge 2 commits into from
Closed

Conversation

halorgium
Copy link
Contributor

Rack now supports a "hijack" API which allows high-level frameworks built on top of Rack (e.g. Rails) to access the underlying socket:

rack/rack#481

It would be great if Reel supported this feature, as it would expose its end-to-end streaming support (along with Celluloid::IO-based sockets) to frameworks wishing to utilize the hijack API.

@digitalextremist
Copy link
Member

@halorgium if you feel like jumping on this, please go ahead. You might get to it before me and will know better than I would. As mentioned in IRC, I am not going to use Rack's hijack support in my own applications. I am seeing I need to regain some ground. It will likely take me several days to get back to this.

@halorgium
Copy link
Contributor

@digitalextremist sure. i hope you learnt some things while giving this a go!

@@ -139,7 +139,7 @@ def respond(response, headers_or_body = {}, body = nil)
# The client disconnected early
@keepalive = false
ensure
if @keepalive
if @keepalive || body.nil?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarcieri needs a more sane approach for telling the Connection to not close the socket.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, all Connections are detached with the Rack handler. see:

Celluloid::Actor[:reel_rack_pool].handle(connection.detach)

@halorgium
Copy link
Contributor

@tarcieri this seems to work, and both the example apps behave.
@raggi would you mind having a look at this too?

@coveralls
Copy link

Coverage Status

Coverage increased (+14.04%) when pulling 2e88d19 on halorgium:hijacking into a273967 on celluloid:master.

@digitalextremist
Copy link
Member

@halorgium nice work! Using your branch, I am able to verify full Rack-WebSocket compatibility using the pre-header hijack approach. Once the socket is taken over by my application, I complete the handshake myself and have a full-duplex connection ( which works very nicely with faye/websocket-protocol-ruby )

@digitalextremist
Copy link
Member

As mentioned, I am using your branch in my development environment --

At times it seems requests for static files are not closing. Trying to isolate //

@halorgium
Copy link
Contributor

@digitalextremist probably best to file a new issue, but static files should not be hijacked... are you using a middleware like Rack::File?

EDIT: should not

@digitalextremist
Copy link
Member

No middleware that might affect static files. Brought this up here because it might also extend to ordinary Requests not coming in through ws:// ...I've been seeing it enough to where I thought you might want to surf around in your branch in a browser that loads images, css, js, etc.

@tarcieri
Copy link
Member Author

@halorgium closing this for now. I'm changing the internal hijacking API in #77. I think we need to redo the Rack adapter completely before we consider supporting Rack's hijack

@tarcieri tarcieri closed this Aug 11, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants