JSteunou / webstomp-client

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

Version 1.2.4 not working anymore on browsers #74

Open Polve opened 6 years ago

Polve commented 6 years ago

I upgraded my code to use the latest version of this library and while it still work fine under node, it stops working (it does not complete the connection) while running in the browser.

I think it has something to do with the way you handle the supported protocols but I've not yet been able to pinpoint exactly.

JSteunou commented 6 years ago

I had no issue upgrading to latest but if you can provide more info I would look into it

Polve commented 6 years ago

after some trial & error it seems like I'm able to use version 1.2.4 where I change this line:

headers['accept-version'] = getSupportedVersions(_this.ws.protocol);

with this one of a previous version:

headers['accept-version'] = VERSIONS.supportedVersions();

JSteunou commented 6 years ago

That's because you are not respecting the websocket protocols property. Look at this issue and associated fix #66

JSteunou commented 6 years ago

If you give a specific list of protocol at your websocket here https://github.com/JSteunou/webstomp-client/blob/master/src/webstomp.js#L12 or let the default list, your server must accept / handle at least one protocol in the list.

example from a real app:

the websocket request contains header Sec-WebSocket-Protocol: v10.stomp, v11.stomp, v12.stomp the response contains header sec-websocket-protocol: v12.stomp then the stomp CONNECT & CONNECTED frames contains accept-version:1.2 & version:1.2

Polve commented 6 years ago

I'm sorry, I'm not sure about where is the problem and what I've to do.

I've no control con the server, it's handled by https://docs.spring.io/spring-integration/reference/html/stomp.html and I suppose they follow specifications. I was using default options and everything worked fine until 1.2.0, and stopped working in newer releases so I suppose some default is not correct. That shouldn't happen, right?

Polve commented 6 years ago

So, to make it work, do I need to specify the exact protocol I want to use?

JSteunou commented 6 years ago

Could you show me how you initialize your webstomp client in JS and what are the headers you can see on the http request (the one upgraded as websocket) from your browser devtool for example.

Polve commented 6 years ago

I use a code like this:

const options = { debug: false, heartbeat: false } stompClient = Stomp.client(stompEndpoint, options)

I'll look for the headers

JSteunou commented 6 years ago

ok that means that the client will send the default list of supported STOMP protocols 'v10.stomp', 'v11.stomp', 'v12.stomp' but maybe your server did not apply to this list and send another version back or worse, no version at all

Polve commented 6 years ago

I see, but that's one of the most used implementations so it's very bad to not work with it.

We can open an issue there if you find that their implementation is against specs.

And I can provide you (privately) a STOMP endpoint if you want to make some tests yourself

JSteunou commented 6 years ago

I will let you open an issue at spring project if there is an issue on their side, I will not have the time to handle all possible server implementation. I follow the spec and handle this client, that's enough for the little time I have for side projects :)

You did not tell me yet what is the http response header you get from your server.

Polve commented 6 years ago

Here is the response, do you need anything else?

screenshot_08_202635

Polve commented 6 years ago

The connection works fine if I use those options:

const options = { debug: false, heartbeat: false, protocols: ['v12.stomp'] }

Polve commented 6 years ago

Debugging your code it seems like in this.ws.onpen() function _this.ws.protocol is empty, while it should not be.

Can this be a problem?

This is happening in node, using the "ws" npm package as a websocket provider

JSteunou commented 6 years ago

I will look into it when I will be back but this is weird. If you let default which is a list of v10.stomp, v11.stomp, v12.stomp, and get v10.stomp back then it should work and 1.0 should be used. On the contrary if you force v12.stomp and get v10.stomp back it should not work as they do not match.

Polve commented 6 years ago

The issue is still marked as "needs input", does it?

JSteunou commented 6 years ago

Yes. Could you give me a clear state of what works and what does not work (which options, which server, ...) and for each case what are the http response headers and ws.protocol value because it's a bit hard to follow you, information is spread out.

And look here https://github.com/JSteunou/webstomp-client/blob/master/dist/webstomp.js#L98 When there are no protocols matching, the accept-version header is a simple list of all 3 possible versions to be not breaking that's why I'm surprised with this issue

Polve commented 6 years ago

What I can tell you for sure is that using the Spring Framework toolkit as a STOMP server, this happens:

JSteunou commented 6 years ago

Ok but I need more information to find the issue, and unless you do not provide it I cannot understand if and where there is an issue from this lib

Polve commented 6 years ago

ok, please tell me the specifics of what you need, I already sent you the snapshot of the capture of the stream

JSteunou commented 6 years ago

I just did

Could you give me a clear state of what works and what does not work (which options, which server, ...) and for each case what are the http response headers and ws.protocol value because it's a bit hard to follow you, information is spread out.

pverheecke commented 6 years ago

Hi there, I'm running into the same issue as @Polve.

Basically what's happening is if the server responds with a Sec-Websocket-Protocol: v10.stomp header, it doesn't work. So including v10.stomp in the protocols list (whether by using the default, or by specifying explicitly) breaks it. Specifying only v11.stomp or only v12.stomp or the list v11.stomp, v12.stomp works fine.

JSteunou commented 6 years ago

That's weird because again they are all in the list of handle protocols, and I cannot reproduce this issue (when WS instance correctly created with a matched list of expected protocols).

Have you try to add some breakpoints in the webstomp client to debug this issue and find what happen?

JSteunou commented 5 years ago

You might be concerned by https://github.com/JSteunou/webstomp-client/issues/75#issuecomment-426578411