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

Websocket support #395

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Jul 30, 2017

This uses Netty's websocket functionality, meaning we do not have to depend on another library.

API

As websockets do not fit neatly into the standard polling socket model, the API is significantly more event based than CCTweaks's. One uses http.websocket to connect, which will wait until a connection is established and then returns the connection object (an async variant is available).

Once you have a websocket object, you can use .send(msg) to transmit a message. Incoming messages will fire a "websocket_message" event, with the URL and content as arguments. A convenience method (.receive()) exists to aid waiting for valid messages.

A websocket_closed event is also fired if the server forcibly terminates the connection, or it times out.

I'd really appreciate feedback on this - I'm not entirely sure it's the best approach, but IMO it's neater than CCTweaks's.

Example

local res, err = http.websocket("wss://echo.websocket.org")
if not res then error(err, 0) end

for i = 1, 10 do
    res.send("hello " .. i)
    print(res.receive())
end

res.close()

Possible enhancements

These are things which probably should be implemented at some point, but I'm looking for some thoughts on them first. Not all of them are entirely websocket related.

  • Limit the number of open sockets. I think it is worth discussing how we can best limit both the number of open sockets and the number of active HTTP requests. It should be relatively trivial to handle either.
  • Automatically close the socket when it is GCd, like we do for file handles. We'd probably have to split up the connection into the ILuaObject part and the socket holding part, detecting when the former is no longer referenced.
  • Have a separate blacklist (and whitelist?) for websockets. There may be some sites for which one would like to allow simple requests, but not websockets.
  • Add a timeout argument to receive(). I'm not sure the best way to go about that - we obviously don't have access to os.startTimer on the Java side.
  • Replace the current HTTP code with netty's. This means we can do it all asynchronously, and so not consume a thread for each request.

Closes #161

@Lupus590
Copy link
Contributor

The API description which you gave at the start of your post sounds like it mimics the rednet/modem API's in interface, which I would consider a plus as that is what some users of the mod will be more familiar with when using the API.

This means we don't have to have lots of shared state between the run
and whenFinished method, and allows for easier chaining of futures later
on.
@MineRobber9000
Copy link
Contributor

I made what I believe to be the first public software to use this PR. It makes a file handle interface to the WebsocketConnection object.

This uses Netty's websocket functionality, meaning we do not have to
depend on another library.

As websockets do not fit neatly into the standard polling socket model,
the API is significantly more event based than CCTweaks's. One uses
http.websocket to connect, which will wait until a connection is
established and then returns the connection object (an async variant is
available).

Once you have a websocket object, you can use .send(msg) to transmit a
message. Incoming messages will fire a "websocket_message" event, with
the URL and content as arguments. A convenience method (.receive())
exists to aid waiting for valid messages.
@dmarcuse dmarcuse mentioned this pull request Jun 14, 2018
 - Add an argument to send which controls whether it's a binary message
   or not. This is a little ugly, but it's probably more effective than
   anything else.
 - Fix binary frames not correctly queueing the correct data in the
   message event.
ccserver pushed a commit to ccserver/ComputerCraft that referenced this pull request Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggestion - Web Sockets
3 participants