Reconnect option?

[binarykitchen]

I do not have much background info on this one problem but I think the websocket-stream emits the end event after a certain timeout of inactivity.

Is this a well-known case? Can it reconnect when needed, i.E. with an option? Or better, on a .write() call?

[AdrianRossouw]

apparently you need to send a ping frame every now and then, to keep things running

http://stackoverflow.com/questions/10585355/sending-websocket-ping-pong-frame-from-browser

[binarykitchen]

fair enough - i ll have to solve this on the app side then

[AdrianRossouw]

I just realized I was having related issues, and was just checking if there was an existing ticket.

The ws library that this websocket-stream depends on has a ws.ping method, so i'll try that.

This seems to be the relevant part of the spec: https://html.spec.whatwg.org/multipage/network.html#ping-and-pong-frames

edit: Socket.io (or rather Engine.io) has a built-in pingInterval, which is why I never knew about this.

[mcollina]

I think this might be a good feature to add here, so it's solved for
everybody. I wrote http://npm.im/retimer for similar purposes, and it does
not have any impact on scalability.

Max, Mathias, what do you think?
Il giorno mar 14 lug 2015 alle 00:45 Adrian Rossouw <
[email protected]> ha scritto:

I just realized I was having related issues, and was just checking if
there was an existing ticket.

The ws library that this websocket-stream depends on has a ws.ping
https://github.com/websockets/ws/blob/master/doc/ws.md#websocketpingdata-options-dontfailwhenclosed
method, so i'll try that.

This seems to be the relevant part of the spec:
https://html.spec.whatwg.org/multipage/network.html#ping-and-pong-frames

—
Reply to this email directly or view it on GitHub
#69 (comment)
.

[mafintosh]

since ping is part of the spec i'd merge a PR that adds pinging to keep the socket alive

[mafintosh]

@mcollina wouldn't a simple setInterval be fine here?

[mcollina]

Probably :). But it would waste bandwidth (sending a ping when it's not
needed). As a server component with thousands of connections per process
that makes a difference.
I should also bench retimer vs setInterval, to make sure they perform the
same.

On second thought, the best approach here is to expose the internal ping()
method on the stream, and then have configurable setInterval thing (so you
can disable it for servers, if you want/need).
Il giorno mar 14 lug 2015 alle 08:39 Mathias Buus [email protected]
ha scritto:

@mcollina https://github.com/mcollina wouldn't a simple setInterval be
fine here?

—
Reply to this email directly or view it on GitHub
#69 (comment)
.

[mafintosh]

@mcollina you could skip the ping if a write was performed since the last time you pinged and still use setInterval (at max you'd ping/send data every 2x interval)

[mcollina]

@mafintosh yeah :D. My usecase is probably a little too specific, (in MQTT doing 2x is invalid)

What's the interval for Websockets, @binarykitchen?

[AdrianRossouw]

engine.io has a 25000 default.

[mcollina]

@binarykitchen @AdrianRossouw would you like to assemble a PR with that 25000 value as a default?

[binarykitchen]

@mcollina after ~60 seconds of inactivity the end event is emitted

... you asking me to do a PR using your retimer()? everyone here happy about this?

[AdrianRossouw]

I was actually trying to write a test that proves that it times out first, so that we know the setInterval and ping/pong resolves it. Maybe its something that only comes to place when an actual browser is involved, and the test will need to be something zuul-selenium related?

Also kind of sucks having a 1 minute timeout in a test, so I was seeing if something like time-mock or sinonjs would be able to help.

I was also spending some time learning how the code in WS works around this stuff.

[mafintosh]

Just make the timeout configurable - then the test can just use a 100ms timeout and there is no need to mock

[mcollina]

@binarykitchen the setInterval thing that @mafintosh pointed out was good.

@AdrianRossouw having a unit test is awesome. A simple solution to test this is to accept a pingInterval option, and lower it down to 50ms or something. That should not take too much time to run in the tests. Most time mocking library have bad side effects that make the tests brittle.

[AdrianRossouw]

@binarykitchen can you tell me more about which browser you are using?

I haven't been able to reproduce the timeout when running with webstream-socket running on node on both sides. The reason I'm asking is because I'm wondering whether the ping interval needs to go on the server or on the client side.

[binarykitchen]

@AdrianRossouw FF v39

[mcollina]

@AdrianRossouw I think this should go in the client side, and probably only on browser.

[AdrianRossouw]

Yeah, so i don't think this is really possible in a standards compliant way (stackoverflow reference)

The ping/pong methods have not been exposed on the browser's WebSocket implementations, which is what WS falls back to.

You can add your own higher level ping/pong messages, which is how engine.io does it, but that feels a bit dirty. I'll think about it a bit more before I hack up a solution.

[mcollina]

Basically, the reason why I never experienced this is because I use MQTT.js with its own keepalive mechanism. Awesome :).

So, it seems this is not really fixable in this module.

[binarykitchen]

So, a reconnect won't be implemented and @mcollina, are you suggesting I should use MQTT.js instead?

[binarykitchen]

Or would you really accept if I do a good PR?

[mcollina]

I investigate this a bit more:

1. we have no way of sending pings from the browser
2. we can send pings from the server, and seems like a good practice to do so
3. ws.ping() works only in the server

I think we can accept a PR that exposes the ping method from ws - this might not be needed, but it is nice.
Then, for any wsstream your server receives, you can do:

setInterval(function () {
  wsstream.ping()
}, 10000)

Remember to clear up the interval when the stream ends.

I think you can wrap all this into a nice module 'websocket-stream-pingpong', which we can link from here, if anybody else needs it.

[binarykitchen]

Alright, I ll think about it when I have time ...

[binarykitchen]

Okay, I just started to write a module but somehow fail to simulate such a timeout in my tests. Why isn't it occuring? Here is the current, experimental test code

var http            = require('http'),
    test            = require('tape'),
    websocketStream = require('websocket-stream'),
    WebSocketServer = require('ws').Server,

test('get disconnected', function(t) {

  var server  = http.createServer(),
      wss     = new WebSocketServer({server: server})

  server.listen(8344, function() {
    var clientStream = websocketStream('ws://localhost:8344')

    wss.on('connection', function(ws) {
      var serverStream = websocketStream(ws)

      setTimeout(function() {
        console.log('server wants to play ...')
        serverStream.write('server wants to play')
      }, 2 * 60 * 1000) // usually, websockets time out after 60 seconds

      setTimeout(function() {
        console.log('client wants to play ...')
        clientStream.write('client wants to play')
      }, 3 * 60 * 1000) // usually, websockets time out after 60 seconds

      // this never happens but why??
      serverStream.on('error', function(err) {
        t.equal(err.toString(), 'Error: write after end', 'server stream timed out')
      })

      // this never happens but why??
      clientStream.on('error', function(err) {
        t.equal(err.toString(), 'Error: write after end', 'client stream timed out')
        clientStream.destroy()
        serverStream.destroy()
        t.end()
      })
    })
  })
})

[mcollina]

@binarykitchen the problem is that's a browser thing, not a websocket one. The browser kills the ws connection if it's inactive (probably for good reasons).

[binarykitchen]

@mcollina Right, the browser. How would you write such a test case then??

[AdrianRossouw]

probably using something like zuul and sauce labs.

[binarykitchen]

not sauce labs. must be testable on localhost from command line without the need for an internet connection.

[AdrianRossouw]

zuul is a test runner that will allow you to run your mocha etc. tests in browser locally, as well as being able to use sauce labs when run automatically from travis.

locally it can run via a browser you open, or via phantomjs.

[binarykitchen]

does phantomjs kill the ws connection if it's inactive?

[mcollina]

Folks, to run tests in the browser: https://github.com/maxogden/websocket-stream#run-the-tests

[binarykitchen]

@mcollina i see, could do that - but isn't there an easier solution to automate this all in the console?

[binarykitchen]

@mcollina I fail to reproduce the timeout in a test environment with Beefy.

Check out this unfinished package:
https://github.com/binarykitchen/websocket-stream-ping-pong

Run npm install && npm test and see, the test fails.

There is no timeout between Beefy and the server. Why?

[binarykitchen]

Maybe Beefy does some pings? chrisdickinson/beefy#93

[binarykitchen]

@mcollina ping? tested https://github.com/binarykitchen/websocket-stream-ping-pong yet?

[mcollina]

Unfortunately I have little time till mid-September (after nodeconf.eu)

[binarykitchen]

okay, i ll wait until end of september then - really need your help and advice by then. grazie!

[mcollina]

Your test is failing on my machine - no error is emitted. I'm not following what that code should prove.

[binarykitchen]

@mcollina exactly. after 60 seconds there should be a timeout. hence writing data from client to server after 70 seconds should emit an error here https://github.com/binarykitchen/websocket-stream-ping-pong/blob/master/test_client.js#L16

but this isn't happening. test fail because errString is undefined.

why?

[mcollina]

I presume the timeout is much higher. You should probably dig into the websocket spec and/or a browser source code to find out.

[binarykitchen]

I already did and found nothing useful. Most sources say the timeout is 60 seconds.

Maybe Beefy already does something with websockets and is messing with the test case?

[mcollina]

Try without beefy.

[binarykitchen]

How do you suggest to run the tests in browser + server then?

[mcollina]

browserify directly and serve using some static module.
We don't really need "tests", we just need to have an on/off switch.

[binarykitchen]

Just did that. No luck, but I guess it is an nginx issue:
http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_read_timeout

Figuring out how to set up a test environment for that.