Skip to content

Commit 048cdc2

Browse files
committed
refactor(cache): tighten retry helper after review
Reuse nopLogger from pkg/cache instead of duplicating a test stub, drop the redundant lastErr local in the retry loop, and trim the initMaxWait comment to its intent. Signed-off-by: Miguel Martinez <miguel@chainloop.dev>
1 parent 3f9a023 commit 048cdc2

2 files changed

Lines changed: 10 additions & 20 deletions

File tree

pkg/cache/natskv.go

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,8 @@ import (
3030

3131
const (
3232
streamUpdateTimeout = 5 * time.Second
33-
// initMaxWait bounds the total time we'll retry bucket init on the initial
34-
// boot path. NATS auto-reconnects every ReconnectWait (2s), so this gives
35-
// the client several reconnect cycles to heal a transient drop before we
36-
// give up and refuse to start.
33+
// initMaxWait covers several nats.ReconnectWait (2s) cycles so a transient
34+
// drop during boot can heal before we give up.
3735
initMaxWait = 30 * time.Second
3836
initRetryPeriod = 2 * time.Second
3937
)
@@ -73,19 +71,18 @@ func newNATSKV[T any](cfg *config) (*natsKVCache[T], error) {
7371
// apply to connectivity errors; configuration errors fail fast.
7472
func (c *natsKVCache[T]) initBucketWithRetry(maxWait, period time.Duration) error {
7573
deadline := time.Now().Add(maxWait)
76-
var lastErr error
7774
for attempt := 1; ; attempt++ {
78-
lastErr = c.initBucket()
79-
if lastErr == nil {
75+
err := c.initBucket()
76+
if err == nil {
8077
return nil
8178
}
82-
if !isRetryableInitError(lastErr) {
83-
return lastErr
79+
if !isRetryableInitError(err) {
80+
return err
8481
}
8582
if time.Now().After(deadline) {
86-
return fmt.Errorf("cache: bucket %q init failed after %s (%d attempts): %w", c.bucket, maxWait, attempt, lastErr)
83+
return fmt.Errorf("cache: bucket %q init failed after %s (%d attempts): %w", c.bucket, maxWait, attempt, err)
8784
}
88-
c.logger.Warnw("msg", "cache: bucket init failed, retrying", "bucket", c.bucket, "attempt", attempt, "error", lastErr)
85+
c.logger.Warnw("msg", "cache: bucket init failed, retrying", "bucket", c.bucket, "attempt", attempt, "error", err)
8986
time.Sleep(period)
9087
}
9188
}

pkg/cache/natskv_test.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,6 @@ type testStruct struct {
5959
Value int `json:"value"`
6060
}
6161

62-
type testLogger struct{}
63-
64-
func (testLogger) Debugw(...any) {}
65-
func (testLogger) Warnw(...any) {}
66-
func (testLogger) Infow(...any) {}
67-
func (testLogger) Errorw(...any) {}
68-
6962
func TestNATSKV_GetSetDelete(t *testing.T) {
7063
nc := startEmbeddedNATS(t)
7164

@@ -307,11 +300,11 @@ func TestNATSKV_InitBucketRetriesOnTransientError(t *testing.T) {
307300
nc.Close()
308301

309302
c := &natsKVCache[[]byte]{
310-
logger: testLogger{},
303+
logger: nopLogger{},
311304
conn: nc,
312305
bucket: "test-retry-exhausted",
313306
cfg: &config{
314-
logger: testLogger{},
307+
logger: nopLogger{},
315308
bucketName: "test-retry-exhausted",
316309
ttl: time.Minute,
317310
maxBytes: 10 * 1024,

0 commit comments

Comments
 (0)