cockroachdb / cockroachdb/cockroach
retry: the `StartWithCtx` API makes it too easy to ignore cancellation
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
This is the general pattern we recommend callers use the `retry` package:
```go
retryOpts := base.DefaultRetryOptions()
var lastErr error
for retryable := retry.StartWithCtx(ctx, retryOpts); retryable.Next(); {
lastErr := fn(ctx)
if lastErr == nil {
return nil
}
}
return lastErr
```
This is more-or-less what every caller currently does.
Alas, this fails to properly work if the context is cancelled _before the first iteration_. In that case, `lastErr` remains `nil` but the function is never caller. Then the `return lastErr` at the end will make the caller of this code believe the function was called successfully where, instead, it was not called at all!
**Expected behavior** `retry` should ensure an error is returned in case of premature cancellation of the retry loop; and the API defined in such a way that call points notice this case properly.
Jira issue: CRDB-26537
Contributor guide
Assessment
This issue has not been assessed yet.