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

Proxy support for WebSockets is g2g! #137

Merged
merged 1 commit into from
Mar 5, 2021
Merged

Proxy support for WebSockets is g2g! #137

merged 1 commit into from
Mar 5, 2021

Conversation

thedodd
Copy link
Member

@thedodd thedodd commented Mar 4, 2021

The issue described in #95 has been thoroughly tested & is now fixed.
Shoutout to @jakule for their work on implementing a solid fix for this.

closes #53, #81, #95
closes #105 (supersedes, though adding co-authored-by to preserve attribution on original work)

Checklist

  • Updated CHANGELOG.md describing pertinent changes.
  • Updated README.md with pertinent info (may not always apply).

@thedodd thedodd force-pushed the proxy-websockets branch 3 times, most recently from 1f5dda0 to 36bb099 Compare March 4, 2021 17:16
structopt = "0.3"
structopt-derive = "0.4"
surf = "2"
tide = { version="0.16.0", features=["unstable"] }
tide = { version = "0.16.0", features = ["unstable"] }
tide-websockets = { git = "https://github.com/http-rs/tide-websockets.git", rev = "270f408cdf4e5ee2bd28c7f5fcb57e5085d49ead" }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jbr thanks for your work on this. I'll update to the next release (which supports sending raw messages) once it lands. It works quite nicely!

@thedodd
Copy link
Member Author

thedodd commented Mar 4, 2021

@hamza1311 this PR supersedes #105. I've given you attribution on it given that you trail-blazed much of this initial work. A few items to note:

  • I've removed the backwards incompatible rename of proxy-rename to proxy-path (I'm happy to go into more details on why).
  • I've preserved the work related to the nipper refactor. This was originally only an issue with Tokio, but it is actually a good refactor, and I think it helps to isolate the rest of the code from the nipper selection types.
  • Everything is still on async-std/tide. I was pretty unhappy with state of affairs with Warp. The encapsulation was not great, its WebSocket support was not so hot. With this change, everything is now squarely based on (async-)tungstenite, so the backend and frontend of the proxy share the same foundation, which is quite nice and has simplified the proxy logic SIGNIFICANTLY.

The only standing drawback of this current state: no HTTP2 support. I'm not super concerned about that. Given that this is a development server/proxy, and given that H2 gracefully degrades to H1.X when needed, there should be no actual development impact. That said, when H2 support lands in Tide, we reap the benefits with no effort on our side.

The issue described in #95 has been thoroughly tested & is now fixed.
Shoutout to @jakule for their work on implementing a solid fix for this.

closes #95

Co-authored-by: Hamza <[email protected]>
@thedodd thedodd force-pushed the proxy-websockets branch from 3639c57 to eb4d33c Compare March 5, 2021 02:25
@thedodd thedodd merged commit e1deab5 into master Mar 5, 2021
@thedodd thedodd deleted the proxy-websockets branch March 5, 2021 02:25
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.

Proxy WebSockets
2 participants