-
Notifications
You must be signed in to change notification settings - Fork 161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Strophe.js needs a roadmap for websockets #68
Comments
I am just helping to push development forward due to metajack's lack of time. Websocket support is highly interesting, but I regard it as just a further optimization over already-working BOSH. Please open pull requests. The superfeedr branch does not implement the IETF draft. Instead it uses Socket.IO which adds a few bits to Websocket messages. Socket.IO is not really needed; instead of the built-in fallback BOSH would probably a better fit in the absence of Websockets. While highly-compatible abstractions like Socket.IO and SockJS are lovely, an implementation on top an existing (draft) spec is more favourable. It'd be nice if you could respect that when preparing a patchset. Also, please make sure both transport implementations stay usable (ie. BOSH + Websocket). The superfeedr branch does that from what I glimpsed. |
Are there any known implementations based on the IETF-Draft? And what does |
@astro Are you still interested in pull requests that implement draft-moffitt-xmpp-over-websocket-00? If so, I may find some time to work on it this week. |
@loopingrage I'd love to see some progress in that topic! |
I wonder if anyone has built an XMPP/WebSockets connection manager like the ones that exist for BOSH. I know ejabberd has WS support but seems like a standalone connection manager may come in handy as well. I can look into that as well while I'm at it. |
@loopingrage https://github.com/dhruvbird/node-xmpp-bosh has a websocket server too. |
@loopingrage did you already implemented something? |
Looking through the history of the superfeedr branch, it was originally using websockets without Socket.IO. Look at: https://github.com/superfeedr/strophejs/commit/94a36ffa4098777980e4accab13ad5f507487148#src Perhaps a clean branch could be made using the work from the superfeedr branch up until Socket.IO was added? |
For the record, I've started implementing BOSH & Websockets in node-xmpp. Using browserify an application can be bundled with all libraries and both connection flavours already work in the browser! I'm testing with node-xmpp-bosh for the server side. End of shameless self-plug; the node-xmpp experiments are not release-ready so far. |
@astro Can your implementation be used with strophe? I guess it replaces the BOSH handling in strophe? I'm thinking about when websockets should be used. Should it automatically detect if the browser can support it and use it? Or should it always use BOSH unless configured to use websockets? |
@lboynton No, node-xmpp is an XMPP library of its own, mostly adhering to node.js conventions. A drop-in Strophe.js replacement on top of my code might be useful but it's not a sexy hack. Yes, ideally BOSH should be a fallback to missing Websockets support. |
Please take a look at my implementation of a protocolified strophe.js: It works so far but I think it could need some love :) |
Hi,I also created a fork that supports Websockets based on the latest master here: I think this is the only fork so far that is not based on superfeedrs branch (I did use some functions from websocket.js though). Unlike branches based on superfeedr this branch is API-compatible (except that attach doesn't work with WebSockets) and detects automatically which protocol should be used by looking at the given url. I tried to reuse as much code as possible so basically the structure of Strophe didn't change and most Strophe-functions now have functions of the same name that protocol objects have to implement. I tried to split my changes in enough commits to make clear what I changed exactly. If possible, I would like to have this merged upstream and send a pull request. |
I just glanced over it (didn't test it) and it seems pretty cool, although I'd like it more if those additional protocols would've been in an own file - this would make it easier to add other protocols like socket io etc. |
Yes, the protocols should have their own file, but splitting this up is easy, I kept them in one file for now because diffing code that was split up from multiple files is a pain. I will split this up if it's "finished" |
@Gordin I just tried your branch and ran into this error: Happens on line 2460 //Edit:
|
@mweibel Could you open an issue for this on my branch? (I don't want to spam this issue here) |
Sure, sorry ;) @Gordin: You'd need to enable Issues for that on your fork :) |
The bug @mweibel found has it's own issue here now and I think I fixed it. |
@nmaster the only "big" changes seem to be that stream start and end tags now must be sent in their own messages (Which was the only case all existing websocket branches supported anyway, I think) and that in case of a stream error during stream setup the complete stream including the error has to be sent together in one message. My branch can handles this now. (I guess, I don't now if any server actually sends the error like this) |
@Gordin: sounds good. |
Btw. I'm using my own branch (not @Gordin's) to test websockets and it will be go into production in some weeks. I'm not using Gordin's branch because being API compatible isn't actually my goal. When not being API compatible, implementors of the library can freely choose per connection what type of connection should be made (i.e. you could implement a fallback mechanism which automatically choses BOSH when Websockets fail after some attempts). |
@mweibel Why would you need to not be API compatible to choose between Websockets and BOSH? You can choose this freely per connection in my branch as well. When you open a Connection with a ws:// URL it uses Websockets and otherwise falls back to BOSH. Choosing this by giving the Connection another Objects doesn't add any information or flexibility, as what Protocol you want to use depends on the URL you connect to anyway. I also don't see why having a fallback mechanism would be any harder to implement in my branch. Also, my branch is used in ROLE (Responsive Open Learning Environments) since the last release and seems really stable. Actually I'm currently just commenting code and cleaning it up a bit and will try a pull request here when that's done. |
Hello everyone, I don't know if you already recognized that ProcessOne has published its implementation of Websockets using Strophe a few days ago: https://gist.github.com/processone/739147 What do you think about it? |
Correct me if I'm wrong, but there is no fallback to BOSH in ProcessOne's library, is there? |
@Letterus I did not get around to test this yet but I've read over it. Things I noticed:
Other than that it looks like they made changes similar to what I did on my branch. |
I also suggest to merge @Gordin branch as API compatibility is big deal for me. |
The discussion should move to https://github.com/strophe/strophejs/issues |
There is an existing websocket version of strophe.js (see https://github.com/superfeedr/strophejs/tree/protocol-ed/src, https://github.com/superfeedr/msgboy/tree/master/lib/strophejs and http://code.google.com/p/node-xmpp-bosh/) and the author of strophe.js has recently drafted xmpp-over-websockets (http://tools.ietf.org/html/draft-moffitt-xmpp-over-websocket-00).
Still, there is no mention of websockets at all on strophe.js project. Elaborating on your plans for websockets would increase the perceived viability of this project.
The text was updated successfully, but these errors were encountered: