[client] Recreate agent when receive new session id #4564
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ICE agent connection logic to handle new session IDs more efficiently. The changes ensure that when a new session ID is received, the agent is properly recreated rather than being skipped.
- Simplified connection state checking by combining agent existence and connection status checks
- Moved session ID assignment to occur earlier in the connection process
- Ensures session ID is set when creating the agent rather than after successful connection
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } | ||
|
|
||
| if w.agent != nil { | ||
| if w.agent != nil || w.agentConnecting { |
There was a problem hiding this comment.
The logic change from separate if w.agentConnecting and if w.agent != nil checks to a combined condition may alter the behavior. Previously, if agentConnecting was true, the function would return early without any session ID checks. Now it will proceed to check session IDs even when connecting. Ensure this behavioral change is intentional and doesn't introduce race conditions.
| if w.agent != nil || w.agentConnecting { | |
| if w.agentConnecting { | |
| w.log.Debugf("agent is connecting, skipping the offer") | |
| w.muxAgent.Unlock() | |
| return | |
| } | |
| if w.agent != nil { |
Copilot uses AI. Check for mistakes.
| if remoteOfferAnswer.SessionID != nil { | ||
| w.remoteSessionID = *remoteOfferAnswer.SessionID | ||
| } |
There was a problem hiding this comment.
The session ID assignment has been moved from the connect method (which runs in a goroutine) to the main thread while holding the mutex. This is good for thread safety, but ensure that any code that depends on w.remoteSessionID being set after connection establishment still works correctly with this earlier assignment.
Copilot uses AI. Check for mistakes.
pappz
changed the title
- Close depreacted agent before send offer answer - Generate new session ID before send answer
| w.muxAgent.Lock() | ||
| defer w.muxAgent.Unlock() | ||
|
|
||
| if w.agentConnecting { |
There was a problem hiding this comment.
could you explain why we create a new session if status is agent connection now?
|
Describe your changes
When an ICE agent connection was in progress, new offers were being ignored. This was incorrect logic because the remote agent could be restarted at any time.
In this change, whenever a new session ID is received, the ongoing handshake is closed and a new one is started.
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__