Skip to content

Commit 9d4a58c

Browse files
authored
fix: allow list group initialization with partial source failures (#1889)
* fix: allow list group initialization with partial source failures When multiple list sources are configured for a blocking group, a transient error (e.g., timeout) on one source no longer fails the entire group if other sources loaded successfully. Previously, any TransientError would cause the entire group to fail initialization, even when other sources in the group had successfully loaded entries. This left users without any DNS blocking functionality. Now, groups succeed if at least one source loads successfully. Failed sources will be retried on the next refresh cycle, maintaining blocking functionality with available lists. Fixes #1825 * fix: race condition * fix lint error
1 parent 1a98cda commit 9d4a58c

File tree

3 files changed

+74
-17
lines changed

3 files changed

+74
-17
lines changed

helpertest/mock_call_sequence.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"fmt"
66
"sync"
7+
"sync/atomic"
78
"time"
89
)
910

@@ -13,7 +14,7 @@ type MockCallSequence[T any] struct {
1314
driver func(chan<- T, chan<- error)
1415
res chan T
1516
err chan error
16-
callCount uint
17+
callCount atomic.Uint32
1718
initOnce sync.Once
1819
closeOnce sync.Once
1920
}
@@ -25,7 +26,7 @@ func NewMockCallSequence[T any](driver func(chan<- T, chan<- error)) MockCallSeq
2526
}
2627

2728
func (m *MockCallSequence[T]) Call() (T, error) {
28-
m.callCount++
29+
m.callCount.Add(1)
2930

3031
m.initOnce.Do(func() {
3132
m.res = make(chan T)
@@ -67,7 +68,7 @@ func (m *MockCallSequence[T]) Call() (T, error) {
6768
}
6869

6970
func (m *MockCallSequence[T]) CallCount() uint {
70-
return m.callCount
71+
return uint(m.callCount.Load())
7172
}
7273

7374
func (m *MockCallSequence[T]) Close() {

lists/list_cache.go

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"errors"
77
"fmt"
88
"net"
9+
"sync/atomic"
910

1011
"github.com/sirupsen/logrus"
1112

@@ -177,12 +178,12 @@ func (b *ListCache) createCacheForGroup(
177178
})
178179
}
179180

180-
hasEntries := false
181+
var hasEntries atomic.Bool
181182

182183
producers.GoConsume(func(ctx context.Context, ch <-chan string) error {
183184
for host := range ch {
184185
if groupFactory.AddEntry(host) {
185-
hasEntries = true
186+
hasEntries.Store(true)
186187
} else {
187188
logger().WithField("host", host).Warn("no list cache was able to use host")
188189
}
@@ -192,18 +193,11 @@ func (b *ListCache) createCacheForGroup(
192193
})
193194

194195
err := producers.Wait()
195-
if err != nil {
196-
if !hasEntries {
197-
// Always fail the group if no entries were parsed
198-
return err
199-
}
200-
201-
var transientErr *TransientError
202-
203-
if errors.As(err, &transientErr) {
204-
// Temporary error: fail the whole group to retry later
205-
return err
206-
}
196+
if err != nil && !hasEntries.Load() {
197+
// Only fail the group if no entries were parsed at all
198+
// If we have entries from some sources, proceed even if other sources had errors
199+
// Transient errors will be retried on the next refresh cycle
200+
return err
207201
}
208202

209203
groupFactory.Finish()

lists/list_cache_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,68 @@ var _ = Describe("ListCache", func() {
437437
})
438438
})
439439
})
440+
441+
Describe("multiple sources with transient error (issue #1825)", func() {
442+
When("one source succeeds and another has a transient error", func() {
443+
BeforeEach(func() {
444+
// First source succeeds, second source has transient error (timeout)
445+
mockDownloader = newMockDownloader(func(res chan<- string, err chan<- error) {
446+
res <- "blocked1.com\nblocked2.com\n"
447+
err <- &TransientError{inner: errors.New("Client.Timeout exceeded while awaiting headers")}
448+
})
449+
450+
sutConfig.Strategy = config.InitStrategyFailOnError
451+
452+
lists = map[string][]config.BytesSource{
453+
"gr1": {
454+
mockDownloader.ListSource(), // First call succeeds with entries
455+
mockDownloader.ListSource(), // Second call has transient error
456+
},
457+
}
458+
})
459+
460+
It("should succeed with entries from the successful source", func() {
461+
// This test reproduces the bug reported in issue #1825
462+
// Currently this FAILS because transient errors cause the entire group to fail
463+
// even when other sources loaded successfully
464+
Expect(err).Should(Succeed(), "initialization should succeed when at least one source loads")
465+
Expect(sut).ShouldNot(BeNil())
466+
467+
// Verify the successful source's entries are available
468+
group := sut.Match("blocked1.com", []string{"gr1"})
469+
Expect(group).Should(ContainElement("gr1"), "entries from successful source should be available")
470+
471+
group = sut.Match("blocked2.com", []string{"gr1"})
472+
Expect(group).Should(ContainElement("gr1"), "entries from successful source should be available")
473+
})
474+
})
475+
476+
When("one source succeeds and another has a transient error with blocking strategy", func() {
477+
BeforeEach(func() {
478+
mockDownloader = newMockDownloader(func(res chan<- string, err chan<- error) {
479+
res <- "blocked1.com\nblocked2.com\n"
480+
err <- &TransientError{inner: errors.New("Client.Timeout exceeded while awaiting headers")}
481+
})
482+
483+
sutConfig.Strategy = config.InitStrategyBlocking
484+
485+
lists = map[string][]config.BytesSource{
486+
"gr1": {
487+
mockDownloader.ListSource(),
488+
mockDownloader.ListSource(),
489+
},
490+
}
491+
})
492+
493+
It("should succeed with entries from the successful source", func() {
494+
Expect(err).Should(Succeed(), "initialization should succeed when at least one source loads")
495+
Expect(sut).ShouldNot(BeNil())
496+
497+
group := sut.Match("blocked1.com", []string{"gr1"})
498+
Expect(group).Should(ContainElement("gr1"))
499+
})
500+
})
501+
})
440502
})
441503

442504
type MockDownloader struct {

0 commit comments

Comments
 (0)