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.

Sign up for GitHub

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

Jump to bottom

WebRTC fixes #684

Merged
merged 9 commits into from
Aug 30, 2023
Merged

WebRTC fixes #684

merged 9 commits into from
Aug 30, 2023

Conversation

GhenadieVP
Copy link
Contributor

@GhenadieVP GhenadieVP commented Aug 23, 2023 •
edited
Loading

Updates:

  1. When sending back response, if there is no active PeerConnection, we do create the RTCClient stack from scratch - disconnect form SS, reconnect and establish the connection.
  2. Increased the connection timeout, as for som VPN networks it might take over 10 seconds to connect.
  3. Consider a connection opened after the DataChannel is opened - now it is needed, because in scenario 1, the Wallet be the one sending the first message over the DataChannel after reconnecting.
  4. Close the connection if DataChannel did close.

@GhenadieVP GhenadieVP changed the title WIP - WebRTC fixes WebRTC fixes Aug 30, 2023
@GhenadieVP GhenadieVP marked this pull request as ready for review August 30, 2023 05:40
CyonAlexRDX
CyonAlexRDX approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX left a comment

Choose a reason for hiding this comment

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

LGTM! Has this been tested behind double VPN?

@GhenadieVP
Copy link
Contributor Author

LGTM! Has this been tested behind double VPN?

I recall that we did fix a related issues some time ago, and the fix was done on the SignalingServer side.

This update, does not impact negotiation in any way.

@GhenadieVP GhenadieVP merged commit 59c9733 into main Aug 30, 2023
5 checks passed
@GhenadieVP GhenadieVP deleted the webRTC_fixes branch August 30, 2023 06:51
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CyonAlexRDX CyonAlexRDX CyonAlexRDX approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@GhenadieVP @CyonAlexRDX