Skip to content

Commit 19404c5

Browse files
bilyesbilyes
authored
feat: Add retry validation and comprehensive tests (#24)
- Add validate() method to RetryConfig with comprehensive field validation - Add godoc documentation for validate() function - Add comprehensive test suite covering all validation edge cases - Simplify retry configuration methods for better usability - Optimize context cancellation and timer handling in concurrent operations
1 parent 6fecbc1 commit 19404c5

3 files changed

Lines changed: 220 additions & 55 deletions

File tree

‎conman.go‎

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -113,16 +113,14 @@ type Task[T any] interface {
113113
//
114114
// Task execution errors are collected and accessible via Errors().
115115
func (c *ConMan[T]) Run(ctx context.Context, t Task[T]) error {
116-
select {
117-
case <-ctx.Done():
118-
return ctx.Err()
119-
default:
120-
c.reserveOne()
121-
go func() {
122-
defer c.releaseOne()
123-
c.executeTask(ctx, t)
124-
}()
116+
if err := ctx.Err(); err != nil {
117+
return err
125118
}
119+
c.reserveOne()
120+
go func() {
121+
defer c.releaseOne()
122+
c.executeTask(ctx, t)
123+
}()
126124
return nil
127125
}
128126

@@ -232,13 +230,12 @@ func (c *ConMan[T]) calculateDelay(attempt int, config *RetryConfig) time.Durati
232230

233231
// waitForNextAttempt waits for the calculated delay before the next retry attempt
234232
func (c *ConMan[T]) waitForNextAttempt(ctx context.Context, attempt int, config *RetryConfig) error {
235-
timer := time.NewTimer(c.calculateDelay(attempt, config))
236-
defer timer.Stop()
233+
delay := c.calculateDelay(attempt, config)
237234

238235
select {
239236
case <-ctx.Done():
240237
return ctx.Err()
241-
case <-timer.C:
238+
case <-time.After(delay):
242239
return nil
243240
}
244241
}

‎retry.go‎

Lines changed: 49 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
package conman
55

6+
import "fmt"
7+
68
// RetryConfig defines the retry behavior for operations that may fail temporarily.
79
// It includes parameters for controlling the number of attempts, delays, and backoff strategy.
810
type RetryConfig struct {
@@ -13,6 +15,33 @@ type RetryConfig struct {
1315
Jitter bool // Whether to add random jitter to delays
1416
}
1517

18+
// validate checks the validity of the RetryConfig fields.
19+
// It ensures that all parameters have valid values and logical relationships.
20+
// Returns an error if any validation fails, otherwise returns nil.
21+
func (rc *RetryConfig) validate() error {
22+
if rc.MaxAttempts <= 0 {
23+
return fmt.Errorf("MaxAttempts must be positive, got %d", rc.MaxAttempts)
24+
}
25+
if rc.InitialDelay < 0 {
26+
return fmt.Errorf("InitialDelay cannot be negative, got %d", rc.InitialDelay)
27+
}
28+
if rc.MaxDelay < 0 {
29+
return fmt.Errorf("MaxDelay cannot be negative, got %d", rc.MaxDelay)
30+
}
31+
if rc.MaxDelay > 0 && rc.InitialDelay > rc.MaxDelay {
32+
return fmt.Errorf("InitialDelay (%d) cannot be greater than MaxDelay (%d)",
33+
rc.InitialDelay, rc.MaxDelay)
34+
}
35+
if rc.BackoffFactor < 0 {
36+
return fmt.Errorf("BackoffFactor cannot be negative, got %f", rc.BackoffFactor)
37+
}
38+
if rc.BackoffFactor == 0.0 && rc.InitialDelay > 0 && rc.MaxAttempts > 1 {
39+
// This creates immediate zero delays after first attempt
40+
return fmt.Errorf("BackoffFactor of 0.0 with non-zero InitialDelay will result in zero delays")
41+
}
42+
return nil
43+
}
44+
1645
// RetriableError is an error type that indicates a task should be retried.
1746
// It includes an embedded RetryConfig to specify the retry strategy.
1847
type RetriableError struct {
@@ -25,72 +54,49 @@ func (e *RetriableError) Error() string {
2554
return e.Err.Error()
2655
}
2756

28-
// WithExponentialBackoff configures the error to use exponential backoff retry strategy.
29-
// Returns the RetriableError for method chaining.
30-
func (e *RetriableError) WithExponentialBackoff() *RetriableError {
31-
e.RetryConfig = ExponentialBackoffRetryPolicy{}.RetryConfig()
32-
return e
33-
}
34-
35-
// WithLinearBackoff configures the error to use linear backoff retry strategy.
36-
// Returns the RetriableError for method chaining.
37-
func (e *RetriableError) WithLinearBackoff() *RetriableError {
38-
e.RetryConfig = LinearBackoffRetryPolicy{}.RetryConfig()
39-
return e
57+
func (e *RetriableError) WithRetryConfig(config *RetryConfig) (*RetriableError, error) {
58+
if err := config.validate(); err != nil {
59+
return nil, err
60+
}
61+
e.RetryConfig = config
62+
return e, nil
4063
}
4164

42-
// WithNoBackoff configures the error to use immediate retries without delays.
65+
// WithExponentialBackoff configures the error to use exponential backoff retry strategy.
4366
// Returns the RetriableError for method chaining.
44-
func (e *RetriableError) WithNoBackoff() *RetriableError {
45-
e.RetryConfig = NoBackoffRetryPolicy{}.RetryConfig()
46-
return e
47-
}
48-
49-
// RetryPolicy defines an interface for different retry strategies.
50-
type RetryPolicy interface {
51-
RetryConfig() *RetryConfig
52-
}
53-
54-
// ExponentialBackoffRetryPolicy implements exponential backoff with jitter.
55-
// Delays increase exponentially with each retry attempt.
56-
type ExponentialBackoffRetryPolicy struct{}
57-
58-
// RetryConfig returns the retry configuration for exponential backoff.
59-
func (e ExponentialBackoffRetryPolicy) RetryConfig() *RetryConfig {
60-
return &RetryConfig{
67+
func (e *RetriableError) WithExponentialBackoff() *RetriableError {
68+
e.RetryConfig = &RetryConfig{
6169
MaxAttempts: 5,
6270
InitialDelay: 100, // 100 milliseconds
6371
BackoffFactor: 2.0,
6472
MaxDelay: 5000, // 5 seconds
6573
Jitter: true,
6674
}
75+
return e
6776
}
6877

69-
// LinearBackoffRetryPolicy implements linear backoff.
70-
// Delays increase linearly with each retry attempt.
71-
type LinearBackoffRetryPolicy struct{}
72-
73-
// RetryConfig returns the retry configuration for linear backoff.
74-
func (l LinearBackoffRetryPolicy) RetryConfig() *RetryConfig {
75-
return &RetryConfig{
78+
// WithLinearBackoff configures the error to use linear backoff retry strategy.
79+
// Returns the RetriableError for method chaining.
80+
func (e *RetriableError) WithLinearBackoff() *RetriableError {
81+
e.RetryConfig = &RetryConfig{
7682
MaxAttempts: 5,
7783
InitialDelay: 200, // 200 milliseconds
7884
BackoffFactor: 1.0,
7985
MaxDelay: 2000, // 2 seconds
8086
Jitter: false,
8187
}
88+
return e
8289
}
8390

84-
// NoBackoffRetryPolicy implements immediate retries without delays.
85-
type NoBackoffRetryPolicy struct{}
86-
87-
// RetryConfig returns the retry configuration for immediate retries.
88-
func (n NoBackoffRetryPolicy) RetryConfig() *RetryConfig {
89-
return &RetryConfig{
91+
// WithNoBackoff configures the error to use immediate retries without delays.
92+
// Returns the RetriableError for method chaining.
93+
func (e *RetriableError) WithNoBackoff() *RetriableError {
94+
e.RetryConfig = &RetryConfig{
9095
MaxAttempts: 3,
9196
InitialDelay: 0,
9297
BackoffFactor: 0.0,
9398
MaxDelay: 0,
9499
Jitter: false,
95100
}
101+
return e
96102
}

‎retry_test.go‎

Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package conman
2+
3+
import (
4+
"testing"
5+
)
6+
7+
func TestRetryConfigValidate(t *testing.T) {
8+
tests := []struct {
9+
name string
10+
config RetryConfig
11+
expectError bool
12+
errorMsg string
13+
}{
14+
{
15+
name: "valid config",
16+
config: RetryConfig{
17+
MaxAttempts: 5,
18+
InitialDelay: 100,
19+
BackoffFactor: 2.0,
20+
MaxDelay: 5000,
21+
Jitter: true,
22+
},
23+
expectError: false,
24+
},
25+
{
26+
name: "zero max attempts",
27+
config: RetryConfig{
28+
MaxAttempts: 0,
29+
InitialDelay: 100,
30+
BackoffFactor: 2.0,
31+
MaxDelay: 5000,
32+
},
33+
expectError: true,
34+
errorMsg: "MaxAttempts must be positive, got 0",
35+
},
36+
{
37+
name: "negative max attempts",
38+
config: RetryConfig{
39+
MaxAttempts: -1,
40+
InitialDelay: 100,
41+
BackoffFactor: 2.0,
42+
MaxDelay: 5000,
43+
},
44+
expectError: true,
45+
errorMsg: "MaxAttempts must be positive, got -1",
46+
},
47+
{
48+
name: "negative initial delay",
49+
config: RetryConfig{
50+
MaxAttempts: 5,
51+
InitialDelay: -100,
52+
BackoffFactor: 2.0,
53+
MaxDelay: 5000,
54+
},
55+
expectError: true,
56+
errorMsg: "InitialDelay cannot be negative, got -100",
57+
},
58+
{
59+
name: "negative max delay",
60+
config: RetryConfig{
61+
MaxAttempts: 5,
62+
InitialDelay: 100,
63+
BackoffFactor: 2.0,
64+
MaxDelay: -5000,
65+
},
66+
expectError: true,
67+
errorMsg: "MaxDelay cannot be negative, got -5000",
68+
},
69+
{
70+
name: "initial delay greater than max delay",
71+
config: RetryConfig{
72+
MaxAttempts: 5,
73+
InitialDelay: 6000,
74+
BackoffFactor: 2.0,
75+
MaxDelay: 5000,
76+
},
77+
expectError: true,
78+
errorMsg: "InitialDelay (6000) cannot be greater than MaxDelay (5000)",
79+
},
80+
{
81+
name: "negative backoff factor",
82+
config: RetryConfig{
83+
MaxAttempts: 5,
84+
InitialDelay: 100,
85+
BackoffFactor: -1.0,
86+
MaxDelay: 5000,
87+
},
88+
expectError: true,
89+
errorMsg: "BackoffFactor cannot be negative, got -1.000000",
90+
},
91+
{
92+
name: "zero backoff factor with non-zero initial delay and multiple attempts",
93+
config: RetryConfig{
94+
MaxAttempts: 5,
95+
InitialDelay: 100,
96+
BackoffFactor: 0.0,
97+
MaxDelay: 5000,
98+
},
99+
expectError: true,
100+
errorMsg: "BackoffFactor of 0.0 with non-zero InitialDelay will result in zero delays",
101+
},
102+
{
103+
name: "zero backoff factor with zero initial delay",
104+
config: RetryConfig{
105+
MaxAttempts: 5,
106+
InitialDelay: 0,
107+
BackoffFactor: 0.0,
108+
MaxDelay: 0,
109+
},
110+
expectError: false,
111+
},
112+
{
113+
name: "zero backoff factor with single attempt",
114+
config: RetryConfig{
115+
MaxAttempts: 1,
116+
InitialDelay: 100,
117+
BackoffFactor: 0.0,
118+
MaxDelay: 5000,
119+
},
120+
expectError: false,
121+
},
122+
{
123+
name: "jitter with zero initial delay",
124+
config: RetryConfig{
125+
MaxAttempts: 5,
126+
InitialDelay: 0,
127+
BackoffFactor: 2.0,
128+
MaxDelay: 5000,
129+
Jitter: true,
130+
},
131+
expectError: false,
132+
},
133+
{
134+
name: "minimal valid config",
135+
config: RetryConfig{
136+
MaxAttempts: 1,
137+
InitialDelay: 0,
138+
BackoffFactor: 0.0,
139+
MaxDelay: 0,
140+
Jitter: false,
141+
},
142+
expectError: false,
143+
},
144+
}
145+
146+
for _, tt := range tests {
147+
t.Run(tt.name, func(t *testing.T) {
148+
err := tt.config.validate()
149+
if tt.expectError {
150+
if err == nil {
151+
t.Errorf("Expected error but got none")
152+
} else if tt.errorMsg != "" && err.Error() != tt.errorMsg {
153+
t.Errorf("Expected error message %q, got %q", tt.errorMsg, err.Error())
154+
}
155+
} else {
156+
if err != nil {
157+
t.Errorf("Expected no error but got: %v", err)
158+
}
159+
}
160+
})
161+
}
162+
}

0 commit comments

Comments
 (0)