Ws: Does ws work with express-status-monitor

[yossisp]

• [x] I've searched for any related issues and avoided creating a duplicate
  issue.

Description

When using ws with express and adding express-status-monitor npm package as middleware, the server crashes with the following error:

node_modules/ws/lib/websocket.js:837
  websocket.readyState = WebSocket.CLOSING;
                       ^

TypeError: Cannot set property 'readyState' of undefined
    at Socket.socketOnClose (node_modules/ws/lib/websocket.js:837:24)
    at Socket.emit (events.js:194:15)
    at Socket.EventEmitter.emit (domain.js:441:20)
    at TCP._handle.close (net.js:600:12)
npm ERR! code ELIFECYCLE

Reproducible in:

• version: ^7.2.0
• Node.js version(s): 10.15.1
• OS version(s): macOS mojave

Steps to reproduce:

import statusMonitor from 'express-status-monitor'
import debug from 'debug'
import express from 'express'
import WebSocket from 'isomorphic-ws'
import { createServer } from 'http'

const app = express()
app.use(statusMonitor())
  const server = createServer(app)
  const wsServer = new WebSocket.Server({
    server,
    perMessageDeflate: false,
    clientTracking: true
  })

  wsServer.on(events.connection, function (wsClient) {})
server.listen(constants.server.port, function () {
    log(`Express server listening on http://localhost:${constants.server.port}`)
  })

Expected result:

server doesn't crash

Actual result:

server crashes

Attachments:

[lpinca]

express-status-monitor uses socket.io which uses ws, so what probably happens here is that two ws servers are created that share the same HTTP server. Two 'upgrade' listeners are added so two 'close' listeners are added to the same underlying net.Socket. As a result socketOnClose() is called twice leading to this error.

The underlying issue is the same of https://github.com/websockets/ws/issues/1660. It's invalid usage. There must be a 1:1 correspondence between a WebSocket and a net.Socket.

[yossisp]

Thank you for the quick response! Do you know any monitoring tools that could be used with ws?

[lpinca]

No. it depends on the project but a proper APM like Elastic APM https://www.elastic.co/guide/en/apm/agent/nodejs/current/express.html might be better.

In the example above is there any reason to create your own WebSocket server? As far as I can see, it is possible to create a socket.io server externally and pass it to express-status-monitor so you could tell socket.io to use only websocket for example but yes this is not the same of using ws.

[yossisp]

I need to monitor the health of websocket server so it's handy to attach health route to that server in order to monitor it and restart if needed. That's why I need websocket to use express server.

[lpinca]

Closing as answered. Discussion can continue if needed.