HenningM / express-ws

WebSocket endpoints for express applications
BSD 2-Clause "Simplified" License
877 stars 142 forks source link

Sending object fails via method ws.send #72

Open ZaDarkSide opened 7 years ago

ZaDarkSide commented 7 years ago

var express = require('express');
var expressWs = require('..');

var expressWs = expressWs(express());
var app = expressWs.app;

app.use(function (req, res, next) {
  console.log('middleware');
  req.testing = 'testing';
  return next();
});

app.get('/', function(req, res, next){
  console.log('get route', req.testing);
  res.end();
});

app.ws('/', function(ws, req) {
  ws.on('message', function(msg) {
    console.log(msg);
    ws.send({test: 'test'});  //<== will fail with "TypeError: First argument must be a string, Buffer, ArrayBuffer, Array, or array-like object."
  });
  console.log('socket', req.testing);
});

app.listen(3000)
HenningM commented 7 years ago

Hi,

I think this snippet would also fail on earlier versions of Node. ws.send() doesn't do any implicit JSON encoding of the data. To transfer an object you would need to encode & decode it explicitly.

The current implementation directly exposes a WebSocket type object from the ws library. It might make sense for us to augment the ws object a little bit here though. I can understand that it's confusing when you're able to pass objects to res.send(), but not to ws.send().

ZaDarkSide commented 7 years ago

Yes, express-ws uses ws to send data, and ws should support sending objects in Sender.js I think I will open an issue there too, because it's a more appropriate place.

lpinca commented 7 years ago

I agree that res.send() and ws.send() may be confusing but the send() method does not work with objects in the WebSocket browser interface.

If you pass an object you'll get the result of obj.toString().

As per this comment https://github.com/websockets/ws/issues/1102#issuecomment-300487700, I think that a WebSocket library should not have anything to do with SerDes.

ZaDarkSide commented 7 years ago

OK, I closed the issue on ws. I will close the issue here too if there is no agreed resolution. It's really a minor issue for new-comers and code readability. This is a really small enhancement. I would prefer to write just ws.send({test: 'test'}); and not ws.send(JSON.stringify({test: 'test'})); and in client JSON.parse everywhere, it's more readable for me, and more logical, I'm sending an object and I don't care what method it is doing for SerDes, and in the client I should receive the same object so the complexity should be abstracted from the developer.

N-McA commented 6 years ago
// This middleware lets you send objects on websockets.
app.use((req, res, next) => {
  if (req.ws && isinstance(req.ws, WebSocket)) {
    let oldSend = req.ws.send
    req.ws.send = function() {
      let args = [].slice.call(arguments, 0);
      let arg0 = args[0]
      if (Object.prototype.toString.call(arg0) == "[object Object]") {
        let arg0String = JSON.stringify(arg0);
        args[0] = arg0String;
      }
      oldSend.apply(this, args)
    }
  }
  next()
})