Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ruspaul013 opened this issue Oct 24, 2023 · 4 comments
Open

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

ruspaul013 opened this issue Oct 24, 2023 · 4 comments
Assignees
Labels
z-documentation-update-needed This type of issues likely have a link to an issue in the Unit Docs repo

Comments

@ruspaul013
Copy link

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
Copy link
Contributor

tippexs commented Nov 24, 2023

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
Copy link
Contributor

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
Copy link
Contributor

tippexs commented Dec 6, 2023

@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.

@tippexs tippexs added the z-documentation-update-needed This type of issues likely have a link to an issue in the Unit Docs repo label Dec 6, 2023
@tippexs tippexs self-assigned this Dec 6, 2023
@ruspaul013
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
z-documentation-update-needed This type of issues likely have a link to an issue in the Unit Docs repo
Projects
None yet
Development

No branches or pull requests

3 participants