TakahikoKawasaki / nv-websocket-client

High-quality WebSocket client implementation in Java.
Apache License 2.0
2.03k stars 292 forks source link

Add RFC 6555 support (Dual Stack / Happy Eyeballs) #183

Closed lgrahl closed 5 years ago

lgrahl commented 5 years ago

This PR adds a happy eyeballs algorithm and thereby provides dual stack support. It allows to configure the dual stack mode and fallback delay on the WebSocketFactory while defaulting to recommendations of RFC 6555.

Be aware that this PR introduces breaking changes to the API, specifically the getSocket method may now return null. To counter this, an additional getConnectedSocket method has been added which establishes the underlying Socket instance first. It was not possible to move this functionality into getSocket since that does not throw an exception.

Resolves #112

lgrahl commented 5 years ago

@TakahikoKawasaki thoughts? :slightly_smiling_face:

TakahikoKawasaki commented 5 years ago

@lgrahl I'm sorry for not responding. I'll take time to comment after DeveloperWeek New York 2019 finished.

dbrgn commented 5 years ago

@TakahikoKawasaki As a library maintainer I hate pings, but here's a reminder anyways, sorry :slightly_smiling_face: I work on the same project as @lgrahl and we need to decide whether to temporarily fork this library for use in our project, or whether we want to wait for upstream support.

If you don't have time at the moment for a review that's perfectly fine, in that case we'll continue with a fork for the time being. Thanks for the maintenance!

(PS: In case this is of interest to you, we're using nv-websocket-client in Threema's webclient support through SaltyRTC. Works nicely so far, besides the IPv6 troubles.)

TakahikoKawasaki commented 5 years ago

@dbrgn @lgrahl Sorry. I start to review the pull request now.

TakahikoKawasaki commented 5 years ago

@lgrahl Thank you for your pull request. To be honest, I cannot check every detail. However, your code looks beautiful enough for me to believe it will work. I'll merge it.

TakahikoKawasaki commented 5 years ago

@lgrahl @dbrgn Released nv-websocket-client version 2.9 which includes your pull request. I hope the new library will appear at the Maven Central Repository soon.

dbrgn commented 5 years ago

@TakahikoKawasaki great, thank you!

lgrahl commented 5 years ago

@TakahikoKawasaki That reaction was unexpected but most appreciated. :slightly_smiling_face: Cheers!