From ed41a4bc47c824510a733ae1d4f2cdddcb8a1ed5 Mon Sep 17 00:00:00 2001 From: Thomas Stromberg Date: Mon, 28 Jul 2025 14:34:21 -0400 Subject: [PATCH] Apply Google Style Guide & performance optimizations --- README.md | 13 ++++-------- options.go | 29 +++++++++++++++++---------- retry.go | 58 ++++++++++++++++++++++++++++-------------------------- 3 files changed, 53 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 33a2436..6e5fe41 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,6 @@ [![Release](https://img.shields.io/github/release/codeGROOVE-dev/retry-go.svg?style=flat-square)](https://github.com/codeGROOVE-dev/retry-go/releases/latest) [![Software License](https://img.shields.io/badge/license-MIT-brightgreen.svg?style=flat-square)](LICENSE.md) -![GitHub Actions](https://github.com/codeGROOVE-dev/retry-go/actions/workflows/workflow.yaml/badge.svg) [![Go Report Card](https://goreportcard.com/badge/github.com/codeGROOVE-dev/retry-go?style=flat-square)](https://goreportcard.com/report/github.com/codeGROOVE-dev/retry-go) [![Go Reference](https://pkg.go.dev/badge/github.com/codeGROOVE-dev/retry-go.svg)](https://pkg.go.dev/github.com/codeGROOVE-dev/retry-go) @@ -26,6 +25,8 @@ This fork retains the original v4 API provided by the retry-go codebase, and is # SYNOPSIS +For full package documentation, see https://pkg.go.dev/github.com/codeGROOVE-dev/retry-go + HTTP GET with retry: url := "http://example.com" @@ -80,6 +81,8 @@ HTTP GET with retry with data: [More examples](https://github.com/codeGROOVE-dev/retry-go/tree/master/examples) + + # SEE ALSO * [giantswarm/retry-go](https://github.com/giantswarm/retry-go) - slightly @@ -102,16 +105,8 @@ nonintuitive interface (for me) Contributions are very much welcome. -### Makefile - -Makefile provides several handy rules, like README.md `generator` , `setup` for prepare build/dev environment, `test`, `cover`, etc... - -Try `make help` for more information. - ### Before pull request -> maybe you need `make setup` in order to setup environment - please try: * run tests (`make test`) * run linter (`make lint`) diff --git a/options.go b/options.go index e3dbd47..9a8003f 100644 --- a/options.go +++ b/options.go @@ -20,12 +20,19 @@ type OnRetryFunc func(attempt uint, err error) // The attempt parameter is the zero-based index of the attempt. type DelayTypeFunc func(attempt uint, err error, config *Config) time.Duration -// Timer represents the timer used to track time for a retry. +// Timer provides an interface for time operations in retry logic. +// This abstraction allows for mocking time in tests and implementing +// custom timing behaviors. The standard implementation uses time.After. type Timer interface { + // After returns a channel that sends the current time after the duration elapses. + // It should behave like time.After. After(time.Duration) <-chan time.Time } -// Config contains all retry configuration. +// Config holds all configuration options for retry behavior. +// It is typically populated using Option functions and should not be +// constructed directly. Use the various Option functions like Attempts, +// Delay, and RetryIf to configure retry behavior. type Config struct { attempts uint attemptsForError map[error]uint @@ -36,9 +43,9 @@ type Config struct { retryIf RetryIfFunc delayType DelayTypeFunc lastErrorOnly bool - context context.Context - timer Timer - wrapContextErrorWithLastError bool + context context.Context + timer Timer + wrapLastErr bool // wrap context error with last function error maxBackOffN uint } @@ -58,10 +65,10 @@ func (c *Config) validate() error { // Ensure we have required functions if c.retryIf == nil { - return fmt.Errorf("retryIf function cannot be nil") + return fmt.Errorf("retry if function cannot be nil") } if c.delayType == nil { - return fmt.Errorf("delayType function cannot be nil") + return fmt.Errorf("delay type function cannot be nil") } if c.timer == nil { return fmt.Errorf("timer cannot be nil") @@ -73,7 +80,9 @@ func (c *Config) validate() error { return nil } -// Option represents an option for retry. +// Option configures retry behavior. Options are applied in the order provided +// to Do or DoWithData. Later options override earlier ones if they modify the +// same configuration field. type Option func(*Config) func emptyOption(c *Config) {} @@ -380,8 +389,8 @@ func WithTimer(t Timer) Option { // retry.Attempts(0), // retry.WrapContextErrorWithLastError(true), // ) -func WrapContextErrorWithLastError(wrapContextErrorWithLastError bool) Option { +func WrapContextErrorWithLastError(wrap bool) Option { return func(c *Config) { - c.wrapContextErrorWithLastError = wrapContextErrorWithLastError + c.wrapLastErr = wrap } } diff --git a/retry.go b/retry.go index c0c064f..518070a 100644 --- a/retry.go +++ b/retry.go @@ -180,14 +180,21 @@ func DoWithData[T any](retryableFunc RetryableFuncWithData[T], opts ...Option) ( } attempt++ - // Wait for next attempt - delayDuration := delay(config, attempt, err) + // Wait for next attempt - inline delay calculation for performance + delayTime := config.delayType(attempt, err, config) + if delayTime < 0 { + delayTime = 0 + } + if config.maxDelay > 0 && delayTime > config.maxDelay { + delayTime = config.maxDelay + } + select { - case <-config.timer.After(delayDuration): + case <-config.timer.After(delayTime): case <-config.context.Done(): contextErr := context.Cause(config.context) if config.lastErrorOnly { - if config.wrapContextErrorWithLastError && err != nil { + if config.wrapLastErr && err != nil { return emptyT, fmt.Errorf("%w: %w", contextErr, err) } return emptyT, contextErr @@ -228,14 +235,25 @@ type Error []error // Error returns a string representation of all errors that occurred during retry attempts. // Each error is prefixed with its attempt number. func (e Error) Error() string { - logWithNumber := make([]string, len(e)) + if len(e) == 0 { + return "All attempts fail:" + } + + // Use strings.Builder for efficient string concatenation + var b strings.Builder + b.WriteString("All attempts fail:") + for i, err := range e { if err != nil { - logWithNumber[i] = fmt.Sprintf("#%d: %s", i+1, err.Error()) + b.WriteByte('\n') + b.WriteByte('#') + b.WriteString(fmt.Sprint(i + 1)) + b.WriteString(": ") + b.WriteString(err.Error()) } } - - return fmt.Sprintf("All attempts fail:\n%s", strings.Join(logWithNumber, "\n")) + + return b.String() } // Is reports whether any error in e matches target. @@ -315,30 +333,14 @@ func IsRecoverable(err error) bool { // Is implements error matching for unrecoverableError. // It supports errors.Is by checking if the target is also an unrecoverableError. func (unrecoverableError) Is(err error) bool { - _, isUnrecoverable := err.(unrecoverableError) - return isUnrecoverable + _, ok := err.(unrecoverableError) + return ok } func unpackUnrecoverable(err error) error { - if unrecoverable, isUnrecoverable := err.(unrecoverableError); isUnrecoverable { - return unrecoverable.error + if u, ok := err.(unrecoverableError); ok { + return u.error } - return err } -func delay(config *Config, attempt uint, err error) time.Duration { - delayTime := config.delayType(attempt, err, config) - - // Ensure delay is non-negative - if delayTime < 0 { - delayTime = 0 - } - - // Apply max delay cap if configured - if config.maxDelay > 0 && delayTime > config.maxDelay { - delayTime = config.maxDelay - } - - return delayTime -}