add hijack io support to streaming once spec is finalized #604

Open
rkh opened this Issue Jan 6, 2013 · 4 comments

Comments

Projects
None yet
5 participants
@rkh
Owner

rkh commented Jan 6, 2013

@digitalextremist

This comment has been minimized.

Show comment Hide comment
@digitalextremist

digitalextremist Apr 18, 2013

Seems like this rests on Stream being more generically evented, and using rack.hijack:

https://github.com/sinatra/sinatra/blob/master/lib/sinatra/base.rb#L425 needs to be scheduler agnostic, or at least not explicitly demanding EventMachine? Celluloid::IO for example, especially when Reel is the Rack handler, ought to be detected injected, or declared and used. Not everyone uses EventMachine for evented IO, as you know. Celluloid has a much better API and performance also.

Support for rack.hijack is being addressed on celluloid/reel#42. Right now I am using Sinatra on top of Reel. I will need to go in this direction and I'd rather it be a reusable solution that contributes toward this issue. Whatever comments you have here would be great.

Right now I have the browser returning a 101 status, switching protocols, and the socket is exposed through rack.hijack. I just need Sinatra to grasp it and hold it as a genuine websocket, and not just as a long-poll, otherwise the browser just keeps reloading, trying to settle into communicating bidirectionally across its websocket connection.

Seems like this rests on Stream being more generically evented, and using rack.hijack:

https://github.com/sinatra/sinatra/blob/master/lib/sinatra/base.rb#L425 needs to be scheduler agnostic, or at least not explicitly demanding EventMachine? Celluloid::IO for example, especially when Reel is the Rack handler, ought to be detected injected, or declared and used. Not everyone uses EventMachine for evented IO, as you know. Celluloid has a much better API and performance also.

Support for rack.hijack is being addressed on celluloid/reel#42. Right now I am using Sinatra on top of Reel. I will need to go in this direction and I'd rather it be a reusable solution that contributes toward this issue. Whatever comments you have here would be great.

Right now I have the browser returning a 101 status, switching protocols, and the socket is exposed through rack.hijack. I just need Sinatra to grasp it and hold it as a genuine websocket, and not just as a long-poll, otherwise the browser just keeps reloading, trying to settle into communicating bidirectionally across its websocket connection.

@rkh

This comment has been minimized.

Show comment Hide comment
@rkh

rkh Apr 18, 2013

Owner

No, moving to the hijack API, Sinatra will not have to deal with concurrency at all anymore. Main issue is that it will stop working with Thin and Rainbows. So what I'd actually like to do is add a shim middleware that will implement the hijacking API for these.

Owner

rkh commented Apr 18, 2013

No, moving to the hijack API, Sinatra will not have to deal with concurrency at all anymore. Main issue is that it will stop working with Thin and Rainbows. So what I'd actually like to do is add a shim middleware that will implement the hijacking API for these.

@maccman

This comment has been minimized.

Show comment Hide comment
@maccman

maccman Jun 27, 2013

Contributor

Looking forward to this!

Contributor

maccman commented Jun 27, 2013

Looking forward to this!

@b-dean

This comment has been minimized.

Show comment Hide comment
@b-dean

b-dean Sep 15, 2015

@rkh, whatever happened to this? Does the shim middleware exist somewhere that we can use it? on rack/rack#481 you mention that this will be in Sinatra 1.4, but that was a long time ago and I still don't see a way to use this with Sinatra's stream helper method (which is where I would expect it to show up).

I actually made sinatra/sinatra#1035 because I was having issues getting streams to work on puma. I'm guessing it's related to this (because puma just keeps closing my stream).

b-dean commented Sep 15, 2015

@rkh, whatever happened to this? Does the shim middleware exist somewhere that we can use it? on rack/rack#481 you mention that this will be in Sinatra 1.4, but that was a long time ago and I still don't see a way to use this with Sinatra's stream helper method (which is where I would expect it to show up).

I actually made sinatra/sinatra#1035 because I was having issues getting streams to work on puma. I'm guessing it's related to this (because puma just keeps closing my stream).

@zzak zzak modified the milestones: 2.0.0, future Jan 31, 2016

@zzak zzak modified the milestones: Beyond, 2.0.0 Aug 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment