fix: Respect context when retrying (#2285)
Respect the cancellation of the context being passed in, preventing us from retrying events that are doomed to fail.
This commit is contained in:
1 parent
5fccc5a0b8
commit
2e3ec967f6
2 files changed
+27
-3
No files matched your search
@@ -49,6 +49,7 @@ func (e *Eventer) retrySend(ctx context.Context, retries uint, backOff backoff,
|
||||
var retryErrors error
|
||||
var attemptStatus eventlogger.Status
|
||||
info := retryInfo{}
|
||||
ATTEMPTS:
|
||||
for attempts := uint(1); ; attempts++ {
|
||||
if attempts > retries+1 {
|
||||
retryErrors = multierror.Append(retryErrors, fmt.Errorf("%s: reached max of %d: %w", op, retries, ErrMaxRetries))
|
||||
@@ -68,8 +69,13 @@ func (e *Eventer) retrySend(ctx context.Context, retries uint, backOff backoff,
|
||||
d := backOff.duration(attempts)
|
||||
info.retries++
|
||||
info.backoff = info.backoff + d
|
||||
time.Sleep(d)
|
||||
continue
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
retryErrors = multierror.Append(retryErrors, ctx.Err())
|
||||
break ATTEMPTS
|
||||
case <-time.After(d):
|
||||
continue
|
||||
}
|
||||
}
|
||||
success = true
|
||||
break
|
||||
|
||||
@@ -16,6 +16,8 @@ import (
|
||||
func TestEventer_retrySend(t *testing.T) {
|
||||
t.Parallel()
|
||||
ctx := context.Background()
|
||||
canceledCtx, cancel := context.WithCancel(ctx)
|
||||
cancel()
|
||||
|
||||
testConfig := TestEventerConfig(t, "TestEventer_retrySend")
|
||||
|
||||
@@ -31,6 +33,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
ctx context.Context
|
||||
retries uint
|
||||
backOff backoff
|
||||
handler sendHandler
|
||||
@@ -39,6 +42,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
}{
|
||||
{
|
||||
name: "missing-backoff",
|
||||
ctx: context.Background(),
|
||||
retries: 1,
|
||||
handler: func() (eventlogger.Status, error) {
|
||||
return eventer.broker.Send(ctx, eventlogger.EventType(ErrorType), testEvent)
|
||||
@@ -48,6 +52,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
},
|
||||
{
|
||||
name: "missing-handler",
|
||||
ctx: context.Background(),
|
||||
retries: 1,
|
||||
backOff: expBackoff{},
|
||||
wantErrIs: ErrInvalidParameter,
|
||||
@@ -55,6 +60,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
},
|
||||
{
|
||||
name: "too-many-retries",
|
||||
ctx: context.Background(),
|
||||
retries: 3,
|
||||
backOff: expBackoff{},
|
||||
handler: func() (eventlogger.Status, error) {
|
||||
@@ -63,8 +69,19 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
wantErrIs: ErrMaxRetries,
|
||||
wantErrContain: "too many retries",
|
||||
},
|
||||
{
|
||||
name: "canceled",
|
||||
ctx: canceledCtx,
|
||||
retries: 3,
|
||||
backOff: expBackoff{},
|
||||
handler: func() (eventlogger.Status, error) {
|
||||
return eventlogger.Status{}, fmt.Errorf("%s: will never work: %w", "TestEventer_retrySend", ErrMaxRetries)
|
||||
},
|
||||
wantErrIs: context.Canceled,
|
||||
},
|
||||
{
|
||||
name: "success-with-warnings",
|
||||
ctx: context.Background(),
|
||||
retries: 3,
|
||||
backOff: expBackoff{},
|
||||
handler: func() (eventlogger.Status, error) {
|
||||
@@ -75,6 +92,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
},
|
||||
{
|
||||
name: "success",
|
||||
ctx: context.Background(),
|
||||
retries: 1,
|
||||
backOff: expBackoff{},
|
||||
handler: func() (eventlogger.Status, error) {
|
||||
@@ -89,7 +107,7 @@ func TestEventer_retrySend(t *testing.T) {
|
||||
defer os.Remove(testConfig.AllEvents.Name())
|
||||
defer os.Remove(testConfig.ErrorEvents.Name())
|
||||
|
||||
err := eventer.retrySend(ctx, tt.retries, tt.backOff, tt.handler)
|
||||
err := eventer.retrySend(tt.ctx, tt.retries, tt.backOff, tt.handler)
|
||||
if tt.wantErrIs != nil {
|
||||
require.Error(err)
|
||||
multi, isMultiError := err.(*multierror.Error)
|
||||
|
||||
Reference in new issue
Block a user