Server fails with error after client emitted event

[OEvgeny]

I faced with strange issue using ws-wrapper. After client emits something server just fails with this error:

[1] /.../node_modules/ws-wrapper/lib/wrapper.js:247
[1]             this.send(JSON.stringify(data) );
[1]                            ^
[1]
[1] TypeError: Converting circular structure to JSON
[1]     at JSON.stringify (<anonymous>)
[1]     at WebSocketWrapper._sendEvent (/.../node_modules/ws-wrapper/lib/wrapper.js:247:18)
[1]     at WebSocketWrapper.emit (/.../node_modules/ws-wrapper/lib/channel.js:85:25)
[1]     at WebSocket.socket.onmessage (/.../node_modules/ws-wrapper/lib/wrapper.js:75:9)
[1]     at WebSocket.onMessage (/.../node_modules/ws/lib/event-target.js:120:16)
[1]     at emitOne (events.js:115:13)
[1]     at WebSocket.emit (events.js:210:7)
[1]     at Receiver.receiverOnMessage (/.../node_modules/ws/lib/websocket.js:718:20)
[1]     at emitOne (events.js:115:13)
[1]     at Receiver.emit (events.js:210:7)

Client (in browser)

  import WebSocketWrapper from 'ws-wrapper'
  const socket = new WebSocketWrapper(new WebSocket('ws://localhost:3000'))
  socket.emit('echo-me', '123')

Server

const WebSocketServer = require("ws").Server
const WebSocketWrapper = require("ws-wrapper")
const wss = new WebSocketServer({port: 3000})

wss.on("connection", (rawSocket) => {
  const socket = new WebSocketWrapper(rawSocket)

  socket.on('echo-me', (data) => {
    console.log('Echo', data)
  })
});

As you can see server just emits nothing.

I'm trying to isolate issue to give you a cleaner reproduction. For some reason I can't reproduce it with node clients.

[bminer]

Thanks for submitting a bug report.

It's crazy because I see nothing wrong with your code, and when I tested it, it worked properly.

One thing I noticed was your error message: TypeError: Converting circular structure to JSON. ws-wrapper does not support serializing circular JSON structures, so you have avoid passing function arguments to socket.emit(...) if one of the arguments refers to back upon itself somehow.

The following code will fail, for example:

const socket = new WebSocketWrapper(new WebSocket('ws://localhost:3000'))
var obj = {};
obj.hello = "world";
obj.myself = obj;  // Don't do this!
socket.emit('echo-me', obj)

[bminer]

If you're still having trouble, you are welcome to post back here with more code, and I'd be happy to help. It's cool to see others using this lib. Best of luck!

Closing this issue for now. Please feel free to re-open if needed.

EDIT: The source of this issue is documented below:

wss.on("connection", (rawSocket) => {
  const socket = new WebSocketWrapper(rawSocket)

  // The following line it what is causing this Error
  socket.name = `Socket${Math.random()}`;

  socket.on('echo', (data) => {
    console.log('Echo', data, socket.name)
  })
});

[OEvgeny]

@bminer Thanks for looking into this! In fact I didn't pass anything to socket.emit(...) but '123'. Seems something is wrong with my setup.

[bminer]

Sure thing.

If you'd like to send me more code, I can try to point you in the right direction. I'm at a loss at the moment. If you'd prefer, we can take this "offline". My email address is here: https://github.com/bminer

[OEvgeny]

@bminer I hid some information which I thought was irrelevant to the issue.

Here is the repro I made: OEvgeny/ws-wrapper-example

Please take a look at this line which causes original issue. It was a bit surprising to me 😄
Not sure is it something expected or not.

[OEvgeny]

Also I noticed there are no any tests yet. If you have any preferences just let me know. But I'm not sure I'll have any time to contribute in near future.

[bminer]

Thanks for posting. I didn't realize it was the server that was crashing with a TypeError. Sorry for glossing over that detail.

When you set the name property of a WebSocketWrapper, your code will break. This is because the WebSocketWrapper extends a WebSocketChannel, and you aren't allowed to set the name property on a WebSocketChannel. In fact, I would recommend against setting any property on the WebSocketChannel.

The best solution to this problem is to use the .set() method to set data for a particular WebSocketWrapper. For example:

const WebSocketServer = require('ws').Server
const WebSocketWrapper = require('ws-wrapper')
const wss = new WebSocketServer({port: 3000})

wss.on('connection', (rawSocket) => {
  const socket = new WebSocketWrapper(rawSocket)
  // Use `set` method to store custom data on the Socket (this is always safe)
  socket.set('name', `Socket${Math.random()}`)
  socket.on('echo', (data) => {
    console.log('Echo', data, socket.name)
    socket.emit('echo', data)
  })
});

[bminer]

A few side notes (not related to this issue):

• I'd recommend taking a look at ws-server-wrapper if you're planning on using Node.js as the server. An example app is provided.
• Tests: I don't have any. It's quite typical of me unfortunately. This is a want but not a need at the moment. To be honest, I will graciously accept tests from nearly any testing framework. I've always been a fan of mocha + Node.js core assert.

[OEvgeny]

Thanks for the information. All of these tips are very useful!

As for original issue I have an idea. Looks like it is possible to make produced by WebSocketWrapper constructor object frozen. Which will make error message more cleaner.

[bminer]

Really good idea. I will open an issue for that!