Skip to content

Commit 26824f4

Browse files
committed
Make tunnel-handshake startup transient instead of fatal
Symptom: 'pilotctl ping <peer>' or any first send to a peer immediately after a daemon restart could return 'send SYN: pending queue full for node X: oldest packet dropped' and abort. The user sees a hard failure even though the daemon is only 1-2 RTTs away from a working tunnel. Root cause: tunnel.go appended the new packet to the pending queue (success), then returned an error solely to surface that an older packet had been dropped to make room. DialConnection's first-SYN send-error path bailed on any error, so it never reached its 6-retry loop — the SYN it just successfully queued never got retransmitted. Changes: * pkg/daemon/tunnel.go: define typed sentinel ErrPendingDropped and return it (wrapped) from sendEncryptedToNode when the new packet is queued but the oldest had to be dropped. * pkg/daemon/daemon.go DialConnection: errors.Is(err, ErrPendingDropped) → log debug, fall through to the existing SYN retransmit loop. Treat any other Send error as fatal as before. * cmd/pilotctl/main.go: classifyDaemonError() recognizes the pending-queue and dial-timeout patterns and emits an actionable hint ("tunnel handshake in progress — retry in a few seconds" / "no reply from peer; relay can take ~30s to converge after a beacon roll") instead of the generic 'check ping' boilerplate. Wired into the connect/send-message dial sites. * pkg/beacon/server.go: drop a stray 'pi' typo on line 1 that was blocking the build (intentional gossip changes preserved).
1 parent 7d728ec commit 26824f4

3 files changed

Lines changed: 71 additions & 9 deletions

File tree

cmd/pilotctl/main.go

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,32 @@ func fatalCode(code string, format string, args ...interface{}) {
9797
os.Exit(1)
9898
}
9999

100+
// classifyDaemonError inspects an error string from the daemon and, when it
101+
// recognizes a transient or operator-friendly failure mode, returns a more
102+
// actionable hint to the user. Returns "" if no specific guidance applies —
103+
// callers fall back to whatever default hint they had.
104+
//
105+
// Currently recognized patterns:
106+
// - "pending queue full ... key exchange pending" — the encrypted tunnel
107+
// to the peer is mid-handshake. The packet was queued; waiting a moment
108+
// and retrying typically succeeds.
109+
// - "dial timeout"/"dial: daemon: dial timeout" — the SYN went out but no
110+
// SYN-ACK came back. Often means the relay path is mid-convergence
111+
// after a beacon roll, or the peer is offline.
112+
func classifyDaemonError(err error) string {
113+
if err == nil {
114+
return ""
115+
}
116+
s := err.Error()
117+
switch {
118+
case strings.Contains(s, "pending queue full") || strings.Contains(s, "key exchange pending"):
119+
return "tunnel handshake in progress — the packet was queued. Wait a few seconds and retry; the SYN will go out as soon as the encrypted session keys are derived."
120+
case strings.Contains(s, "dial timeout"):
121+
return "no reply from peer. Check reachability with `pilotctl ping <peer>`; if relay is fresh after a beacon roll it can take ~30s for endpoints to converge."
122+
}
123+
return ""
124+
}
125+
100126
// fatalHint is like fatalCode but adds an actionable hint telling the user what to do next.
101127
func fatalHint(code, hint, format string, args ...interface{}) {
102128
msg := fmt.Sprintf(format, args...)
@@ -2418,8 +2444,11 @@ func cmdConnect(args []string) {
24182444
if message != "" {
24192445
conn, err := d.DialAddr(target, port)
24202446
if err != nil {
2421-
fatalHint("connection_failed",
2422-
fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target),
2447+
hint := classifyDaemonError(err)
2448+
if hint == "" {
2449+
hint = fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target)
2450+
}
2451+
fatalHint("connection_failed", hint,
24232452
"cannot connect to %s port %d", target, port)
24242453
}
24252454
defer conn.Close()
@@ -2771,8 +2800,11 @@ func cmdSendFile(args []string) {
27712800

27722801
client, err := dataexchange.Dial(d, target)
27732802
if err != nil {
2774-
fatalHint("connection_failed",
2775-
fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target),
2803+
hint := classifyDaemonError(err)
2804+
if hint == "" {
2805+
hint = fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target)
2806+
}
2807+
fatalHint("connection_failed", hint,
27762808
"cannot connect to %s (data exchange port %d)", target, protocol.PortDataExchange)
27772809
}
27782810
defer client.Close()
@@ -2832,8 +2864,11 @@ func cmdSendMessage(args []string) {
28322864

28332865
client, err := dataexchange.Dial(d, target)
28342866
if err != nil {
2835-
fatalHint("connection_failed",
2836-
fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target),
2867+
hint := classifyDaemonError(err)
2868+
if hint == "" {
2869+
hint = fmt.Sprintf("check that %s is reachable: pilotctl ping %s", target, target)
2870+
}
2871+
fatalHint("connection_failed", hint,
28372872
"cannot connect to %s (data exchange port %d)", target, protocol.PortDataExchange)
28382873
}
28392874
defer client.Close()

pkg/daemon/daemon.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2412,8 +2412,18 @@ func (d *Daemon) dialConnectionLocked(ctx context.Context, dstAddr protocol.Addr
24122412
}
24132413

24142414
if err := d.tunnels.Send(dstAddr.Node, syn); err != nil {
2415-
d.ports.RemoveConnection(conn.ID)
2416-
return nil, fmt.Errorf("send SYN: %w", err)
2415+
// ErrPendingDropped means the SYN itself IS queued; only an older
2416+
// packet was dropped to make room. The tunnel is mid-handshake.
2417+
// Don't abort the dial — the SYN will go out as soon as the key
2418+
// exchange completes, and if not, the retry loop below will
2419+
// retransmit. Aborting here turns a transient handshake-startup
2420+
// condition into a hard "dial failed" for the user.
2421+
if !errors.Is(err, ErrPendingDropped) {
2422+
d.ports.RemoveConnection(conn.ID)
2423+
return nil, fmt.Errorf("send SYN: %w", err)
2424+
}
2425+
slog.Debug("dial: SYN queued during tunnel handshake (will retransmit)",
2426+
"peer_node_id", dstAddr.Node, "dst_port", dstPort)
24172427
}
24182428
conn.Mu.Lock()
24192429
conn.SendSeq++

pkg/daemon/tunnel.go

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,19 @@ const maxPendingPerPeer = 64
250250
// maxPendingPeers limits the total number of peers with pending key exchanges.
251251
const maxPendingPeers = 256
252252

253+
// ErrPendingDropped is returned by sendEncryptedToNode when the per-peer
254+
// pending queue was already at maxPendingPerPeer and the oldest queued
255+
// packet had to be dropped to make room for the new one. The CALLER's
256+
// packet is still queued — it will be sent as soon as key exchange
257+
// finishes — but an older packet was lost to back-pressure.
258+
//
259+
// Callers that distinguish this error from a hard failure can choose to
260+
// retry (the dial path does this; one of the SYN retransmits will land
261+
// after the queue drains). Surfacing it as a typed error also lets
262+
// pilotctl render a "tunnel handshaking" hint instead of an opaque
263+
// "send SYN: pending queue full" message.
264+
var ErrPendingDropped = errors.New("pending queue full: oldest queued packet dropped while key exchange pending")
265+
253266
// RecvChSize is the capacity of the incoming packet channel.
254267
// Increased from 1024 to 8192 for 1M-node scale to prevent drops during
255268
// bursts (e.g., many peers sending simultaneously after a cron trigger).
@@ -1808,7 +1821,11 @@ func (tm *TunnelManager) SendTo(addr *net.UDPAddr, nodeID uint32, pkt *protocol.
18081821
"peer_node_id", nodeID,
18091822
"queue_len", qlen,
18101823
"limit", maxPendingPerPeer)
1811-
return fmt.Errorf("pending queue full for node %d: oldest packet dropped", nodeID)
1824+
// The new packet IS queued (line above appended it). What was
1825+
// dropped is the oldest packet, not this one. Callers that
1826+
// errors.Is(err, ErrPendingDropped) can treat this as transient
1827+
// — a SYN retransmit will succeed once the queue drains.
1828+
return fmt.Errorf("%w (peer_node_id=%d)", ErrPendingDropped, nodeID)
18121829
}
18131830
return nil // queued, will be sent encrypted after key exchange
18141831
}

0 commit comments

Comments
 (0)