Commit Graph
5 Commits
Author SHA1 Message Date
Riccardo Manfrin 70ddb2a393 [client] Bound the Signal stream registration wait and track its state based on stream health (#8175)
* [client] Move the Signal registration wait into its own helper

Pure move, no behavior change. The wait for the registration header and
the check on its value leave connect() for awaitRegistration(), so the
next commit can bound that wait in one place.

* [client] Split the Signal test server helper from its listener setup

Pure move, no behavior change. serveTestSignalExchange serves any
SignalExchangeServer on a local listener; startTestSignalServer keeps
serving the real server through it. Lets a test wrap the real server.

* [client] Bound the wait for the Signal registration header

After an outage the Signal client could wait forever for the registration
header of a new stream. Keepalive cannot end that wait when whatever
answers the transport (a proxy in front of Signal, or Signal itself)
accepts the stream and never answers it, and the receive watchdog only
starts once registration succeeds. Signal then stayed down until the
daemon was restarted, while Management and Relay recovered.

Cancel the stream when the header does not arrive within 10 seconds, so
connect() fails and the existing retry loop opens a new stream. A timer
is used instead of a context deadline because the stream context must
outlive registration.

* [client] Report Signal unhealthy while no stream is registered

IsHealthy judged Signal by the gRPC connection state alone and marked it
connected whenever the connection was Ready or Idle. With the connection
up but no stream registered, the client cannot receive offers or answers,
yet netbird status showed Signal: Connected, hiding a stuck registration
for as long as it lasted.

Require a registered stream before reporting healthy. When there is none,
return false without touching the recorder: the disconnect that dropped
the stream already reported its reason.

* [client] Name the server that reconnected in the SR watcher log

Signal and Relay shared one reconnect callback, which logged "reconnected
to Signal or Relay server" for either. A Relay reconnect while the Signal
stream was down read as Signal having recovered, which misled diagnosis
of a stuck Signal registration.

Register one callback per server so the log names the one that actually
reconnected. Which listeners get notified, and when, is unchanged.

* [client] Trim comments in the Signal registration and health code

Comment-only change, no behavior change. Drop comments that restate the
code and keep only the reason a timer is used instead of a context
deadline.

* [client] Check Signal stream registration before the connection state

IsHealthy returned true for a Connecting or Shutdown channel before the
registration check ran, so a client with no registered stream could
still be reported healthy while the channel was reconnecting. Check the
stream first; the connection-state cases now only apply to a client
that has a registered stream.

* [client] Log a plain connect in the SR watcher

The Signal and Relay callbacks also fire on the first connection, not
only after a disconnect, so "reconnected" was wrong at startup. Log
"connected to <server> server" for both cases.

* [client] Say what the SR watcher does when it logs

The line was read as a per-server connect log, which Signal and Relay
already emit themselves. What it marks is the peer guards being told to
reset their reconnection backoff, which happens only once both are
ready. Log that, naming the server whose event triggered it.
2026-10-13 10:54:30 +02:00
Zoltan Papp 5a89e6621b [client] Supress ICE signaling (#5820)
* [client] Suppress ICE signaling and periodic offers in force-relay mode

When NB_FORCE_RELAY is enabled, skip WorkerICE creation entirely,
suppress ICE credentials in offer/answer messages, disable the
periodic ICE candidate monitor, and fix isConnectedOnAllWay to
only check relay status so the guard stops sending unnecessary offers.

* [client] Dynamically suppress ICE based on remote peer's offer credentials

Track whether the remote peer includes ICE credentials in its
offers/answers. When remote stops sending ICE credentials, skip
ICE listener dispatch, suppress ICE credentials in responses, and
exclude ICE from the guard connectivity check. When remote resumes
sending ICE credentials, re-enable all ICE behavior.

* [client] Fix nil SessionID panic and force ICE teardown on relay-only transition

Fix nil pointer dereference in signalOfferAnswer when SessionID is nil
(relay-only offers). Close stale ICE agent immediately when remote peer
stops sending ICE credentials to avoid traffic black-hole during the
ICE disconnect timeout.

* [client] Add relay-only fallback check when ICE is unavailable

Ensure the relay connection is supported with the peer when ICE is disabled to prevent connectivity issues.

* [client] Add tri-state connection status to guard for smarter ICE retry (#5828)

* [client] Add tri-state connection status to guard for smarter ICE retry

Refactor isConnectedOnAllWay to return a ConnStatus enum (Connected,
Disconnected, PartiallyConnected) instead of a boolean. When relay is
up but ICE is not (PartiallyConnected), limit ICE offers to 3 retries
with exponential backoff then fall back to hourly attempts, reducing
unnecessary signaling traffic. Fully disconnected peers continue to
retry aggressively. External events (relay/ICE disconnect, signal/relay
reconnect) reset retry state to give ICE a fresh chance.

* [client] Clarify guard ICE retry state and trace log trigger

Split iceRetryState.attempt into shouldRetry (pure predicate) and
enterHourlyMode (explicit state transition) so the caller in
reconnectLoopWithRetry reads top-to-bottom. Restore the original
trace-log behavior in isConnectedOnAllWay so it only logs on full
disconnection, not on the new PartiallyConnected state.

* [client] Extract pure evalConnStatus and add unit tests

Split isConnectedOnAllWay into a thin method that snapshots state and
a pure evalConnStatus helper that takes a connStatusInputs struct, so
the tri-state decision logic can be exercised without constructing
full Worker or Handshaker objects. Add table-driven tests covering
force-relay, ICE-unavailable and fully-available code paths, plus
unit tests for iceRetryState budget/hourly transitions and reset.

* [client] Improve grammar in logs and refactor ICE credential checks
2026-04-21 15:52:08 +02:00
Viktor Liu c92e6c1b5f [client] Block on all subsystems on shutdown (#4709) 2025-11-05 12:15:37 +01:00
Zoltan Papp 9021bb512b [client] Recreate agent when receive new session id (#4564)
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.
2025-10-08 17:14:24 +02:00
Zoltan Papp 4e918e55ba [client] Fix controller re-connection (#2758)
Rethink the peer reconnection implementation
2024-10-24 11:43:14 +02:00