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

init upgrade submodule #115

Merged
merged 8 commits into from
May 29, 2020
Merged

init upgrade submodule #115

merged 8 commits into from
May 29, 2020

Conversation

yoshuawuyts
Copy link
Member

Initializes the "upgrade" submodule.

@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented May 14, 2020

Ref: bootstrapping websockets over h2 -- this is an example of how handshakes are done.

@yoshuawuyts
Copy link
Member Author

The way this is intended to be used is:

  1. client makes a request that includes details to what it wants to upgrade to
  2. an http backend parser (e.g. async-h1) takes ownership of the sender from send_upgrade
  3. some endpoint (e.g. in tide) takes ownership of the upgrade receiver from recv_upgrade and moves it into a task
    • inside the task it should wait for the connection to be passed
  4. that same endpoint then returns the right portion of the server to indicate it's successfully upgraded
  5. [missing] the http backend parser checks whether an upgrade is in progress.
  6. the http backend parser sends through the response in full
  7. if an upgrade was found to be have been requested, the http backend parser then sends through the connection to the endpoint using the upgrade sender obtained earlier
  8. the endpoint is now operating on an upgraded connection and can speak the new protocol

@yoshuawuyts
Copy link
Member Author

Okay, made a mistake. The upgrade sender / receiver needs to live on Response. That way we can add an is_upgrading method to Response that checks whether the upgrade::Receiver has been taken and the backend can decide to then send over the connection. This would resolve point 5 in my previous comment.

@yoshuawuyts
Copy link
Member Author

@dignifiedquire even with the patch merged we're still having issues:

Screenshot 2020-05-15 20 11 01

Tested by running cargo check --features unstable. I think I might have forgotten to add this to CI.

@yoshuawuyts
Copy link
Member Author

Okay, we still need to move this to Response. Let's prioritize this for http-types 2.1.0.

@yoshuawuyts yoshuawuyts marked this pull request as ready for review May 29, 2020 13:10
@yoshuawuyts
Copy link
Member Author

Okay, moved the logic to Response and added a has_upgrade method that returns whether we're upgrading. Everything in this patch is marked as "unstable" and by merging it now we can start experimenting with integrations into async-h1 and Tide. Going to go ahead and merge. Thanks!

@yoshuawuyts yoshuawuyts merged commit edb6cbd into master May 29, 2020
@yoshuawuyts yoshuawuyts deleted the upgrade branch August 4, 2020 10:20
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.

2 participants