pass websockets through rack environment

[collin]

Made some changes so that a WebSocket request will go through the RackWorker.

Extracted some methods for remote addr/remote host and uri parts (path/query/fragment) to modules so the could be re-used on WebSocket requests.

Right now it just sticks the WebSocket instance in the rack env at env["async.connection"] Not really ideal, but then nothing about mixing websockets and rack together is ideal.

Also on the negative side is things will be weird if some middleware depend on [status, headers, body].

Having implemented it, I'm not 100% convinced it's a worthy idea. But when hacking around with things it can be nice to pretend Rack and WebSockets can live together in harmony.

[collin]

saw #16 and rebased against master

[tarcieri]

Sorry I haven't commented on this yet.

So this is... interesting. It'd be really neat if there were a standard for accommodating Websockets within Rack. Attempting to create one is likewise... interesting.

Not sure how I feel about this. Perhaps I should consult some outside opinions?

[ghost]

good implementation.

i have a similar one, started cause i was unattentive and missed this pull request. so just my two cents here:

• perhaps closing socket when app responds with a status code higher than 300?
• is not async.connection? too EM-ish? perhaps using rack.websocket instead?

[collin]

@tarcieri yeah, I hear you. If you can get others talking and opining about it please do. Getting the websocket/rack situation resolved is incredibly important.

I found the reel websocket API a breath of fresh air. But the node community is kicking ass here, not because they have the best API, but the have an API that's easy to use without the rack baggage.

@slivu I think I like both of those ideas.

[zacksiri]

IMO right now websocket just seems like a pain in the ass, I've been looking into SSE lately, and it seems to be much nicer and a better fit for rack, perhaps we should look into that direction?

[ghost]

@zacksiri, SSE are "supported out of the box" and has nothing to do with this pull request.

[ghost]

Tony, imo it is a good start point. I would encourage you to merge this, then we will can move forward with more Rack stuff. Thank you.

[zacksiri]

What kind of baggage does rack have? @collin

[shtirlic]

+1 to this maybe some git squash for commits

[collin]

@zacksiri you have to return the [status,headers,body] tuple even though http allows for streamed responses. Any API to provide a socket-like object in rack has to hack around that basic rack assumption.

@tarcieri @slivu you guys think I should squash this down?

[tarcieri]

@collin honestly I don't care. For a project like this I'd probably prefer not to squash

[collin]

@tarcieri :) I will leave it alone.

[ghost]

@collin, the important for me is to have it in master, no matter how :)

[tarcieri]

@collin @silvu Okay, how about this: I'm a fan of rack.websocket over async.connection. How about change that and we'll call it good? ;)

[collin]

@tarcieri @slivu :dancer:

[collin]

@shtirlic yes

[ghost]

@tarcieri,

"change that and we'll call it good? ;)"

by calling it good do you mean merge it? :)

[tarcieri]

@silvu confirm

[ghost]

@tarcieri, looks ok to me.

async stuff converted to rack.websocket, merged well with @shtirlic's updates and all specs passing.

please merge it to stop bothering you about this :)

thank you.

[tarcieri]

Cool, let's give this a shot