[PR #4545] [client, signal] Refactor/reduce signaling #20641

New Issue

saavagebueno · 2026-08-05T05:07:31-04:00

saavagebueno commented

2026-08-05 05:07:31 -04:00

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/4545
Author: @pappz
Created: 9/25/2025
Status: 🔄 Open

Base: main ← Head: refactor/reducate-signaling

📝 Commits (10+)

56c67fd Remove retry logic for P2P connection
52d8bdf Merge branch 'main' into refactor/reducate-signaling
0753766 Add signal error handling
4d46adb Add wantDeliveryError option for signal proto
71733df Add error code handling
7ba8b92 Fix offer error sending channel
9184a0c Update logs and doc
d9c585f allow ACK messages on signal
2e20c97 Handle unimplemented method
48e45b6 Resend msg

📊 Changes

20 files changed (+557 additions, -159 deletions)

View changed files

📝 client/cmd/testutil_test.go (+1 -1)
📝 client/internal/engine_test.go (+1 -1)
📝 client/internal/peer/conn.go (+53 -3)
➕ client/internal/peer/guard.go (+10 -0)
📝 client/internal/peer/guard/guard.go (+40 -59)
➕ client/internal/peer/guard/guard_retry.go (+139 -0)
📝 client/internal/peer/signaler.go (+35 -2)
📝 client/server/server_test.go (+1 -1)
📝 go.mod (+1 -1)
📝 go.sum (+2 -2)
📝 shared/signal/client/client.go (+1 -0)
📝 shared/signal/client/client_test.go (+1 -1)
📝 shared/signal/client/grpc.go (+36 -0)
📝 shared/signal/client/mock.go (+8 -0)
📝 shared/signal/proto/signalexchange.pb.go (+80 -69)
📝 shared/signal/proto/signalexchange.proto (+2 -0)
📝 shared/signal/proto/signalexchange_grpc.pb.go (+37 -0)
📝 signal/cmd/env.go (+14 -0)
📝 signal/cmd/run.go (+10 -3)
📝 signal/server/signal.go (+85 -16)

📄 Description

Describe your changes

When the client sends a signaling offer or answer, the Signal server may return an error if the message cannot be delivered. The client then checks the error type to decide how to proceed:

If the error suggests the remote peer is unavailable, the client waits for the peer instead of retrying immediately.
If the method is not implemented, then switch back to the original retry mechanism (backward compatibility case)
If the error indicates another error, the client retries by sending a new offer.

Whenever a peer reconnects to the signaling server and no active peer connection exists, it must send a new offer. This ensures that any signaling messages lost while the peer was offline are recovered and the connection can be re-established.

The NB_DISABLE_SEND_WITH_DELIVERY_CHECK: true|false option can be used to enforce that clients always use the server’s original methods.

For the review, keep in mind that these Signal server changes must support both old and new clients, and old clients must still connect properly with each other.

Issue ticket number and link

Stack

Checklist

Is it a bug fix
Is a typo/documentation fix
Is a feature enhancement
It is a refactor
Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

I added/updated documentation for this change
Documentation is not needed for this change (explain why)

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/__

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/4545 **Author:** [@pappz](https://github.com/pappz) **Created:** 9/25/2025 **Status:** 🔄 Open **Base:** `main` ← **Head:** `refactor/reducate-signaling` --- ### 📝 Commits (10+) - [`56c67fd`](https://github.com/netbirdio/netbird/commit/56c67fdf0819fdea9de00d40b6f9dd289f40c501) Remove retry logic for P2P connection - [`52d8bdf`](https://github.com/netbirdio/netbird/commit/52d8bdfc78a532727dc1efa64e55aafed0adb36b) Merge branch 'main' into refactor/reducate-signaling - [`0753766`](https://github.com/netbirdio/netbird/commit/07537663362fa031d9f22420754b1105f0a7eee0) Add signal error handling - [`4d46adb`](https://github.com/netbirdio/netbird/commit/4d46adbb6819e74371858eac6b219808a2239ff0) Add wantDeliveryError option for signal proto - [`71733df`](https://github.com/netbirdio/netbird/commit/71733dff3e02bc0ecccf0e021d9072921e249115) Add error code handling - [`7ba8b92`](https://github.com/netbirdio/netbird/commit/7ba8b926f0875e290d531b82b02504e1deef5255) Fix offer error sending channel - [`9184a0c`](https://github.com/netbirdio/netbird/commit/9184a0c6ac13f2c73cd31f712239e28a26b93ed2) Update logs and doc - [`d9c585f`](https://github.com/netbirdio/netbird/commit/d9c585f575a5b2545493f43447ce107094f8d821) allow ACK messages on signal - [`2e20c97`](https://github.com/netbirdio/netbird/commit/2e20c978b38ebe914b35a66d3cf4975faa7326f0) Handle unimplemented method - [`48e45b6`](https://github.com/netbirdio/netbird/commit/48e45b64bb6ac88226977383a908175ce71f22e5) Resend msg ### 📊 Changes **20 files changed** (+557 additions, -159 deletions) <details> <summary>View changed files</summary> 📝 `client/cmd/testutil_test.go` (+1 -1) 📝 `client/internal/engine_test.go` (+1 -1) 📝 `client/internal/peer/conn.go` (+53 -3) ➕ `client/internal/peer/guard.go` (+10 -0) 📝 `client/internal/peer/guard/guard.go` (+40 -59) ➕ `client/internal/peer/guard/guard_retry.go` (+139 -0) 📝 `client/internal/peer/signaler.go` (+35 -2) 📝 `client/server/server_test.go` (+1 -1) 📝 `go.mod` (+1 -1) 📝 `go.sum` (+2 -2) 📝 `shared/signal/client/client.go` (+1 -0) 📝 `shared/signal/client/client_test.go` (+1 -1) 📝 `shared/signal/client/grpc.go` (+36 -0) 📝 `shared/signal/client/mock.go` (+8 -0) 📝 `shared/signal/proto/signalexchange.pb.go` (+80 -69) 📝 `shared/signal/proto/signalexchange.proto` (+2 -0) 📝 `shared/signal/proto/signalexchange_grpc.pb.go` (+37 -0) 📝 `signal/cmd/env.go` (+14 -0) 📝 `signal/cmd/run.go` (+10 -3) 📝 `signal/server/signal.go` (+85 -16) </details> ### 📄 Description ## Describe your changes When the client sends a signaling offer or answer, the Signal server may return an error if the message cannot be delivered. The client then checks the error type to decide how to proceed: - If the error suggests the remote peer is unavailable, the client waits for the peer instead of retrying immediately. - If the method is not implemented, then switch back to the original retry mechanism (backward compatibility case) - If the error indicates another error, the client retries by sending a new offer. Whenever a peer reconnects to the signaling server and no active peer connection exists, it must send a new offer. This ensures that any signaling messages lost while the peer was offline are recovered and the connection can be re-established. The NB_DISABLE_SEND_WITH_DELIVERY_CHECK: true|false option can be used to enforce that clients always use the server’s original methods. For the review, keep in mind that these Signal server changes must support both old and new clients, and old clients must still connect properly with each other. ## Issue ticket number and link ## Stack  ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [x] It is a refactor - [ ] Created tests that fail without the change (if possible) > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### 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/__ --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

saavagebueno added the pull-request label 2026-08-05 05:07:31 -04:00

Sign in to join this conversation.

Branches Tags

main

fix/wails-go-vendor

fix/update-wails-fork-ref

add-atomic-cache-ops

feat/agent-network-per-account-serving

feat-post_quantum_ml_kem

revert/component-types

wins_nrpt_dns_delegation

embedded-vnc

dependabot/github_actions/actions-a940c7c866

fix/quickstart-subnet-and-domain-alias

notification-localization

ssh-settings-elevation

refactor/peer-event-bus

fix/relay-proxy-redirect-when-ice-active

disambiguate_p2p_metrics

claude/agent-network-test-cases-p743vw

android/gui-integration

ice-stun-wg-demux

dependabot/npm_and_yarn/proxy/web/npm_and_yarn-b39864987c

agent-network-setup-poc

dependabot/go_modules/aws-sdk-8f849ebaed

dependabot/go_modules/otel-e34c790afd

dependabot/go_modules/testcontainers-de325c0dd6

dependabot/go_modules/wireguard-dbd6b95108

dependabot/go_modules/pion-5f703e1eca

dependabot/go_modules/gorm-2271c8195b

fix-login-needed-check

dependabot/go_modules/google.golang.org/grpc-1.82.1

revert/component-types-hookup

feature/ios-ssh

docs/agent-network-docs-update

dependabot/go_modules/github.com/aws/aws-sdk-go-v2/service/s3-1.106.3

dependabot/go_modules/github.com/pion/stun/v3-3.1.5

fix-ssh-authorized-users-multi-rule

peer-acl-multi-source

reverse-proxy-crowdsec-appsec

reverse-proxy-allow-match-or

client-local-metrics

lazy-conn-per-peer

lazy-conn-rosenpass

dependabot/go_modules/github.com/gopacket/gopacket-1.7.0

dependabot/go_modules/goauthentik.io/api/v3-3.2026050.6

dependabot/go_modules/github.com/pires/go-proxyproto-0.15.0

dependabot/go_modules/github.com/jackc/pgx/v5-5.10.0

dependabot/go_modules/github.com/oapi-codegen/runtime-1.6.0

dependabot/go_modules/github.com/eko/gocache/lib/v4-4.2.4

dependabot/go_modules/github.com/pkg/sftp-1.13.11

dependabot/go_modules/github.com/coreos/go-oidc/v3-3.20.0

ssh-windows-privilege-check

fix/explicit-cors-handling

fix/remove-math-rand

test/gui-memory-leak-fix

fix/ui-status-dispatch

install-script-ui-dependencies

fix/grpc-get-network-map

fix/subscribe-status-coalesce

fix/tray-menu-item-leak

fix/windows-tray-race

feature/changeset

worktree-dns-route-qtype-fallthrough

mdm_integration

mlsmaycon-patch-2

feat/agent-network-ollama

proxy-tunnel-cache-ttl-env

coderabbitai/utg/1e5b0a5

grpc-acl

test/battery-drain

components-impl-drop-indexes-use-xids-no-resource-policy-map

fix/nmap-relevant-groups

e2e-guardrail-blocks-unselected-model

fix/lazyconn-cold-start-allowed-ips

vertex-guardrails-model-access-e2e

refactor/relay-foreign-cache

ci/trigger-release-tests

feature/kimi-3-agent-networks-dns-warmup

feature/dns-lazy-conn-warmup

daemon-ipc-acl

feature/ui-translation-key-parity-check

refactor/relay-foreign-cache-tests

fix/lazyconn-idle-keep-wg-peer

dmitri-propagate-auth-grant-types-on-combined

0.74.7-branch

diagnose-empty-vs-corrupt-state

windows-sleep-detector

fix/cli-up-wait-for-daemon

rp_key_persistency

feature/native-grpc

0.74.6-branch

0.74.6-branch-sync

0.74.5-branch-sync

0.74.4-branch

fix/remove-stale-peers-removal

fix/remove-stale-proxy-logic

fix/nsis-preserve-autostart-on-upgrade

dependabot/go_modules/goauthentik.io/api/v3-3.2026050.3

dependabot/go_modules/github.com/pkg/sftp-1.13.10

components-impl-drop-indexes

fix-reset-aggregation-window-flake

dependabot/go_modules/github.com/pion/dtls/v3-3.1.5

0.74.x

fix/relay-states-lock

update-process-pkg

update-gopsutil-v4

fix/relay_states_lock

increase-sysinfo-timeout

dependabot/go_modules/github.com/Azure/go-ntlmssp-0.1.1

dependabot/go_modules/github.com/eko/gocache/store/redis/v4-4.2.6

dependabot/go_modules/github.com/eko/gocache/lib/v4-4.2.3

fix/fail-to-create-upnp-port-mapping-on-opnsense-firewall

0.74.3-branch

fix/routeselector-atomic-exit-node

netmap_progressive_alignment

nmap/components-impl

dependabot/go_modules/github.com/jackc/pgx/v5-5.9.2

dependabot/go_modules/github.com/oapi-codegen/runtime-1.4.2

dependabot/go_modules/github.com/gopacket/gopacket-1.6.1

dependabot/go_modules/github.com/coreos/go-oidc/v3-3.19.0

dependabot/go_modules/github.com/pires/go-proxyproto-0.12.0

fix/signal-watchdog-sync-stop

docs/agent-network

test/affected-logic

fix/revert-ice-filter

refactor/simplify-affected-peers

pascal-filter-policies-by-direction

claude/lock-contention-peer-connect-g8t6au

dmitri-filter-policies-by-direction

refactor/migrate-profiles-to-go

profile-bindings-ios

fix/skip-restart-unchanged-route

fix/mgmt-cache-async-resolve

refactor/wails-update-105

client_lifetime_serialization_refactor

fix/browser-ssh-2

fix/ipv6-and-netstack-accept-loop

fix/browser-ssh

profile-id-name-test

refactor/mgmt-bootstrap

feat/getting-started-unified-wizard

socket-grpc-permissions

fix/mysql-index-migration

windows-dns-firewall

tests/enable-race-on-tests

ui-refactor-gtk3

feature/affected-peers-grpc

profile-id

lazyconn-first-packet-fix-v2

claude/focused-gates-VMTgb

ui-tray-linux-leftclick

fix/ctx-enrichment

daemon-owner

feature/android-client-ssh

worktree-accept-ra-forwarding

nmap/combined-deploy

task/align_protobuff_toolset

feature/session-extend

add-json-yaml-flags

refactor/ephemeral-cleanup

claude/webtransport-relay-wasm-mUjY9

claude/vnc-udp-feasibility-6KB1U

fix/wgport-config

e2e-windows-dns-combined

fix/login-cmd-root-flags

feat/reseller-openapi-spec

github-issue-resolver

add-steamos-support

fix-darwin-uninstaller

flutter-test

ci/freebsd-pkg-bootstrap

cached-serial-check-on-sync

fix-mgmt-cache-bypass-overlay

revert-easyjson-5938

revert-ice-5820

revert-firewalld-5928

refactor/permissions-manager

revert-dns-5935-systemd-resolved

revert-dns-5935-5945

revert-dns-5945-mgmt-cache

feature/log-most-busy-peers

prototype/ui-wails

coderabbitai/utg/8ae8f20

feature/use-peer-fqdn-on-https

release/0.68.3

add-slack-channel

claude/rdp-token-passthrough-eNcqW

transparent-proxy

fix/macos-stale-route-eexist

crowdsec-selfhosted

fix/remove-otel-units

entire/checkpoints/v1

fix/getting-started

feat/static-connectors-combined-server

feature/use-local-keys-embedded

feature/fleetdm

set-env-only-if-not-fork

feature/expose-has-channel

fix/connection-status-race

fix/filter-cgnat-cni-ice-candidates

feature/check-cert-locker-before-acme

test/proxy-fixes

test/proxy-mtu

prototype/ui-tauri

test/proxy-speed

fix-reused-ports

feat/migrate-to-embedded-idp

feature/add-serial-to-proxy-merged

deploy/proxy-serial

test/connection

feature/disable-legacy-port

feature/flag-to-disable-legacy-port

test/perftest

fix/http-redirect

poc-token-command

dn-reverse-proxy

prototype/reverse-proxy-rename

prototype/reverse-proxy-logs-pagination

feature/client-metrics

prototype/reverse-proxy-clusters

debug-dns-route

fix/win-dns-batch

add-extra-route-logs

job-stream-notify-disconnection-eof

deploy/secrets-manager

trigger-proxy-update

bug/update-ios-client-code-build-tags

sync-client-netmap-serial

log/conn-disconn

nmap/compaction-deploy

ci-win-test

feature/disk-encryption-check

wasm-debug

swap-dns-prio

fix/dex-config

feature/migrate-auto-groups-to-table

nmap/compaction

dex-nocgo-stub

feature/exclude-terraform-from-rate-limiting

test-freebsd

retries-refactor

coderabbitai/docstrings/b7e98ac

feat/integrate-zitadel

bug/ios-hanging-reconection

zitadel-idp

feat/network-map-serial

refactor/get-account-no-users

feat/auto-upgrade

feature/report-high-pat-id

feature/temporary-access-for-resource

fix/nmap-fwrules

dont-restart-dns

prototype/ui

update-gomobile

go-dns-for-ice

wasm-ldflags

test-ldflags

wasmbuild-test

feature/networks-s2s

vk/compare-nmaps

dbg/bothmaps

reorder-dns-shutdown

fix/relay-reconnection-race

fix/nmap-exitnodes

vk/debug/nmap-both

move-licensed-code

feat/better-daemon-connection-lost-message

feat/auto-update-2

test/timings

refactor/getaccount-raw

tests/nmap-getaccount

refactor/nmap

refactor/nmap-limit-buffer

feature/detect-mac-wakeup

feature/extract-modules

quick-setings

feat/sync-limiter

feature/store-cache-impl

fix-install-version

feature/store-metrics

feature/metrics-on-store

feature/use-gorm-cache

loadtest-signal

unsymmetrical-squash

refactor/reducate-signaling

test/update-reduce

feature/store-cache

feature/remote-debug

cli-ws-proxy-backend-addr

feat/mgmt-map-serial

snyk-fix-d9d0081a4c7f9137bdb59d0d50a141a2

snyk-fix-7415cea5a11acd66753540ca2c598c63

job-yml-update

feature/android-allow-selecting-routes

fix/up-sequence

fix/dns-hash-update

snyk-fix-967adae9863f17f108ce8948d9117b8d

log/getaccount-by-peer

signal-suppressor

dns-exit-node

feature/auto-updates

feature/cache-srv-key

merged-fixes

fix/missed-offers-and-debug

debug-and-fixes

poc-wasm-clean-backend-s2s

test/remote-debug

debug-api

fix/remove-gpo-if-empty

fix/test-freebsd

fix/mysql-setup

fix/remove-logout-btn

handle-existing-domain-user

chore/unify-domain-validation

snyk-fix-c5fafc8a50ce1f29046e25a1fc346185

feat/profile-edit-btn

snyk-fix-a54966211e18d4cf67e5a2757cc006d1

log-short-id

feat/logout-ephemeral

log-checks

batch-wg-ops

nb-interface-default

feat/aws-integration

add/race-test

feature/relay-feature-versioning

fix/systemd-service-logs

poc/preprocessed-map

add-account-onboarding

bind-ipv6

fix/merge-main

logs/peerlogs-addpeer

feature/net-297-network-migration

feature/support-skip-auto-apply-exit-node-routes

set-cmd

set-command-with-cursor

feature/limit-update-channel

stop-using-locking-share

feature/poc-lazy-detection

feature/net-248-removal-of-sync-mutex-locks

test/multiple-peer-logging

preresolve

add-ns-punnycode-support

apply-routes-early

windows-search-domains

fix/connecting-route-filter

feature/management/rest-client/impersonate

debug-local-records

resource-fields-snake-case

test/grpc-rate-limit

traffic-correlation-policy

feature/rest-client-options

feat/events-metrics

feature/buf-cli

test/add-ratelimiter

test/remove-write-lock-on-add-peer

fix/add-peer-semaphore

feature/users-roles-endpoint

mlsmaycon-patch-1

debug-user-role

chore/primary-key-on-networks

feature/update-account-peers-buffer-startup

remove-ubuntu2004-runners

refactor/permissions-no-pat-allowed

ref/logrus-factory

use-conntrack-zone

deploy/permissions-account

feature/lazy-connection-idle

ref/improve-test-cov

restore-pr-3440

test/increase-grpc-timeouts

feat/buffer-account-peers-update

test/networkmapgeneration-changes

feature/base-manager

feature/flow-receiver

chore/benchmark-with-large-runner

refactor/handshake-initiator

client/ui-update-systray-icons

userspace-router

wgwatcher-test

output-if-key-already-exists

fix/relay-reconnection

feature/port-forwarding-client-codecleaning

detached2

test/callbacks-nil-iceconninfo

refactor/optimize-peer-expiration

enable-udp-port-for-docker-template

fix/relay-update

feature/apply-posture-netmap

fix/group-update-existing-resource

conntrack-stats

upgrade-okta-sdk

multi-price

test/conn-stat

set-min-parallel-tests-for-management

dns-interceptor

debug-dns

router-dns

add-static-system-info

debug-0.29.4

debug-0.33.0

account-refactoring

relay/2800_quic

route-get-account-refactoring

test/seed-random-routes

feature/get-account-refactoring

test/reconnect-race-condition

refactor/get-account-usage

feature/add-session-id-to-update-channel

improve-ipv4conn

fix/async-pion-event-handling

debug

add-offload

feature/validate-group-association-debug

fix/limit-conn-for-sqlite

test/engine-iface

test/transaction-for-jwt-sync

fix/engine-stop-in-foreground

feature/add-mysql-support

test-migration

refactor/header-size-values

relay/eliminate-gob

test/signal-dispatcher-with-relay

relay/debug

validate-icon

feature/ipv6-support

use-pre-expanded-peers-map

feature/use-signal-dispatcher

validate/peer-status

add-read-write-times

fix/sync-peer-race

feature/relay-status

netmap

evaluate/network-map-hash

fix/lower-dns-resolve-interval-on-fail

feature/relay

fix/go-mod-version

upgrade-nftables

synology-userspace-mode

fix/use-ip-for-default-routes-on-darwin

fix/proxy_close

enable-release-workflow-on-pr

deploy/peer-performance

feature/permanent-turn

feature/permanent-turn-proxy

deploy/posture-check-sqlite

feature/optimize_sqlite_save

debug-ios-behavior

fix/delete-route-only-after-adding

tshoot/windows-logger

remove-new-routing

refactor/eliminate-repo-dependency

add-arm-to-ci

refactor-demo-account-object

test/abc2

test/abc

send-ssh-rosenpass-config-meta

refactor-demo

ensure-schedule-never-runs-non-positive

feature/peer-validator-groupmgm

feature/peer-validator-fix

fix/include-active-dashboard-users

fix/handle-canceling-schedule

fix/geo-download

debug-google-workspace

yury/resolve-ip-to-location

feature/extend-sysinfo

sqlite-async-peer-status

yury/add-postgresql-store

fix/route

test-build

posture-checks-poc

debug-keycloak-idp

poc/netstack

for-pascal-tmp

peer-logout-management

manual-peer-logout

detached

chore/refactor-management

test/dns-bind

fix/enforce-acl-for-containers

yury/use-sync-map-in-updatechannel

fix/events-key-handling

filter-cache-on-load-account

fix/user-expiration

handle-user-context-cancellation

nb-client-k8s-statefulset

fake-addr

fix/iptables_in_docker

ebpf-debug

update-getting-started-flow-use-postgres

fix/peer_list_notification

feature/device-authentication-with-client-secret

feature/keep_alive

feat-groups-from-jwt

separate_proxy_from_wgconfig

fix/wg_conn

wg_conn_fix

wg_bind_parallel_processing

fix-rollback-get-acls

proxy_cfg_cleanup

performance-improvement-rego

update-lock-log-level

feat-client-side-acl

refactor/move_grpcserver_logic_to_account_manager

feature/event-storage

feature/update-idp-redeeming-invite

feature/api-peer-info

return-groupminimum-setupkey

feature/interface-bind

documentation_enhancement

fix-peer-registration

ssh

users_cache

pass-client-caller

client_caller_type

revert-283-feat-fix-windows-installer

periodic-peer-updates

ebpf

braginini/wasm

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: DYNR/netbird#20641