From 71733dff3e02bc0ecccf0e021d9072921e249115 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Thu, 25 Sep 2025 12:46:05 +0200 Subject: [PATCH] Add error code handling --- client/internal/peer/conn.go | 7 ++++++- client/internal/peer/signaler.go | 7 +++++++ shared/signal/client/grpc.go | 16 ++++++++++++++++ signal/server/signal.go | 8 ++++++-- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/client/internal/peer/conn.go b/client/internal/peer/conn.go index 37a54d1da..284438689 100644 --- a/client/internal/peer/conn.go +++ b/client/internal/peer/conn.go @@ -2,6 +2,7 @@ package peer import ( "context" + "errors" "fmt" "math/rand" "net" @@ -209,7 +210,11 @@ func (conn *Conn) Open(engineCtx context.Context) error { // both peer send offer if err := conn.handshaker.SendOffer(); err != nil { conn.Log.Errorf("failed to send offer: %v", err) - conn.guard.FailedToSendOffer() + // if remote peer is offline, no need to try to reconnect. + // The remote peer when online will send an offer to us + if !errors.Is(err, ErrPeerNotAvailable) { + conn.guard.FailedToSendOffer() + } } conn.opened = true diff --git a/client/internal/peer/signaler.go b/client/internal/peer/signaler.go index d3e892a0a..58be304cc 100644 --- a/client/internal/peer/signaler.go +++ b/client/internal/peer/signaler.go @@ -1,6 +1,8 @@ package peer import ( + "errors" + "github.com/pion/ice/v4" log "github.com/sirupsen/logrus" "golang.zx2c4.com/wireguard/wgctrl/wgtypes" @@ -9,6 +11,8 @@ import ( sProto "github.com/netbirdio/netbird/shared/signal/proto" ) +var ErrPeerNotAvailable = signal.ErrPeerNotAvailable + type Signaler struct { signal signal.Client wgPrivateKey wgtypes.Key @@ -68,6 +72,9 @@ func (s *Signaler) signalOfferAnswer(offerAnswer OfferAnswer, remoteKey string, } if err = s.signal.SendWithDeliveryCheck(msg); err != nil { + if errors.Is(err, signal.ErrPeerNotAvailable) { + return ErrPeerNotAvailable + } return err } diff --git a/shared/signal/client/grpc.go b/shared/signal/client/grpc.go index 3909af5ca..eeb7f9c0c 100644 --- a/shared/signal/client/grpc.go +++ b/shared/signal/client/grpc.go @@ -2,6 +2,7 @@ package client import ( "context" + "errors" "fmt" "io" "sync" @@ -22,6 +23,10 @@ import ( "github.com/netbirdio/netbird/shared/signal/proto" ) +var ( + ErrPeerNotAvailable = errors.New("peer not available") +) + // ConnStateNotifier is a wrapper interface of the status recorder type ConnStateNotifier interface { MarkSignalDisconnected(error) @@ -410,6 +415,17 @@ func (c *GrpcClient) SendWithDeliveryCheck(msg *proto.Message) error { defer cancel() _, err = c.realClient.SendWithDeliveryCheck(ctx, encryptedMessage) + if err != nil { + if st, ok := status.FromError(err); ok { + switch st.Code() { + case codes.NotFound: + return ErrPeerNotAvailable + default: + return fmt.Errorf("grpc error %s: %w", st.Code(), err) + } + } + return err // Not a gRPC status error + } return err } diff --git a/signal/server/signal.go b/signal/server/signal.go index 4ab57666f..aa92b53d3 100644 --- a/signal/server/signal.go +++ b/signal/server/signal.go @@ -105,6 +105,11 @@ func (s *Server) Send(ctx context.Context, msg *proto.EncryptedMessage) (*proto. } // SendWithDeliveryCheck forwards a message to the signal peer with error handling +// When the remote peer is not connected it returns codes.NotFound error, otherwise it returns other types of errors +// that can be retried. In case codes.NotFound is returned the caller should not retry sending the message. The remote +// peer should send a new offer to re-establish the connection when it comes back online. +// Todo: double check the thread safe registry management. When both peer come online at the same time then both peers +// might not be registered yet when the first message is sent. func (s *Server) SendWithDeliveryCheck(ctx context.Context, msg *proto.EncryptedMessage) (*emptypb.Empty, error) { log.Tracef("received a new message to send from peer [%s] to peer [%s]", msg.Key, msg.RemoteKey) @@ -113,8 +118,7 @@ func (s *Server) SendWithDeliveryCheck(ctx context.Context, msg *proto.Encrypted s.forwardMessageToPeer(ctx, msg) return &emptypb.Empty{}, nil } - - return nil, status.Errorf(codes.FailedPrecondition, "remote peer not connected") + return nil, status.Errorf(codes.NotFound, "remote peer not connected") //return s.dispatcher.SendMessage(ctx, msg) }