websockets / ws

Simple to use, blazing fast and thoroughly tested WebSocket client and server for Node.js
MIT License
21.65k stars 2.42k forks source link

ws>=5.1.0 breaks autobahn.js #1358

Closed vdiez closed 6 years ago

vdiez commented 6 years ago

As I've reported on crossbario/autobahn-js#347. Autobahn.js is not able to connect to Crossbar.io server when using a ws version > 5.0.0

Reproducible in:

version: 5.1.0, 5.1.1 Node.js version(s): 8.7.0 OS version(s): Ubuntu 16.04

Steps to reproduce:

> let autobahn = require('autobahn')
> let wamp = new autobahn.Connection({url:'ws://10.67.19.64:8080/ws', realm:'realm1', max_retries: 0});
> wamp.onopen = ses => {session = ses; console.log("connected")}
> wamp.onclose = (reason, details) => console.log(reason, details);
> wamp.open()

Expected result:

connected

Actual result:


> unreachable { reason: null,
  message: null,
  retry_delay: null,
  retry_count: null,
  will_retry: false }
unreachable { reason: null,
  message: null,
  retry_delay: null,
  retry_count: null,
  will_retry: false }

Thank you

lpinca commented 6 years ago

Can you please create a reproducible test case? This only shows the client part. The subprotocols seem to be correctly handled by autobahn as per https://github.com/crossbario/autobahn-js/blob/c3a800a359838fbbc1b47af55cd122fa01dd0de4/lib/transport/websocket.js#L94-L100

vdiez commented 6 years ago

Hello,

I've tried against crossbar.io and wamp.rt, both behaving equally. As a test for wamp.rt (easier to reproduce):

user@host:~$ mkdir ws_test
user@host:~$ cd ws_test/
user@host:~/ws_test$ npm install wamp.rt
user@host:~/ws_test$ node

> let wamp_router = require("wamp.rt");
> new wamp_router({port: 8080});

Now try the initial post lines using 'ws://127.0.0.1:8080' as url. With 5.0.0 you will see "connected" and with 5.1.[0-1] you get an unreachable error.

Thanks

lpinca commented 6 years ago

What version of autobahn are you using on the client? I think this commit https://github.com/crossbario/autobahn-js/commit/79b444ca10f8323c3abe7e3cfd88eb52e5b42053 fixes the issue.

vdiez commented 6 years ago

I'm using 17.5.2. Let me try latest

vdiez commented 6 years ago

Correct, using last autobahn version fixes the issue, sorry I assumed I was using it already. Thanks for your help!

lpinca commented 6 years ago

For posterity: I accidentally removed options.protocol in https://github.com/websockets/ws/commit/9e152f920a1818d8a23e0a113b4625c83f90d30d#diff-13a0e23b4df05bd40179ab73312bf77bR486 and autobahn was using only that before https://github.com/crossbario/autobahn-js/commit/79b444ca10f8323c3abe7e3cfd88eb52e5b42053.