Skip to content

Automated socket reconnect - #72

Open
lugehorsam wants to merge 12 commits into
masterfrom
luke/socket-reconnect
Open

Automated socket reconnect#72
lugehorsam wants to merge 12 commits into
masterfrom
luke/socket-reconnect

Conversation

@lugehorsam

@lugehorsam lugehorsam commented Aug 13, 2021

Copy link
Copy Markdown
Contributor
  • Adds retry and cancellation token to the actual connect operation
  • Adds connect with retry if the socket disconnects due to an IOException.
  • Defaults autoReconnect to false. The reason I did this is in case users have existing code doing socket reconnects. If they upgrade and forgot to change a true parameter, they would be "double reconnecting" in the case of disconnects.

@lugehorsam
lugehorsam force-pushed the luke/socket-reconnect branch from 841c14f to 5182ab8 Compare August 13, 2021 14:33
@LobsterUberlord

Copy link
Copy Markdown

Are there any known issues that need to be addressed before this is ready to merge in? I'd like to use this instead of implementing my own socket keep-alive logic.

@novabyte

Copy link
Copy Markdown
Member

@LobsterUberlord The socket code was refactored earlier in the week so I'll need to rework this code to support socket-based reconnects: 6811339

This code can't be rebased and merged in its current form. If you're interested to introduce a pull request to bring this logic into the SDK I'd be happy to review it for you. 🙏

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Luke Gehorsam seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants