Skip to content
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

Can't create multiple client-side connections to the same server-channel #1

Open
kdvolder opened this issue Nov 9, 2012 · 6 comments

Comments

@kdvolder
Copy link

kdvolder commented Nov 9, 2012

I was trying to fixup some code that uses multiple SockJS instances to use WebSocketMultiplex instead.

But it seems that there is an built-in assumption that I can not open more than one simultaneous channel with the same name.

I.e.

   var newsChannelForBob = multiplexer.channel('news');
   var newsChannelForAnn = multiplexer.channel('news');

The second call overwrites the first call's instance in 'channels' registry. This leads to problems down the line.

I think that when I write similar code using 'SockJS' directly this works fine and I simply get two independent socket instances each openening a independent websockets connection to the server, connecting to the same websocket url.

If I am correct (not really 100% sure, as not an expert on websockets) then WebSocketMultiplex isn't really what it purports to be. I.e. a transparant replacement for directly using SockJS.

Is this a problem with WebSocketMultiplex? Or is it rather a bug in my understanding / use of the websocket API?

Is there a way to open multiple independent client side channels to the same server-side 'service'?

I would rather avoid implementing my own multiplexer if possible :-)

@majek
Copy link
Member

majek commented Nov 10, 2012

Interesting! Well, the idea behind websocket-multiplex is that you can have more than one emulated 'websocket' connection over a single real/sockjs 'websocket' connection. But emulated 'websocket' still behaves like a websocket - it is one-to-one connection.

That means registering channel twice make no sense - names are only used for convenience do tell one emulated ws connection from another in the protocol layer.

If you want to do pub-sub (which is what I see you're trying to do), register a single 'news' channel, and create your own abstraction that can distribute single event to multiple interested parties. Use EventEmitter pattern from node.js, or something alike.

Hope that helps, and thanks for using websocket-multiplex!

@kdvolder
Copy link
Author

My use of 'news' was misleading. I am not after a 'pub-sub' but more of a 'dialogue' between a client and service.

  • client contacts service called 'search'
  • client sends some initial request parameters (e.g. search for 'foo')
  • service starts processing and sends results back as it discovers them
  • client can make changes to original request on the fly (while processing is still going on) e.g 'foob'
  • service re-adjust and sends an update as to how this changes the already sent results (some results
    already sent are invalidated, search process in progress is notified to change the search term for the
    remaining work)

This goes on as long as the client keeps the connection open.

In theory it should be possible to start multiple searches simultaneously from the same client (though in practice this may be rare :-)

I.e. each client side connection initiates a distinct conversation with the server side service. Eachserver-client connection has its own state.

I'm not certain whether this is supposed to be possible with 'WebSockets' in general but it does work when I use plain SockJS and just instantiate multiple client side SockJS instances to connect to the same URL on the server.

And it doesn't work when I use multiplexed connections.

It seems to me that the 'name' of the service is not the right thing to use to identify a connection. Instead, the connection should be some unique id generated on the client side when a channel is created. The name should only serve to identify the service one tries to connect to, not to identify the connection itself.

registering channel twice make no sense

You are probably right. But I'm not trying to register a 'channel'. I'm trying to open a connection to service. Each connection attempt starts an independent two way conversation between the service and the client.

Hope that helps, and thanks for using websocket-multiplex!

Well... actually, I won't be able to use it unless I figure out some way to make it work the way I want
But thanks for taking the time to respond and for making websocket-multiplex in the first place... Maybe I can figure out how to patch it to make it behave the way I want... but its tricky... it it wasn't I would have already implemented my own instead of coming here :-)

@majek
Copy link
Member

majek commented Nov 10, 2012

"name" of the channel must be unique, this is a design decision of websocket-multiplex, and as you describe it now I see that it's probably wrong.

But, please note that websocket-multiplex was always intended as a sketch library that shows one of the approaches of doing multiplexing over sockjs (the simplest approach). There are tons of more complex cases it doesn't cover. It's 100% fine for you to fork the code and modify it to your needs. The code is short and understandable so that should be quite easy.

Thanks for this comment, I think at some point I should revisit websocket-multiplex and make it more like websockets (ie: with channel name identifying the endpoint, rather than particular connection).

@kdvolder
Copy link
Author

But, please note that websocket-multiplex was always intended as a sketch library

Duely noted. Despite your 'disclaimer' it is still the best 'socket multiplexer' thing I found so far :-)

I might fork and patch if I really need this fixed. For now, I'm probably good just creating multiple SockJs instances (but that could become a problem if we need more services).

@epferrari
Copy link

This is an ancient issue, but for anyone who's finds themselves here it'd be nice to find a solution. So use a constraint in your request payload to describe the nature of your request to the service, and the same key on the response payload. Written using ES6 syntax:

/* server */

// require stuff, etc.

var searchChannel = mplexer.registerChannel("/search");
searchChannel.on("connection", conn => {
    conn.on("data", req => {
       var {api,id} = req;
       delete req.id;
       delete req.api;
       // assuming you have some apis written here
       service.get("api/"+api)
           .then( res => conn.write(JSON.stringify(merge({},res,{id: id}))) );
    });
});


/* client - Search.js */

import merge from 'object-assign';
import multiplex from './path/to/client/multiplex.js';

var searches = {};
var searchChannel  = multiplex.channel("/search");
searchChannel.on("message",msg => {
    let data = msg.data, id = data.id;
    delete data.id;
    searches[id] && searches[id].emit("response",data);
});

var s = 0;

function Search(api,query){
    searchStreams[s] = this;
    this.id = s;
    this.listeners = {};
    this.api = api;
    this.query = query;

    s++;
}
Search.prototype = {
    find(constraints){
        searchChannel.send(merge({},(constraints || this.query),{id: this.id,api: this.api});
    },
    emit(eventType,payload){
       let eventHandler = "on" + eventType;
       this[eventHandler] && this[eventHandler](payload);
       (this.listeners[eventType] || []).forEach( listener => listener(payload) );
    },
    on(eventType,handler){
       (this.listeners[eventType] || (this.listeners[eventType] = [])).push(handler);
    }
};

export default Search;


/* client  - app.js */

import Search from './path/to/Search.js'

var search1 = new Search("people");

search1.onresponse = function(res){
     console.log(res);
};
search1.on("response",function(res){
    console.log(res[0].name);
});

// with default constraints, say you have a query you're going to be repeating
var search2 = new Search("pets",{legs: 4, color: "brown"});
search2.onresponse = function(res){
    console.log(res[0]);
};


search1.find({gender: "female", status:["single","separated"], smoker: 0});
....
/*
 logs 
   {name: "Becka Spradlin", dist: 2.7, imgUrl: "yourserver.com/images/8jn83.jpg"}),
   {name: "Nell Donahue", dist: 4.3, imgUrl: "yourserver.com/images/ajh7j0sj.jpg"}
   ... and so on
*/
// logs "Becka Spradlin"

search2.find();
...
// logs {name: "Alfonse", species: "canine", breed: "labradoodle"}

@mmm8955405
Copy link

I think the registration channel is a very stupid design! Are you kidding? Every time the client opens a channel, it needs to register a namespace on the server side corresponding to the channel change? What if some clients open one and some clients open more than one? Don't open source if you can't, like a joke

报错

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants