lightbug_http icon indicating copy to clipboard operation
lightbug_http copied to clipboard

Websocket: first commit

Open rd4com opened this issue 1 year ago • 1 comments

Hello, here is what we've got so far:

  • upgrade http request to websocket
  • send all size
  • receive all size
  • works with select for non-blocking

all sizes means len(data) <=125, <2**16, <((2**64)>>8)

 

Many things not done:

  • ping/pong
  • fragment message

(and more)

 

There is a need to think about how it's gonna be integrated, we'll need to organize the flags in aliases, for example.

Error handling seem like a great priority :+1:

 

Note that the code appears to be more complicated than what it is doing, because we're doing extra steps to work with ByteArray (PythonObject) in this example.

rd4com avatar Aug 20 '24 19:08 rd4com

Thanks, looks great! Will review tomorrow. Will let you know if I have any questions

saviorand avatar Aug 20 '24 19:08 saviorand

Alright, merged into the branch. Let's open new PRs for improvements/refactors. I've also merged the nightly updates into this, as main is on the latest stable version and breaks with nightly.

The main to-do's I'm seeing so far before merging into main, in order of importance:

  1. Integrate with the rest of the code by adding this websocket infra as an option into the SysServer or making a separate server struct for this. For now thinking what's the best way to approach this for best ergonomics/developer experience
  2. Replace python imports with equivalent Mojo/libc calls. E.g. in socketwe've got implementations interfacing with C via external_calls for operations like opening socket, recv, bind and so on
  3. Localhost IP/port is currently hardcoded, can make it configurable since it's already passed as a parameter, the magic number can be an actual constant (alias). General clean up - we should remove prints/extra comments
  4. The index.html file is useful for trying it out, but we should remove it and write unit tests + a simple e2e test instead

I can work on 1, 2 and 4. Can you look into 3? Also if you have any other scope in mind feel free to open new PRs into the same branch

saviorand avatar Aug 21 '24 19:08 saviorand