Merge pull request #50 from ssb-js/upgrade-ws

Upgrade pull-ws

Author: arj03

package.json

@@ -12,7 +12,7 @@
     "multicb": "^1.2.2",
     "multiserver-scopes": "^1.0.0",
     "pull-stream": "^3.6.1",
-    "pull-ws": "^3.3.0",
+    "pull-websocket": "^3.4.0",
     "secret-handshake": "^1.1.16",
     "separator-escape": "0.0.1",
     "socks": "^2.2.3",

plugins/ws.js

@@ -1,11 +1,11 @@
-var WS = require('pull-ws')
+var WS = require('pull-websocket')
 var URL = require('url')
 var pull = require('pull-stream/pull')
 var Map = require('pull-stream/throughs/map')
 var scopes = require('multiserver-scopes')
 var http = require('http')
-var https      = require('https')
-var fs         = require('fs')
+var https = require('https')
+var fs = require('fs')
 var debug = require('debug')('multiserver:ws')
 
 function safe_origin (origin, address, port) {
@@ -51,9 +51,7 @@ module.exports = function (opts = {}) {
     name: 'ws',
     scope: () => scope,
     server: function (onConnect, startedCb) {
-      if (WS.createServer == null) { 
-        return null
-      }
+      if (WS.createServer == null) return null
 
       // Maybe weird: this sets a random port each time that `server()` is run
       // whereas the net plugin sets the port when the outer function is run.
@@ -68,9 +66,10 @@ module.exports = function (opts = {}) {
         opts.cert = fs.readFileSync(opts.cert)
 
       var server = opts.server ||
-        (opts.key && opts.cert ? https.createServer({ key: opts.key, cert: opts.cert }, opts.handler) : http.createServer(opts.handler))
+          (opts.key && opts.cert ? https.createServer({ key: opts.key, cert: opts.cert }, opts.handler) : http.createServer(opts.handler))
 
-      WS.createServer(Object.assign({}, opts, {server: server}), function (stream) {
+      const serverOpts = Object.assign({}, opts, {server: server})
+      let wsServer = WS.createServer(serverOpts, function (stream) {
         stream.address = safe_origin(
           stream.headers.origin,
           stream.remoteAddress,
@@ -90,7 +89,8 @@ module.exports = function (opts = {}) {
 
       return function (cb) {
         debug('Closing server on %s:%d', opts.host, opts.port)
-        server.close(function(err) {
+        wsServer.close((err) => {
+          debug('after WS close', err)
           if (err) console.error(err)
           else debug('No longer listening on %s:%d', opts.host, opts.port)
           if (cb) cb(err)
@@ -117,7 +117,7 @@ module.exports = function (opts = {}) {
       stream.address = addr
 
       return function () {
-        stream.close(cb)
+        stream.close()
       }
     },
     stringify: function (targetScope = 'device') {
@@ -157,7 +157,3 @@ module.exports = function (opts = {}) {
     }
   }
 }
-
-
-
-

test/plugs.js

@@ -356,85 +356,107 @@ tape('onion plug', function (t) {
   t.end()
 })
 
-tape('id of stream from server', function (t) {
-  check = function (id, cb) {
-    cb(null, true)
-  }
-  var close = combined.server(function (stream) {
-    var addr = combined.parse(stream.address)
-    t.ok(addr)
-    //console.log('address as seen on server', addr)
-    t.equal(addr[0].name, 'net')
-    t.deepEqual(addr[1], combined.parse(combined.stringify())[1])
-
-    pull(stream.source, stream.sink) //echo
-  }, function (err) {
-    if(err) throw err
-  }, function () {
-
-    combined.client(combined.stringify(), function (err, stream) {
-      if(err) throw err
+function testServerId(combined, name, port) {
+  tape('id of stream from server', function (t) {
+    check = function (id, cb) {
+      cb(null, true)
+    }
+    var close = combined.server(function (stream) {
+      console.log('raw address on server:', stream.address)
       var addr = combined.parse(stream.address)
-      t.equal(addr[0].name, 'net')
-      t.equal(addr[0].port, 4848)
+      t.ok(addr)
+      console.log('address as seen on server', addr)
+      t.equal((addr[0].name || addr[0].protocol).replace(':', ''), name)
       t.deepEqual(addr[1], combined.parse(combined.stringify())[1])
-      stream.source(true, function () {
-        close(t.end)
+
+      pull(stream.source, stream.sink) //echo
+    }, function (err) {
+      if(err) throw err
+    }, function () {
+
+      combined.client(combined.stringify(), function (err, stream) {
+        if(err) throw err
+        var addr = combined.parse(stream.address)
+        t.equal((addr[0].name || addr[0].protocol).replace(':', ''), name)
+        if (addr[0].protocol === 'ws:')
+          t.equal(+addr[0].port, 4849)
+        else
+          t.equal(+addr[0].port, 4848)
+        t.deepEqual(addr[1], combined.parse(combined.stringify())[1])
+        stream.source(true, function () {
+          close(t.end)
+        })
       })
     })
   })
-})
+
+}
+
+testServerId(combined, 'net')
+testServerId(combined_ws, 'ws')
+
+
+
 
 function testAbort (name, combined) {
   tape(name+', aborted', function (t) {
     var close = combined.server(function onConnection() {
       throw new Error('should never happen')
-    }, null, () => {
-      var abort = combined.client(combined.stringify(), function (err, stream) {
-        t.ok(err, 'the error is expected')
-
-        // NOTE: without the timeout, we try to close the server
-        // before it actually started listening, which fails and then
-        // the server keeps runnung, causing the next test to fail with EADDRINUSE
-        //
-        // This is messy, combined.server should be a proper async call
-        setTimeout( function() {
-          //console.log('Calling close')
-          close(t.end)
-        }, 500)
-      })
+    })
 
-      abort()
+    var abort = combined.client(combined.stringify(), function (err, stream) {
+      t.ok(err)
+      console.error("CLIENT ABORTED", err)
+      // NOTE: without the timeout, we try to close the server
+      // before it actually started listening, which fails and then
+      // the server keeps runnung, causing the next test to fail with EADDRINUSE
+      //
+      // This is messy, combined.server should be a proper async call
+      setTimeout( function() {
+        console.log('Calling close')
+        close(t.end)
+      }, 500)
     })
+
+    abort()
   })
 }
 
 testAbort('combined', combined)
 testAbort('combined.ws', combined_ws)
 
-tape('error should have client address on it', function (t) {
-  //  return t.end()
-  check = function (id, cb) {
-    throw new Error('should never happen')
-  }
-  var close = combined.server(function (stream) {
-    throw new Error('should never happen')
-  }, function (err) {
-    t.ok(/^net:/.test(err.address))
-    t.ok(/~shs:/.test(err.address))
-    //the shs address won't actually parse, because it doesn't have the key in it
-    //because the key is not known in a wrong number.
-  }, function () {
-    //very unlikely this is the address, which will give a wrong number at the server.
-    var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX')
-    combined.client(addr, function (err, stream) {
-      //client should see client auth rejected
-      t.ok(err)
-      //console.log('Calling close')
-      close(t.end)
+function testErrorAddress(combined, type) {
+  tape('error should have client address on it:' + type, function (t) {
+    check = function (id, cb) {
+      throw new Error('should never happen')
+    }
+    var close = combined.server(function (stream) {
+      throw new Error('should never happen')
+    }, function (err) {
+      var addr = err.address
+      t.ok(err.address.indexOf(type) == 0) //net or ws
+      t.ok(/\~shs\:/.test(err.address))
+      //the shs address won't actually parse, because it doesn't have the key in it
+      //because the key is not known in a wrong number.
+    }, function () {
+      //very unlikely this is the address, which will give a wrong number at the server.
+      var addr = combined.stringify().replace(/shs:......../, 'shs:XXXXXXXX')
+      combined.client(addr, function (err, stream) {
+        //client should see client auth rejected
+        t.ok(err)
+        close(() => {
+          if (type === 'ws') // we need to wait for the kill
+            setTimeout(t.end, 1100)
+          else
+            t.end()
+        })
+      })
     })
   })
-})
+}
+
+testErrorAddress(combined, 'net')
+testErrorAddress(combined_ws, 'ws')
 
 tape('multiple public different hosts', function(t) {
   var net1 = Net({ host: '127.0.0.1', port: 4854, scope: 'public'})
@@ -593,4 +615,4 @@ tape('ws: external is an array w/ multiple entries & shs transform', function (t
     'ws://domain.de:9966~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw=;ws://funtime.net:9966~shs:+y42DK+BGzqvU00EWMKiyj4fITskSm+Drxq1Dt2s3Yw='
   )
   t.end()
-})
+})