Skip to content

Commit

Permalink
feat: add an option to log errors being retried
Browse files Browse the repository at this point in the history
This allows to provide feedback immediately while the code is still doing
retries in the background.

Signed-off-by: Andrey Smirnov <smirnov.andrey@gmail.com>
  • Loading branch information
smira authored and talos-bot committed Sep 22, 2020
1 parent 073067b commit 752f081
Show file tree
Hide file tree
Showing 8 changed files with 47 additions and 37 deletions.
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2020-09-01T21:14:08Z by kres ee90a80-dirty.
# Generated on 2020-09-21T13:20:49Z by kres 7e146df-dirty.

# common variables

Expand Down Expand Up @@ -34,7 +34,7 @@ COMMON_ARGS += --build-arg=USERNAME=$(USERNAME)
COMMON_ARGS += --build-arg=TOOLCHAIN=$(TOOLCHAIN)
COMMON_ARGS += --build-arg=GOFUMPT_VERSION=$(GOFUMPT_VERSION)
COMMON_ARGS += --build-arg=TESTPKGS=$(TESTPKGS)
TOOLCHAIN ?= docker.io/golang:1.14-alpine
TOOLCHAIN ?= docker.io/golang:1.15-alpine

# help menu

Expand Down
4 changes: 2 additions & 2 deletions hack/git-chglog/config.yaml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# THIS FILE WAS AUTOMATICALLY GENERATED, PLEASE DO NOT EDIT.
#
# Generated on 2020-09-01T21:13:54Z by kres ee90a80-dirty.
# Generated on 2020-09-21T13:20:49Z by kres 7e146df-dirty.

style: github
template: CHANGELOG.tpl.md
info:
title: CHANGELOG
repository_url: https://github.com/talos-systems/talos
repository_url: https://github.com/talos-systems/go-retry
options:
commits:
# filters:
Expand Down
2 changes: 1 addition & 1 deletion retry/constant.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func (c constantRetryer) Retry(f RetryableFunc) error {
tick := NewConstantTicker(c.options)
defer tick.Stop()

return retry(f, c.duration, tick)
return retry(f, c.duration, tick, c.options)
}

// Tick implements the Ticker interface.
Expand Down
2 changes: 1 addition & 1 deletion retry/exponential.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func (e exponentialRetryer) Retry(f RetryableFunc) error {
tick := NewExponentialTicker(e.options)
defer tick.Stop()

return retry(f, e.duration, tick)
return retry(f, e.duration, tick, e.options)
}

// Tick implements the Ticker interface.
Expand Down
2 changes: 1 addition & 1 deletion retry/linear.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (l linearRetryer) Retry(f RetryableFunc) error {
tick := NewLinearTicker(l.options)
defer tick.Stop()

return retry(f, l.duration, tick)
return retry(f, l.duration, tick, l.options)
}

// Tick implements the Ticker interface.
Expand Down
16 changes: 13 additions & 3 deletions retry/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,15 @@

package retry

import "time"
import (
"time"
)

// Options is the functional options struct.
type Options struct {
Units time.Duration
Jitter time.Duration
Units time.Duration
Jitter time.Duration
LogErrors bool
}

// Option is the functional option func.
Expand All @@ -29,6 +32,13 @@ func WithJitter(o time.Duration) Option {
}
}

// WithErrorLogging logs errors as they are encountered during the retry loop.
func WithErrorLogging(enable bool) Option {
return func(args *Options) {
args.LogErrors = enable
}
}

// NewDefaultOptions initializes a Options struct with default values.
func NewDefaultOptions(setters ...Option) *Options {
opts := &Options{
Expand Down
48 changes: 22 additions & 26 deletions retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package retry

import (
"fmt"
"log"
"math/rand"
"sync"
"time"
Expand Down Expand Up @@ -48,7 +49,7 @@ func (e *ErrorSet) Error() string {
}

// Append adds the error to the set if the error is not already present.
func (e *ErrorSet) Append(err error) error {
func (e *ErrorSet) Append(err error) bool {
e.mu.Lock()
defer e.mu.Unlock()

Expand All @@ -70,7 +71,7 @@ func (e *ErrorSet) Append(err error) error {
e.errs = append(e.errs, err)
}

return e
return ok
}

// TimeoutError represents a timeout error.
Expand Down Expand Up @@ -143,42 +144,37 @@ func UnexpectedError(err error) error {
return unexpectedError{err}
}

func retry(f RetryableFunc, d time.Duration, t Ticker) error {
func retry(f RetryableFunc, d time.Duration, t Ticker, o *Options) error {
timer := time.NewTimer(d)
defer timer.Stop()

errs := &ErrorSet{}

// We run the func first to avoid having to wait for the next tick.
if err := f(); err != nil {
if _, ok := err.(unexpectedError); ok {
return errs.Append(err)
for {
if err := f(); err != nil {
exists := errs.Append(err)

switch err.(type) {
case expectedError:
// retry expected errors
if !exists && o.LogErrors {
log.Printf("retrying error: %s", err)
}
default:
return errs
}
} else {
return nil
}
} else {
return nil
}

for {
select {
case <-timer.C:
return errs.Append(TimeoutError{})
errs.Append(TimeoutError{})

return errs
case <-t.StopChan():
return nil
case <-time.After(t.Tick()):
}

if err := f(); err != nil {
switch err.(type) {
case expectedError:
//nolint: errcheck
errs.Append(err)

continue
case unexpectedError:
return errs.Append(err)
}
}

return nil
}
}
6 changes: 5 additions & 1 deletion retry/retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ func Test_retry(t *testing.T) {
f RetryableFunc
d time.Duration
t Ticker
o *Options
}

tests := []struct {
Expand All @@ -30,6 +31,7 @@ func Test_retry(t *testing.T) {
f: func() error { return ExpectedError(errors.New("test")) },
d: 2 * time.Second,
t: NewConstantTicker(NewDefaultOptions()),
o: &Options{},
},
wantString: "2 error(s) occurred:\n\ttest\n\ttimeout",
},
Expand All @@ -39,6 +41,7 @@ func Test_retry(t *testing.T) {
f: func() error { return UnexpectedError(errors.New("test")) },
d: 2 * time.Second,
t: NewConstantTicker(NewDefaultOptions()),
o: &Options{},
},
wantString: "1 error(s) occurred:\n\ttest",
},
Expand All @@ -48,14 +51,15 @@ func Test_retry(t *testing.T) {
f: func() error { return nil },
d: 2 * time.Second,
t: NewConstantTicker(NewDefaultOptions()),
o: &Options{},
},
wantString: "",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := retry(tt.args.f, tt.args.d, tt.args.t); err != nil && tt.wantString != err.Error() {
if err := retry(tt.args.f, tt.args.d, tt.args.t, tt.args.o); err != nil && tt.wantString != err.Error() {
t.Errorf("retry() error = %q\nwant:\n%q", err, tt.wantString)
}
})
Expand Down

0 comments on commit 752f081

Please sign in to comment.