nginx / unit

NGINX Unit - universal web app server - a lightweight and versatile open source server that simplifies the application stack by natively executing application code across eight different programming language runtimes.
https://unit.nginx.org
Apache License 2.0
5.29k stars 325 forks source link

Cannot read properties of undefined ( ws + nodejs ) #981

Open ruspaul013 opened 9 months ago

ruspaul013 commented 9 months ago

Hello! Is there any support for ws planned?

I'm trying to use unit with ws and node loader in a demo app, but I still getting this error:

/test/node_modules/ws/lib/websocket.js:231
    if (head.length > 0) socket.unshift(head);
             ^

TypeError: Cannot read properties of undefined (reading 'length')
    at WebSocket.setSocket (/test/node_modules/ws/lib/websocket.js:231:14)
    at WebSocketServer.completeUpgrade (/test/node_modules/ws/lib/websocket-server.js:411:8)
    at WebSocketServer.handleUpgrade (/test/node_modules/ws/lib/websocket-server.js:336:10)
    at Server.upgrade (/test/node_modules/ws/lib/websocket-server.js:112:16)
    at Server.emit (node:events:513:28)
    at Server.emit_request (/usr/lib/node_modules/unit-http/http_server.js:485:14)

Demo app

index_ws.js file:

const { WebSocketServer } = require('ws')
const sockserver = new WebSocketServer({ port: 3010 })
sockserver.on('connection', ws => {
         console.log('New client connected!')
         ws.send('connection established')
         ws.on('close', () => console.log('Client has disconnected!'))
         ws.on('message', data => {
                    sockserver.clients.forEach(client => {
                                 console.log(`distributing message: ${data}`)
                                 client.send(`${data}`)
                               })
                  })
         ws.onerror = function () {
                    console.log('websocket error')
                  }
})

Config file

Unit config file:

{
    "listeners": {
        "*:9096": {
            "pass": "routes"
        }
    },
    "routes": [
        {   "match": {
                "uri": [
                    "/ws1"
                ]
            },
            "action": {
                "rewrite": "/",
                "pass": "applications/node-test-ws-1"
            }
        }
    ],
    "applications": {
        "node-test-ws-1": {
            "user": "node",
            "group": "node",
            "type": "external",
            "working_directory": "/test/ws",
            "executable": "/usr/bin/node",
            "arguments": [
                "--loader",
                "unit-http/loader.mjs",
                "--require",
                "unit-http/loader",
                "index_ws.js"
            ]
        }
    }
}

Application with node it's working.

Environment

OS:

AlmaLinux release 8.8 (Sapphire Caracal)

Unit version:

unit version: 1.31.1
configured as ./configure --prefix=/usr/ --state=/var/lib/unit --control=unix:/var/run/control.unit.sock --pid=/var/run/unit.pid --log=/var/log/unit.log --tmp=/var/tmp --user=unit --group=unit --tests --openssl --modules=/usr/lib/unit/modules --libdir=/usr/lib/

unit-http build local:

./configure nodejs --node=/usr/bin/node --npm=/usr/bin/npm --node-gyp=/usr/bin/node-gyp
tippexs commented 8 months ago

Hi @ruspaul013 Thanks for reaching out and sorry for the long delay in my response. We had lately another Node related issue and this one felt of the radar. Will pick it up again.

You can have a look at the test application posted here https://github.com/nginx/unit/blob/master/test/node/loader/es_modules_websocket/app.mjs

This test makes use of the loader you are using as well. The loader configuration can be found here https://github.com/nginx/unit/blob/master/test/unit/applications/lang/node.py

Copied from our tests, can you give this a try?

import http from "http"
import websocket from "websocket"

let server = http.createServer(function() {});
let webSocketServer = websocket.server;

server.listen(8080, function() {});

var wsServer = new webSocketServer({
    maxReceivedMessageSize: 0x1000000000,
    maxReceivedFrameSize: 0x1000000000,
    fragmentOutgoingMessages: false,
    fragmentationThreshold: 0x1000000000,
    httpServer: server,
});

wsServer.on('request', function(request) {
    var connection = request.accept(null);

    connection.on('message', function(message) {
        if (message.type === 'utf8') {
            connection.send(message.utf8Data);
        } else if (message.type === 'binary') {
            connection.send(message.binaryData);
        }

  });

  connection.on('close', function(r) {});
});

However as you can see we are using different imports. Can you try to use

import http from "http"
import websocket from "websocket"

as well? We will have to play around with ws to see why and where we have issues with that.

andrey-zelenkov commented 8 months ago

Adding some info to the @tippexs reply. We do support only websockets for now. So you if you rewrite you demo app to something like:

server = require('http').createServer(function() {});
webSocketServer = require('websocket').server;

server.listen(3010, function() {});

var wsServer = new webSocketServer({
    maxReceivedMessageSize: 0x1000000000,
    maxReceivedFrameSize: 0x1000000000,
    fragmentOutgoingMessages: false,
    fragmentationThreshold: 0x1000000000,
    httpServer: server,
})

var clients = [];

wsServer.on('request', request => {
    var connection = request.accept(null);
    console.log('New client connected!');

    clients.push(connection);

    connection.send('connection established');
    connection.on('message', data => {
        clients.forEach(client => {
            console.log(`distributing message: ${data}`)

            if (data.type === 'utf8') {
                client.send(data.utf8Data);
            } else if (data.type === 'binary') {
                client.send(data.binaryData);
            }

        })
    })
    connection.onerror = function () {
        console.log('websocket error')
    }

    connection.on('close', () => console.log('Client has disconnected!'));
})

then it should work with our Node module.

tippexs commented 7 months ago

@andrey-zelenkov thanks for adding this. I think we can be more explicit about what websockets and ws is and what we support. A setense should be enough I guess.

ruspaul013 commented 7 months ago

Hello @tippexs @andrey-zelenkov !

I know that websocket is working ( I made a demo app using it ) but it's complicated to change from ws to websocket for what I need. I am interested if do you plan any support for ws in the future.