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

Fix regression in wasm connections behind NATs #65

Merged
merged 5 commits into from
Dec 27, 2022

Conversation

johanhelsing
Copy link
Owner

@johanhelsing johanhelsing commented Dec 26, 2022

Something in #54 broke connections between me and another peer (both of us behind NATs)

In this PR, we start sending null ice candidates, and add them on the receiving end. I'm not completely sure what the point of it is, but MDN examples do this so it's probably best if we do it too.

Sadly, this alone wasn't enough to fix the regression. Adding back the wait for ice gathering to complete, however, did fix the issue. So that means that on wasm, we now have some weird frankenstein of ice-trickle and non-trickle, but the regression is gone, and cross-play still seems to work. So it's still better than before, and I'm really anxious to get a fix for the regression merged, even if it's ugly and I don't really understand how it fixes it.

If anyone has ideas about what could be wrong with the trickle implementation, please let me know.

Fixes: #58

@rozgo

@johanhelsing johanhelsing added the bug Something isn't working label Dec 26, 2022
@johanhelsing johanhelsing added this to the 0.5 milestone Dec 26, 2022
@rozgo
Copy link
Contributor

rozgo commented Jan 9, 2023

This WebRTC business is so brittle. How unfortunate that this broke NAT. I'm catching up with all the changes and testing on this side. Thanks for looking into this.

EDIT: Works on our apps.

@johanhelsing
Copy link
Owner Author

Thanks for confirming. I think it's just the wait (357144f) that makes the difference. I found a way to more reliably test the regression without asking someone else to help me test. Here's how I do it:

  1. My regular dev machine behind a regular ipv4 nat.
  2. My laptop connected to tethering on my phone. (note, just connecting with the phone itself is not enough, but using tethering is)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression: Some peers can no longer connect
2 participants