Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 72 additions & 0 deletions pkg/puller/error_summary_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2025 The Swarm Authors. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2025 -> 2026

// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package puller

import (
"errors"
"testing"
)

func TestCountErrors(t *testing.T) {
t.Parallel()

t.Run("nil error", func(t *testing.T) {
result := countErrors(nil)
if result != 0 {
t.Errorf("expected 0 for nil error, got %d", result)
}
})

t.Run("single error", func(t *testing.T) {
err := errors.New("test error")
result := countErrors(err)
if result != 1 {
t.Errorf("expected 1, got %d", result)
}
})

t.Run("two different errors", func(t *testing.T) {
err1 := errors.New("error one")
err2 := errors.New("error two")
joined := errors.Join(err1, err2)
result := countErrors(joined)

if result != 2 {
t.Errorf("expected 2, got %d", result)
}
})

t.Run("many identical errors", func(t *testing.T) {
baseErr := errors.New("storage: not found")
var joined error
for range 100 {
joined = errors.Join(joined, baseErr)
}

result := countErrors(joined)

if result != 100 {
t.Errorf("expected 100, got %d", result)
}
})

t.Run("mixed repeated errors", func(t *testing.T) {
err1 := errors.New("storage: not found")
err2 := errors.New("invalid stamp")
var joined error
for range 50 {
joined = errors.Join(joined, err1)
}
for range 30 {
joined = errors.Join(joined, err2)
}

result := countErrors(joined)

if result != 80 {
t.Errorf("expected 80, got %d", result)
}
})
}
40 changes: 39 additions & 1 deletion pkg/puller/puller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,39 @@ const loggerName = "puller"

var errCursorsLength = errors.New("cursors length mismatch")

// countErrors counts the total number of errors in a joined error.
// This prevents massive log lines when many errors are joined together.
func countErrors(err error) int {
if err == nil {
return 0
}

count := 0
var walkErrors func(error)
walkErrors = func(e error) {
if e == nil {
return
}

// Check if this is a joined error (has multiple wrapped errors)
type unwrapper interface {
Unwrap() []error
}
if u, ok := e.(unwrapper); ok {
for _, wrapped := range u.Unwrap() {
walkErrors(wrapped)
}
return
}

// This is a leaf error
count++
}

walkErrors(err)
return count
}

const (
DefaultHistRateWindow = time.Minute * 15

Expand Down Expand Up @@ -348,7 +381,12 @@ func (p *Puller) syncPeerBin(parentCtx context.Context, peer *syncPeer, bin uint
p.logger.Debug("syncWorker interval failed, quitting", "error", err, "peer_address", address, "bin", bin, "cursor", cursor, "start", start, "topmost", top)
return
}
p.logger.Debug("syncWorker interval failed", "error", err, "peer_address", address, "bin", bin, "cursor", cursor, "start", start, "topmost", top)
errCount := countErrors(err)
if errCount > 1 {
p.logger.Debug("syncWorker interval failed", "error_count", errCount, "example_error", errors.Unwrap(err), "peer_address", address, "bin", bin, "cursor", cursor, "start", start, "topmost", top)
} else {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.Unwrap() looks for Unwrap() error, but errors.Join() implements Unwrap() []error.
Would this return nil here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand the question?

By the way @gacevicljubisa, maybe it is just better to remove the if statement and log both ways with the error_count field? Not sure if such granularity is needed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I didn't find a way to test and see errors locally in puller logs, so I created this unit test in error_summary_test.go to validate it but i got the issue that i meantioned.

t.Run("errors.Unwrap returns nil for joined errors", func(t *testing.T) {
		err1 := errors.New("storage: not found")
		err2 := errors.New("storage: not found")
		joined := errors.Join(err1, err2)
		
		unwrapped := errors.Unwrap(joined)
		if unwrapped == nil {
			t.Errorf("expected not nil, got %v", unwrapped)
		}
	})

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.Join() creates an error with Unwrap() []error but errors.Unwrap() looks for Unwrap() error.
Since the signatures don't match, errors.Unwrap() returns nil.

p.logger.Debug("syncWorker interval failed", "error", err, "peer_address", address, "bin", bin, "cursor", cursor, "start", start, "topmost", top)
}
}

_ = p.limiter.WaitN(ctx, count)
Expand Down
Loading