-
Notifications
You must be signed in to change notification settings - Fork 136
Fix bugs with WebSocket fallback #844
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
Fix bugs with WebSocket fallback #844
Conversation
rs/moq-native/Cargo.toml
Outdated
| serde_with = "3" | ||
| time = "0.3" | ||
| tokio = { workspace = true, features = ["full"] } | ||
| tokio-tungstenite = { version = "0.24", features = ["rustls-tls-webpki-roots"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would probably be better if we could continue to use this crate via web-transport's re-export, so we can go that route if preferred. We need to enable this feature to enable rustls support. The tokio-tungstenite crate only exposes that via two higher-level features, either rustls-tls-native-roots or rustls-tls-webpki-roots, so you have to pick one or the other. I chose this one because the webpki crates are already in the dependency tree elsewhere so I thought that was the lesser of two evils.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can set feature flags when using re-exports unfortunately. You'd have to add these features to web_transport_ws and proxy them through that way, which is probably better because then you won't run into versioning issues.
3ced7b9 to
1917075
Compare
The clap::arg default value only applies when parsing args.
This version re-exports the tokio-tungstenite traits needed to modify TLS behaviors.
Previously it did not respect flags controlling TLS behavior.
4886436 to
84fb6b3
Compare
WalkthroughThe pull request updates the 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (1)rs/**/*.rs📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📚 Learning: 2025-12-20T13:14:11.182ZApplied to files:
📚 Learning: 2026-01-11T03:47:52.766ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ClientConfig::default()