Skip to content

Commit

Permalink
add panic recovery and detailed logging in peer update comparison
Browse files Browse the repository at this point in the history 
Signed-off-by: bcmmbaga <bethuelmbaga12@gmail.com>
  • Loading branch information
@bcmmbaga
bcmmbaga committed Oct 22, 2024
1 parent abdba6c commit 74e2986
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 18 deletions.
12 changes: 10 additions & 2 deletions management/server/updatechannel.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"context"
"fmt"
"runtime/debug"
"sync"
"time"

Expand Down Expand Up @@ -207,7 +208,7 @@ func (p *PeersUpdateManager) handlePeerMessageUpdate(ctx context.Context, peerID
p.channelsMux.RUnlock()

if lastSentUpdate != nil {
updated, err := isNewPeerUpdateMessage(lastSentUpdate, update)
updated, err := isNewPeerUpdateMessage(ctx, lastSentUpdate, update)
if err != nil {
log.WithContext(ctx).Errorf("error checking for SyncResponse updates: %v", err)
return false
Expand All @@ -222,7 +223,14 @@ func (p *PeersUpdateManager) handlePeerMessageUpdate(ctx context.Context, peerID
}

// isNewPeerUpdateMessage checks if the given current update message is a new update that should be sent.
func isNewPeerUpdateMessage(lastSentUpdate, currUpdateToSend *UpdateMessage) (bool, error) {
func isNewPeerUpdateMessage(ctx context.Context, lastSentUpdate, currUpdateToSend *UpdateMessage) (isNew bool, err error) {
defer func() {
if r := recover(); r != nil {
log.WithContext(ctx).Panicf("comparing peer update messages. Trace: %s", debug.Stack())
isNew, err = true, nil
}
}()

if lastSentUpdate.Update.NetworkMap.GetSerial() > currUpdateToSend.Update.NetworkMap.GetSerial() {
return false, nil
}
Expand Down
32 changes: 16 additions & 16 deletions management/server/updatechannel_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage1 := createMockUpdateMessage(t)
newUpdateMessage2 := createMockUpdateMessage(t)

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.False(t, message)
})
Expand All @@ -200,7 +200,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {

newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.False(t, message)
})
Expand All @@ -212,7 +212,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Routes[0].Network = netip.MustParsePrefix("1.1.1.1/32")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -225,7 +225,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Routes[0].Groups = []string{"randomGroup1"}
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -244,7 +244,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Peers = append(newUpdateMessage2.NetworkMap.Peers, newPeer)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -254,14 +254,14 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {

newUpdateMessage2 := createMockUpdateMessage(t)
newUpdateMessage2.Update.NetworkMap.Serial++
message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.False(t, message)

newUpdateMessage3 := createMockUpdateMessage(t)
newUpdateMessage3.Update.Checks = []*proto.Checks{}
newUpdateMessage3.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage3)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage3)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -280,7 +280,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
}
newUpdateMessage4.Update.Checks = []*proto.Checks{toProtocolCheck(check)}
newUpdateMessage4.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage4)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage4)
assert.NoError(t, err)
assert.True(t, message)

Expand All @@ -300,7 +300,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
}
newUpdateMessage5.Update.Checks = []*proto.Checks{toProtocolCheck(check)}
newUpdateMessage5.Update.NetworkMap.Serial++
message, err = isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage5)
message, err = isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage5)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -316,7 +316,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -328,7 +328,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.Peers[0].IP = net.ParseIP("192.168.1.10")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -340,7 +340,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.FirewallRules[0].Port = "443"
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -359,7 +359,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.FirewallRules = append(newUpdateMessage2.NetworkMap.FirewallRules, newRule)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -371,7 +371,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.NameServerGroups[0].NameServers = make([]nbdns.NameServer, 0)
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -383,7 +383,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.NameServerGroups[0].NameServers[0].IP = netip.MustParseAddr("8.8.4.4")
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand All @@ -395,7 +395,7 @@ func TestIsNewPeerUpdateMessage(t *testing.T) {
newUpdateMessage2.NetworkMap.DNSConfig.CustomZones[0].Records[0].RData = "100.64.0.2"
newUpdateMessage2.Update.NetworkMap.Serial++

message, err := isNewPeerUpdateMessage(newUpdateMessage1, newUpdateMessage2)
message, err := isNewPeerUpdateMessage(context.Background(), newUpdateMessage1, newUpdateMessage2)
assert.NoError(t, err)
assert.True(t, message)
})
Expand Down

0 comments on commit 74e2986

Please sign in to comment.