Skip to content

fix(connlib): ensure ICE timing config applies immediately#9014

Merged
thomaseizinger merged 1 commit into
mainfrom
fix/bump-str0m-timeout
May 2, 2025
Merged

fix(connlib): ensure ICE timing config applies immediately#9014
thomaseizinger merged 1 commit into
mainfrom
fix/bump-str0m-timeout

Conversation

@thomaseizinger
Copy link
Copy Markdown
Member

When there is no traffic going through the tunnel, Firezone switches into a low-power mode where it only sends STUN bindings every 60s. As soon as we see traffic, we move out of this low-power mode to detect connectivity problems early.

Applying this new timing config however does not clear some internal caches in str0m and therefore, it can take up to the previously set timeout value until str0m actually picks up on the new timers.

This is being fixed in algesten/str0m#649. Until that is merged, we depend on our fork that has these changes merged already.

Resolves: #8999

@thomaseizinger thomaseizinger requested review from Copilot and jamilbk May 2, 2025 08:11
@vercel
Copy link
Copy Markdown

vercel Bot commented May 2, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
firezone ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 2, 2025 8:11am

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the ICE timing issue by ensuring the new timer configuration applies immediately by switching to a fork of str0m that clears internal caches more promptly.

  • Updated dependency for str0m in Cargo.toml to point to the fork with the necessary fixes.

Comment thread rust/Cargo.toml
@thomaseizinger
Copy link
Copy Markdown
Member Author

I tested this locally (was only reproducible if you have a single candidate pair, i.e. what usually happens with relayed connections). It is unit-tested in str0m which is why I didn't add an additional test here in Firezone.

@thomaseizinger thomaseizinger enabled auto-merge May 2, 2025 08:16
Copy link
Copy Markdown
Member

@jamilbk jamilbk left a comment

Choose a reason for hiding this comment

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

Nice find!

@thomaseizinger thomaseizinger added this pull request to the merge queue May 2, 2025
Merged via the queue into main with commit 56a4cad May 2, 2025
108 checks passed
@thomaseizinger thomaseizinger deleted the fix/bump-str0m-timeout branch May 2, 2025 14:25
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.

ICE timeout takes ~60s

3 participants