search

niv / websocket.nim

websockets for nim
http://niv.github.io/websocket.nim/docs/0.1.1/websocket.html
Other
101 stars 25 forks source link

Some observations #1

Closed dom96 closed 6 years ago

dom96 commented 8 years ago

hey, I've been playing around with this library over the past couple of days and it's working very well so far. Awesome job :)

One thing I just painfully observed is that using send instead of sendText by accident can have some really confusing consequences. I just added a bunch of code to my program, both a "reading" procedure and a "writing" procedure. Suddenly my code just started to crash with a "socket closed". I was really confused and started to wonder whether there was a bug in asynchttpserver, asyncnet, or even in Nim itself. Turns out it was because I used send instead of sendText :\

So perhaps it would be a good idea to introduce a distinct type here to force the use of sendText? :)

dom96 commented 7 years ago

Any thoughts @niv?

I also don't think the readData/sendText etc procs should need a isClientSocket param. You should again use a new type to wrap the AsyncSocket.

dom96 commented 7 years ago

Looks like you created a separate type for the client. But then there is no send procedures for this client type :(

metagn commented 6 years ago

I've moved the AsyncWebSocket type to the shared module, and added a kind: SocketKind field to it which can be Client or Server. Now you can do ws.readData() instead of ws.sock.readData(true), and ws.sendText(masked = false) is possible as well.

I'm guessing this is good enough, but I don't know if ws.send(data) should be added as an alias for sendText or not as a future direction. I vote no but I'd like another opinion

dom96 commented 6 years ago

Thanks for doing that @hlaaftana. I don't really mind what send is called, but sendText implies it can only send text, is that the case? or can I also send binary data?

metagn commented 6 years ago

sendText sends a frame with a text opcode, sendBinary sends a frame with a binary opcode, even though they both take strings. I didn't know if a text opcode was a more common use than a binary opcode, in which case send would be easier to write

dom96 commented 6 years ago

That seems to make sense then since websockets distinguish between both.