websockets / ws

Simple to use, blazing fast and thoroughly tested WebSocket client and server for Node.js
MIT License
21.34k stars 2.3k forks source link

Incorrect/incomplete documentation #2223

Closed omnicron-llc closed 2 months ago

omnicron-llc commented 2 months ago

Is there an existing issue for this?

Description

A little embarrassing that the example code in the README.md is incorrect in multiple places - for example, the call back to wss.on('connection') only has 2 parameters, so the line wss.on('connection', function connection(ws, request, client) { is incorrect.

In the following 'upgrade' code block, there's also an undefined reference to client, which isn't even present in that block at all.

It would seem that at code samples should not at least include things like references to variables that do not exist, or completely invalid function prototypes.

ws version

No response

Node.js Version

No response

System

No response

Expected result

No response

Actual result

No response

Attachments

No response

lpinca commented 2 months ago

A little embarrassing that the example code in the README.md is incorrect in multiple places - for example, the call back to wss.on('connection') only has 2 parameters, so the line wss.on('connection', function connection(ws, request, client) { is incorrect.

See this line

wss.emit('connection', ws, request, client);

In the following 'upgrade' code block, there's also an undefined reference to client, which isn't even present in that block at all.

client is in the authenticate callback. It is not undefined. The authenticate function might look like this:

function authenticate(request, callback) {
  // Get `client` somehow, for example from a database.
  // ...
  callback(null, client);
}

The example is correct. I'm closing this as answered.

omnicron-llc commented 2 months ago

Your code example does not run, even if an authenticate method is provided that matches the prototype provided. Test your own code. I already have a working example, but this is pathetic.

lpinca commented 2 months ago

Your code example does not run, even if an authenticate method is provided that matches the prototype provided. Test your own code. I already have a working example, but this is pathetic.

I did...

import { createServer } from 'http';
import { WebSocket, WebSocketServer } from 'ws';

function onSocketError(err) {
  console.error(err);
}

function authenticate(request, callback) {
  const client = new URL(request.url, 'wss://base.url').searchParams.get(
    'user'
  );

  if (!client) {
    callback(new Error('message'));
    return;
  }

  callback(null, client);
}

const server = createServer();
const wss = new WebSocketServer({ noServer: true });

wss.on('connection', function connection(ws, request, client) {
  ws.on('error', console.error);

  ws.on('message', function message(data) {
    console.log(`Received message ${data} from user ${client}`);
  });
});

server.on('upgrade', function upgrade(request, socket, head) {
  socket.on('error', onSocketError);

  // This function is not defined on purpose. Implement it with your own logic.
  authenticate(request, function next(err, client) {
    if (err || !client) {
      socket.write('HTTP/1.1 401 Unauthorized\r\n\r\n');
      socket.destroy();
      return;
    }

    socket.removeListener('error', onSocketError);

    wss.handleUpgrade(request, socket, head, function done(ws) {
      wss.emit('connection', ws, request, client);
    });
  });
});

server.listen(8080, function () {
  const ws = new WebSocket('ws://localhost:8080/?user=John');

  ws.on('open', function () {
    ws.send('Hello');
  });
});

It works as expected.