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

Factor out web media connection into model::connection::Connection #105

Merged
merged 4 commits into from
Jul 27, 2023

Conversation

ronen
Copy link
Collaborator

@ronen ronen commented Jul 17, 2023

[Another step towards #74]

@darioalessandro: I haven't tested this with WebTransport -- I don't know how.

Even in main I can't/don't know how to get webtransport working?!

So the WebTransport parts of this PR are all of the hopeful vein "if it compiles it probably works" -- I didn't write any real new logic it's all just rearranging the existing code, so that's got a reasonable chance of being true :) But of course you should test it. And/or can you explain how to run with webtransport locally on my mac?

Here's what's in the PR:

Pulled all the relevant code out of attendants.rs into model/connection/* including eliminating a bunch of yew Messages that had been used to set up the WebTransport connection.

In doing the refactoring, organized the code into a few levels of abstraction. From the bottom up, they are:

  • webmedia.rs defines a WebMedia trait for MediaPacket-based connections, to provide a connect() method that sets up the desired callbacks (in particular on_incoming_packet), and a send_packet() method to transmit packets.

  • websocket.rs and webtransport.rs implement the WebMedia trait for their respective mechanisms. websocket.rs is pretty trivial; webtransport contains the logic to handle the different types of streams and to buffer the data to construct and dispatch packets.

  • task.rs defines a generic Task interface over websocket and webtransport, and in particular handles the fallback rollover from webtransport to websocket

  • connection.rs defines the top-level public interface, which wraps the Task interface in a Connection struct that also handles heartbeats and keeps track of the connection status, so that it can suppress attempts to send packets when the connection has been closed and can provide an is_connected() method.

…ction

Pulled all the relevant code out of attendants.rs into model/connection/*
including eliminating a bunch of yew Messages that had been used to set
up the WebTransport connection.

In doing the refactoring, organized the code into a few levels of
abstraction:

* webmedia.rs defines a WebMedia trait for MediaPacket based
  connections, to provide a connect() method that sets up the desired
  callbacks (in particular on_incoming_packet), and a send_packet()
  method to transmit packets.

* websocket.rs and webtransport.rs implement the WebMedia trait for
  their respective mechanisms.  websocket.rs is pretty trivial;
  webtransport contains the logic to handle the different types of
  streams and to buffer the data to construct and dispatch packets.

* task.rs defines a generic interface over websocket and webtransport,
  and in particular handles the fallback rollover from webtransport to
  websocket

* connection.rs defines the top-level public interface, which wraps the
  Task generic interface in a Connection struct that also handles
  heartbeats and keeps track of the connection status (to suppress
  attempts to send packets when the connection has been closed and to
  provide an is_connected() method).
@darioalessandro
Copy link
Member

hey @ronen !! are you facing cert issues? if so, please launch chrome with https://github.com/security-union/videocall-rs/blob/main/launch_chrome.sh make sure that you launch chrome first, then the API so that certs match

@griffobeid
Copy link
Contributor

You can also use https://transport.rustlemania.com:443 I believe to avoid launching chrome with that script

@ronen
Copy link
Collaborator Author

ronen commented Jul 19, 2023

Thanks, but I don't get as far as cert issue :)

My question is more basic: How do I even run with WebTransport enabled?

My intent to run with the server on my local machine, trying it out by connecting a few times to http://localhost/meeting/username/room from several Chrome tabs with various usernames.

I normally just run:

$ make up

Which only attempts a Websocket connection. I've tried

$ WEBTRANSPORT_ENABLED=1 make up

But then the server seems to crash and the web pages don't load at all. I haven't dug into it to figure out why, just assumed I'm doing something wrong.

What is the proper way to run with WebTransport?

@darioalessandro
Copy link
Member

Try with the cloud version first:

https://rustlemania.com/meeting/ronen/ronen-meeting/true

The last true is the webtransport feature (I know, pretty horrible).
Screenshot 2023-07-19 at 12 28 17 AM

Once you can see that it is working, try:

http://localhost/meeting/ronen/ronen-meeting/true

You can set the server to point to prod so that you do not need to launch chrome with crazy flags

@ronen
Copy link
Collaborator Author

ronen commented Jul 21, 2023

(Sorry for slow response. I'm traveling with only intermittent time & web access.)

I can successfully connect to rustlemania.com with webtransport true. But I haven't been able to do it locally in main. Chrome console error:

Failed to establish a connection to https://127.0.0.1:4433/lobby/other/asdf: net::ERR_QUIC_PROTOCOL_ERROR.QUIC_NETWORK_IDLE_TIMEOUT (No recent network activity after 4010240us. Timeout:4snum_undecryptable_packets: 0 {}).
yew-ui-6accc82abf3e650e.js:472 Failed to read incoming unidirectional streams WebTransportError: Opening handshake failed.

also, in the terminal the messages from make up include a bunch of rust warnings both for dev-websocket-api and dev-webtransport-api, and a runtime panic:

docker-webtransport-api-1  | thread 'main' panicked at 'expected HEALTH_LISTEN_URL to be set: NotPresent', src/bin/webtransport_server.rs:22:10

Can you give me really basic step-by-step instructions for how to get main to compile and run cleanly locally, with webtransport working? Thanks!

@ronen
Copy link
Collaborator Author

ronen commented Jul 23, 2023

Hi, I saw #108 go by that addresses HEALTH_LISTEN_URL -- and with that I was able to get WebTransport working in main running locally (using the script to set the certs). Yay!

So I merged the latest main into this PR, and now have verified that both WebSocket and WebTransport are working in this PR. (Yay again!)

So AFAICT this PR is ready for you to look at / try out yourself.

@darioalessandro
Copy link
Member

@ronen fantastic!! I'll do that

yew-ui/src/components/attendants.rs Outdated Show resolved Hide resolved
@darioalessandro darioalessandro merged commit b539a7b into security-union:main Jul 27, 2023
BanjoFox pushed a commit to BanjoFox/videocall-rs that referenced this pull request Aug 18, 2023
…ecurity-union#105)

* Factor out web media connection and use into model::connection::Connection

Pulled all the relevant code out of attendants.rs into model/connection/*
including eliminating a bunch of yew Messages that had been used to set
up the WebTransport connection.

In doing the refactoring, organized the code into a few levels of
abstraction:

* webmedia.rs defines a WebMedia trait for MediaPacket based
  connections, to provide a connect() method that sets up the desired
  callbacks (in particular on_incoming_packet), and a send_packet()
  method to transmit packets.

* websocket.rs and webtransport.rs implement the WebMedia trait for
  their respective mechanisms.  websocket.rs is pretty trivial;
  webtransport contains the logic to handle the different types of
  streams and to buffer the data to construct and dispatch packets.

* task.rs defines a generic interface over websocket and webtransport,
  and in particular handles the fallback rollover from webtransport to
  websocket

* connection.rs defines the top-level public interface, which wraps the
  Task generic interface in a Connection struct that also handles
  heartbeats and keeps track of the connection status (to suppress
  attempts to send packets when the connection has been closed and to
  provide an is_connected() method).

* connection log messages

* Update yew-ui/src/components/attendants.rs

Co-authored-by: Griffin Obeid <[email protected]>

---------

Co-authored-by: Dario A Lencina-Talarico <[email protected]>
Co-authored-by: Griffin Obeid <[email protected]>
BanjoFox pushed a commit to BanjoFox/videocall-rs that referenced this pull request Aug 22, 2023
…ecurity-union#105)

* Factor out web media connection and use into model::connection::Connection

Pulled all the relevant code out of attendants.rs into model/connection/*
including eliminating a bunch of yew Messages that had been used to set
up the WebTransport connection.

In doing the refactoring, organized the code into a few levels of
abstraction:

* webmedia.rs defines a WebMedia trait for MediaPacket based
  connections, to provide a connect() method that sets up the desired
  callbacks (in particular on_incoming_packet), and a send_packet()
  method to transmit packets.

* websocket.rs and webtransport.rs implement the WebMedia trait for
  their respective mechanisms.  websocket.rs is pretty trivial;
  webtransport contains the logic to handle the different types of
  streams and to buffer the data to construct and dispatch packets.

* task.rs defines a generic interface over websocket and webtransport,
  and in particular handles the fallback rollover from webtransport to
  websocket

* connection.rs defines the top-level public interface, which wraps the
  Task generic interface in a Connection struct that also handles
  heartbeats and keeps track of the connection status (to suppress
  attempts to send packets when the connection has been closed and to
  provide an is_connected() method).

* connection log messages

* Update yew-ui/src/components/attendants.rs

Co-authored-by: Griffin Obeid <[email protected]>

---------

Co-authored-by: Dario A Lencina-Talarico <[email protected]>
Co-authored-by: Griffin Obeid <[email protected]>
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.

3 participants