Skip to content

Commit e65a1dd

Browse files
committed
Introduce attempt and total timouts
1 parent b140d11 commit e65a1dd

File tree

10 files changed

+73
-37
lines changed

10 files changed

+73
-37
lines changed

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,8 @@ client, err := netconf.NewClient(
8888
netconf.Password("secret"),
8989
netconf.Port(830),
9090
netconf.MaxRetries(5),
91-
netconf.OperationTimeout(120*time.Second),
91+
netconf.AttemptTimeout(60*time.Second),
92+
netconf.TotalTimeout(5*time.Minute),
9293
)
9394

9495
// Connection established automatically on first operation

client.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,9 @@ const (
5454
DefaultBackoffMaxDelay = 60 * time.Second
5555
DefaultBackoffDelayFactor = 2
5656
DefaultLockReleaseTimeout = 120 * time.Second
57-
DefaultConnectTimeout = 30 * time.Second
58-
DefaultOperationTimeout = 60 * time.Second
57+
DefaultConnectTimeout = 10 * time.Second
58+
DefaultAttemptTimeout = 30 * time.Second
59+
DefaultTotalTimeout = 2 * time.Minute
5960
DefaultPrettyPrintLogs = true
6061
)
6162

@@ -150,7 +151,8 @@ type Client struct {
150151
BackoffDelayFactor float64
151152
LockReleaseTimeout time.Duration
152153
ConnectTimeout time.Duration
153-
OperationTimeout time.Duration
154+
AttemptTimeout time.Duration // Timeout for a single operation attempt (scrapligo timeout)
155+
TotalTimeout time.Duration // Total timeout across all retry attempts
154156

155157
// Capability tracking
156158
Capabilities []string
@@ -219,7 +221,7 @@ func (c *Client) buildScrapligoOptions() []util.Option {
219221
options.WithAuthPassword(c.password),
220222
options.WithPort(c.Port),
221223
options.WithTimeoutSocket(c.ConnectTimeout),
222-
options.WithTimeoutOps(c.OperationTimeout),
224+
options.WithTimeoutOps(c.AttemptTimeout),
223225
options.WithTransportType(transport.StandardTransport),
224226
}
225227

@@ -248,7 +250,8 @@ func NewClient(host string, opts ...func(*Client)) (*Client, error) {
248250
BackoffDelayFactor: DefaultBackoffDelayFactor,
249251
LockReleaseTimeout: DefaultLockReleaseTimeout,
250252
ConnectTimeout: DefaultConnectTimeout,
251-
OperationTimeout: DefaultOperationTimeout,
253+
AttemptTimeout: DefaultAttemptTimeout,
254+
TotalTimeout: DefaultTotalTimeout,
252255
logger: &NoOpLogger{},
253256
prettyPrintLogs: DefaultPrettyPrintLogs,
254257
redactionPatterns: defaultRedactionPatterns,
@@ -1348,7 +1351,7 @@ func (c *Client) sendRPC(ctx context.Context, req *Req) (Res, error) {
13481351
// Apply timeout with proper priority:
13491352
// 1. Request-specific timeout (highest priority)
13501353
// 2. Context deadline (if already set)
1351-
// 3. Default operation timeout (fallback)
1354+
// 3. Default total timeout (fallback - spans all retry attempts)
13521355
if req.Timeout > 0 {
13531356
// Request-specific timeout takes precedence
13541357
var cancel context.CancelFunc
@@ -1357,7 +1360,7 @@ func (c *Client) sendRPC(ctx context.Context, req *Req) (Res, error) {
13571360
} else if _, hasDeadline := ctx.Deadline(); !hasDeadline {
13581361
// Only apply default if no deadline already set
13591362
var cancel context.CancelFunc
1360-
ctx, cancel = context.WithTimeout(ctx, c.OperationTimeout)
1363+
ctx, cancel = context.WithTimeout(ctx, c.TotalTimeout)
13611364
defer cancel()
13621365
}
13631366

client_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ func TestClientDefaultConfiguration(t *testing.T) {
3838
BackoffDelayFactor: 1.2,
3939
LockReleaseTimeout: 120 * time.Second,
4040
ConnectTimeout: 30 * time.Second,
41-
OperationTimeout: 60 * time.Second,
41+
AttemptTimeout: 30 * time.Second,
42+
TotalTimeout: 2 * time.Minute,
4243
}
4344

4445
// Apply options
@@ -148,7 +149,8 @@ func TestClientOpen(t *testing.T) {
148149
username: "test",
149150
password: "test",
150151
ConnectTimeout: 1 * time.Second,
151-
OperationTimeout: 1 * time.Second,
152+
AttemptTimeout: 1 * time.Second,
153+
TotalTimeout: 5 * time.Second,
152154
logger: &NoOpLogger{},
153155
prettyPrintLogs: false,
154156
redactionPatterns: defaultRedactionPatterns,
@@ -176,7 +178,8 @@ func TestClientOpen(t *testing.T) {
176178
BackoffDelayFactor: 3.0,
177179
LockReleaseTimeout: 240 * time.Second,
178180
ConnectTimeout: 45 * time.Second,
179-
OperationTimeout: 90 * time.Second,
181+
AttemptTimeout: 60 * time.Second,
182+
TotalTimeout: 5 * time.Minute,
180183
logger: &NoOpLogger{},
181184
prettyPrintLogs: true,
182185
redactionPatterns: defaultRedactionPatterns,
@@ -231,7 +234,8 @@ func TestClientOpen(t *testing.T) {
231234
password: "test",
232235
driver: nil, // Simulate closed state
233236
ConnectTimeout: 1 * time.Second,
234-
OperationTimeout: 1 * time.Second,
237+
AttemptTimeout: 1 * time.Second,
238+
TotalTimeout: 5 * time.Second,
235239
logger: &NoOpLogger{},
236240
prettyPrintLogs: false,
237241
redactionPatterns: defaultRedactionPatterns,

docs/concurrency.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -368,7 +368,7 @@ func NewNetconfManager(host, username, password string) (*NetconfManager, error)
368368
readClient, err := netconf.NewClient(host,
369369
netconf.Username(username),
370370
netconf.Password(password),
371-
netconf.OperationTimeout(30*time.Second))
371+
netconf.TotalTimeout(1*time.Minute))
372372
if err != nil {
373373
return nil, err
374374
}
@@ -377,7 +377,8 @@ func NewNetconfManager(host, username, password string) (*NetconfManager, error)
377377
writeClient, err := netconf.NewClient(host,
378378
netconf.Username(username),
379379
netconf.Password(password),
380-
netconf.OperationTimeout(5*time.Minute),
380+
netconf.AttemptTimeout(60*time.Second),
381+
netconf.TotalTimeout(10*time.Minute),
381382
netconf.MaxRetries(5))
382383
if err != nil {
383384
readClient.Close()

docs/error-handling.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ Configure timeouts based on operation:
571571
```go
572572
// Quick operations
573573
client, _ := netconf.NewClient("192.168.1.1",
574-
netconf.OperationTimeout(30*time.Second))
574+
netconf.TotalTimeout(1*time.Minute))
575575

576576
// Long-running operations
577577
res, err := client.Commit(ctx, netconf.Timeout(5*time.Minute))
@@ -648,9 +648,10 @@ client, err := netconf.NewClient("192.168.1.1",
648648

649649
**Solutions:**
650650
```go
651-
// Increase global timeout
651+
// Increase timeouts
652652
client, _ := netconf.NewClient("192.168.1.1",
653-
netconf.OperationTimeout(120*time.Second))
653+
netconf.AttemptTimeout(60*time.Second), // Per-attempt timeout
654+
netconf.TotalTimeout(5*time.Minute)) // Total timeout across retries
654655

655656
// Increase per-operation timeout
656657
res, err := client.Get(ctx, filter, netconf.Timeout(5*time.Minute))
@@ -775,7 +776,8 @@ func main() {
775776
netconf.Username("automation"),
776777
netconf.SSHKey("/path/to/key"),
777778
netconf.MaxRetries(5),
778-
netconf.OperationTimeout(2*time.Minute),
779+
netconf.AttemptTimeout(60*time.Second),
780+
netconf.TotalTimeout(5*time.Minute),
779781
netconf.LockReleaseTimeout(3*time.Minute),
780782
netconf.BackoffMinDelay(2*time.Second),
781783
netconf.BackoffMaxDelay(60*time.Second),

docs/operations.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,8 +735,9 @@ res, err := client.Get(ctx, filter, netconf.Timeout(30*time.Second))
735735

736736
**Notes:**
737737
- Request-specific timeout takes precedence over context deadline
738-
- If no timeout is specified, uses the client's `OperationTimeout` (default: 60s)
738+
- If no timeout is specified, uses the client's `TotalTimeout` (default: 2min)
739739
- Useful for operations that may take longer than the default timeout
740+
- This timeout spans all retry attempts
740741

741742
### DefaultOperation
742743

@@ -951,7 +952,7 @@ if err != nil {
951952

952953
### Best Practices
953954

954-
- **Configure appropriate timeouts**: Set `OperationTimeout` based on expected operation duration
955+
- **Configure appropriate timeouts**: Set `AttemptTimeout` for per-operation timeout and `TotalTimeout` for total retry duration
955956
- **Use context deadlines**: Combine with context timeouts for request-level control
956957
- **Monitor retry counts**: High retry counts may indicate systemic issues
957958
- **Handle non-transient errors**: Not all errors are retryable - implement proper error handling

docs/quickstart.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,8 +382,9 @@ func main() {
382382
"192.168.1.1",
383383
netconf.Username("admin"),
384384
netconf.Password("secret"),
385-
netconf.ConnectTimeout(30*time.Second),
386-
netconf.OperationTimeout(60*time.Second),
385+
netconf.ConnectTimeout(15*time.Second),
386+
netconf.AttemptTimeout(45*time.Second),
387+
netconf.TotalTimeout(3*time.Minute),
387388
netconf.MaxRetries(5),
388389
)
389390
if err != nil {
@@ -439,8 +440,9 @@ func main() {
439440
"192.168.1.1",
440441
netconf.Username("admin"),
441442
netconf.Password("secret"),
442-
netconf.ConnectTimeout(30*time.Second),
443-
netconf.OperationTimeout(2*time.Minute),
443+
netconf.ConnectTimeout(15*time.Second),
444+
netconf.AttemptTimeout(60*time.Second),
445+
netconf.TotalTimeout(5*time.Minute),
444446
netconf.MaxRetries(5),
445447
netconf.LockReleaseTimeout(2*time.Minute),
446448
)

operations_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,13 @@ func TestContextTimeout(t *testing.T) {
9191
// that the code path exists and is syntactically correct
9292

9393
client := &Client{
94-
OperationTimeout: 60,
94+
TotalTimeout: 60,
9595
}
9696

9797
ctx := context.Background()
9898
// Verify that client has timeout configured
99-
if client.OperationTimeout == 0 {
100-
t.Error("expected OperationTimeout to be set")
99+
if client.TotalTimeout == 0 {
100+
t.Error("expected TotalTimeout to be set")
101101
}
102102

103103
// Context should respect the timeout

options.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,19 @@ func ConnectTimeout(duration time.Duration) func(*Client) {
4646
}
4747
}
4848

49-
// OperationTimeout sets the operation timeout (default: 60s)
50-
func OperationTimeout(duration time.Duration) func(*Client) {
49+
// AttemptTimeout sets the timeout for a single operation attempt (default: 30s)
50+
// This timeout is enforced by scrapligo for each individual RPC call.
51+
func AttemptTimeout(duration time.Duration) func(*Client) {
5152
return func(c *Client) {
52-
c.OperationTimeout = duration
53+
c.AttemptTimeout = duration
54+
}
55+
}
56+
57+
// TotalTimeout sets the total timeout across all retry attempts (default: 2min)
58+
// This timeout spans all retry attempts including backoff delays.
59+
func TotalTimeout(duration time.Duration) func(*Client) {
60+
return func(c *Client) {
61+
c.TotalTimeout = duration
5362
}
5463
}
5564

options_test.go

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,20 @@ func TestClientOptions(t *testing.T) {
6161
},
6262
},
6363
{
64-
name: "OperationTimeout option",
65-
option: OperationTimeout(120 * time.Second),
64+
name: "AttemptTimeout option",
65+
option: AttemptTimeout(45 * time.Second),
6666
validate: func(t *testing.T, c *Client) {
67-
if c.OperationTimeout != 120*time.Second {
68-
t.Errorf("expected OperationTimeout 120s, got %v", c.OperationTimeout)
67+
if c.AttemptTimeout != 45*time.Second {
68+
t.Errorf("expected AttemptTimeout 45s, got %v", c.AttemptTimeout)
69+
}
70+
},
71+
},
72+
{
73+
name: "TotalTimeout option",
74+
option: TotalTimeout(3 * time.Minute),
75+
validate: func(t *testing.T, c *Client) {
76+
if c.TotalTimeout != 3*time.Minute {
77+
t.Errorf("expected TotalTimeout 3min, got %v", c.TotalTimeout)
6978
}
7079
},
7180
},
@@ -219,7 +228,8 @@ func TestMultipleOptions(t *testing.T) {
219228
Port(8830),
220229
MaxRetries(3),
221230
ConnectTimeout(20 * time.Second),
222-
OperationTimeout(90 * time.Second),
231+
AttemptTimeout(45 * time.Second),
232+
TotalTimeout(3 * time.Minute),
223233
BackoffMinDelay(2 * time.Second),
224234
BackoffMaxDelay(90 * time.Second),
225235
BackoffDelayFactor(1.5),
@@ -246,8 +256,11 @@ func TestMultipleOptions(t *testing.T) {
246256
if client.ConnectTimeout != 20*time.Second {
247257
t.Errorf("expected ConnectTimeout 20s, got %v", client.ConnectTimeout)
248258
}
249-
if client.OperationTimeout != 90*time.Second {
250-
t.Errorf("expected OperationTimeout 90s, got %v", client.OperationTimeout)
259+
if client.AttemptTimeout != 45*time.Second {
260+
t.Errorf("expected AttemptTimeout 45s, got %v", client.AttemptTimeout)
261+
}
262+
if client.TotalTimeout != 3*time.Minute {
263+
t.Errorf("expected TotalTimeout 3min, got %v", client.TotalTimeout)
251264
}
252265
if client.BackoffMinDelay != 2*time.Second {
253266
t.Errorf("expected BackoffMinDelay 2s, got %v", client.BackoffMinDelay)

0 commit comments

Comments
 (0)