JSteunou / webstomp-client

Stomp client over websocket for browsers
Apache License 2.0
299 stars 59 forks source link

webstomp.over() not working from a callback #5

Closed auchEnia closed 8 years ago

auchEnia commented 8 years ago

There is a problem when establishing connection over SockJS from a callback function. The original stomp client had the same issue.

The following code works

var ws = new SockJS(GLOBAL.apiUrl.replace('/v3', '') + '/notifications');
ws.onopen = function () {
    var client = webstomp.client('ws:/localhost/notifications');
    client.connect({}, function () {});
}

but if I replace webstomp.client(...); with webstomp.over(ws);, the client is stuck on "Opening Web Socket..." message.

Also this is not a scoping issue, because I can console.log the ws variable from within the callback. I have tried switching ws for this in the callback (as this in the context points to ws), but that does not work either.

JSteunou commented 8 years ago

I don't know what are your trying to achieve here. Why are you not just doing

var ws = new SockJS(GLOBAL.apiUrl.replace('/v3', '') + '/notifications');
var client = webstomp.over(ws);
client.connect(...);

Look at the SockJS broadcast example.

auchEnia commented 8 years ago

The problem is that the SockJS connection is not instant and sometimes it takes some time. If that happens and stomp tries to connect over that SockJS connection while it is still connecting, SockJS throws the following error: InvalidStateError: The connection has not been established yet. That is why we are waiting for the .onopen callback to make sure the readyState equals to SockJS.OPEN.

JSteunou commented 8 years ago

Ok that is the real issue to solve then. Even though using webstomp.over with an existing sockjs instance should work flawless. Le 11 avr. 2016 19:17, "Vladimir Zdenek" notifications@github.com a écrit :

The problem is that the SockJS connection is not instant and sometimes it takes some time. If that happens and stomp tries to connect over that SockJS connection while it is still connecting, SockJS throws the following error: InvalidStateError: The connection has not been established yet. That is why we are waiting for the .onopen callback to make sure the readyState equals to SockJS.OPEN.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/JSteunou/webstomp-client/issues/5#issuecomment-208455693

auchEnia commented 8 years ago

The example you posted works, but SockJS throws the error sometimes. When I try to establish stomp connection from within a function, it does not work at all. Either or, there is a bug somewhere.

It would probably help to do some sort of check to make sure this.ws.readyState equals to 1 and retry if not before calling this._transmit('CONNECT', headers);. That way I would not have to call webstomp.over(...) from within a function. But I am not sure. Just an idea.

JSteunou commented 8 years ago

Back to your 1st example, you cannot use webstomp-client like this, because the client itself listens for the onopen event in order to send the connect once the socket is open actually. That's why I found it weird the error you have with SockJS sometimes. I'm wondering if it's not a dedicated SockJS or a network issue.

But, you can use #over into a callback, DOM callback, event callback, Promise, whatever...

So I'm sorry but I have no other solution than https://github.com/JSteunou/webstomp-client/issues/5#issuecomment-208330614

auchEnia commented 8 years ago

I have just noticed something else. The callback itself does not seem to be the problem. It seems the stomp connection is not established if there is delay between creating the SockJS connection and the stomp connection.

The example below hangs on "Opening Web Socket...".

var ws = new SockJS(GLOBAL.apiUrl.replace('/v3', '') + '/notifications');
var client = Stomp.over(ws);

function connect() {
    if (ws.readyState !== SockJS.OPEN) {
        setTimeout(connect, 100);
        return;
    }
    client.connect({}, function () { ... });
}

In my opinion the stomp should be able to use SockJS connection at any time. Do you have any idea what could cause the stomp client not wanting to connect after a delay?

auchEnia commented 8 years ago

I probably know why. The connection is being established after ws.onopen() is called. If I delay client.connect() then it will miss the ws.onopen() therefore it never gets connected.

Closing this as it seems like it is an issue on SockJS side after all if it executes .onopen callback while the readyState is still 0.

JSteunou commented 8 years ago

As I said, client.connect use onopen event in order to connect only once the ws is open. Maybe binding on that event after it really took place does not replay it. I will check if it is only with SockJS or with browser WebSocket too.