Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Added sockjs support #97

Merged
merged 5 commits into from

4 participants

@nelsonsilva

I've added sockjs support. The server side sockjs is practically equal to browserchannel's so we should probably extract most of it to another class.
In order to use sockjs on the server just pass sockjs: {} in the options.
For the client I kept browserchannel as default (since it's the default on the server as well). Basically it only uses sockjs when browserchannel is not available. You can try it by removing bcsocket.js and adding http://cdn.sockjs.org/sockjs-0.2.1.js to one of the examples.

@josephg
Owner

Nice work. I agree - we should pull all the server-side communication code out into its own file, and then we can plug sockjs / browserchannel / whatever in really easily. All those communication systems have almost the same API anyway.

Want to have a stab at that?

@josephg
Owner

Also, this needs tests. It should be pretty easy to run the browserchannel tests (test/browserchannel.coffee) against the sockjs interface.

@nelsonsilva

Thanks. I'll try to tackle it as soon as i have some free time. This will require looking at socketio to understand if there any major differences.
Regarding the tests i postponned them until the refactoring to reuse the code...

On a side note this is actually part of a bigger plan... get ShareJS working on vert.x... which i already managed.
The goal is to have a pristine ShareJS version working so i have custom versions of node, connect and sockjs-node all running in Rhino. The only changes i need to make to ShareJS are to not use coffeescript at runtime (point to ./lib instead of ./src and add prebuilt lib to cvs) and remove some requires (which should'nt even be there).
Regarding the ./lib vs ./src i was hopping you'd consider it along with a watch target for development.

@josephg
Owner

Don't worry about socketio for now. Just refactoring it to make it work for browserchannel & sockjs would be a good start. Socket.io's sessions try to last forever (using cookies and stuff). When I tried to use it before there were bugs where a client would reconnect (using an old sessionid) without the server being notified. You'd just start getting more messages from a client which was supposed to have been closed.

I had a watch target for compiling to /lib for awhile. Its just coffee -bcwo lib src.

@nelsonsilva

Joseph is this headed in the right direction ? Managed to extract practically all of it to a common session handling module.

@nelsonsilva

I've added tests for sockjs. I'm using a websocket client and basically copied from browserchannel tests.

@camshaft

I've been using this PR for awhile now and it works really well. This would be awesome merged in.

@wmertens wmertens commented on the diff
src/server/sockjs.coffee
((4 lines not shown))
+sessionHandler = require('./session').handler
+
+wrapSession = (conn) ->
+ conn.abort = -> @close()
+ conn.stop = -> @end()
+ conn.send = (response) -> @write JSON.stringify(response)
+ conn.ready = -> @readyState is 1
+ conn.on 'data', (data) -> @emit 'message', JSON.parse(data)
+ conn.address = conn.remoteAddress
+ conn
+
+
+exports.attach = (server, createAgent, options) ->
+ sjsServer = sockjs.createServer options
+ sjsServer.on 'connection', (conn) -> sessionHandler wrapSession(conn), createAgent
+ sjsServer.installHandlers server, {prefix: "/sockjs"}
@wmertens Collaborator
wmertens added a note

Shouldn't the prefix be configurable like for browserchannel? How about inserting options.prefix ?= "/sockjs" on line 17?

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

Tested this and it works nicely. Pulling, we'll do that prefix thing later.

@wmertens wmertens merged commit 109b0b9 into share:master
@wmertens
Collaborator

Thanks BTW :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 17, 2012
  1. @nelsonsilva

    Added sockjs support

    nelsonsilva authored
Commits on May 22, 2012
  1. @nelsonsilva
  2. @nelsonsilva
Commits on May 23, 2012
  1. @nelsonsilva

    Added sockjs tests

    nelsonsilva authored
  2. @nelsonsilva

    Formatting

    nelsonsilva authored
Something went wrong with that request. Please try again.