Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retry.DelayType gets inconsistent values #124

Open
haaawk opened this issue Jan 21, 2025 · 1 comment · May be fixed by #125
Open

retry.DelayType gets inconsistent values #124

haaawk opened this issue Jan 21, 2025 · 1 comment · May be fixed by #125

Comments

@haaawk
Copy link

haaawk commented Jan 21, 2025

Depending on whether retry.Attempts is set to 0 or something bigger than 0 the function given as retry.DelayType gets n either from 0,1,2,... or 1,2,3,....

Here's a reproducer:

/*
module go-retry-test

go 1.23

require github.com/avast/retry-go/v4 v4.6.0
*/
package main

import (
	"context"
	"errors"
	"time"
)
import "github.com/avast/retry-go/v4"

var count uint64 = 6

func action() error {
	if count == 0 {
		return nil
	}
	count -= 1
	return errors.New("something went wrong")
}

func main() {
	_ = retry.Do(action,
		retry.Attempts(3),
		retry.DelayType(func(n uint, err error, config *retry.Config) time.Duration {
			println("Delay with attempts != 0 -> ", n)
			return time.Second
		}),
		retry.LastErrorOnly(true),
		retry.Context(context.Background()))

	println("")

	_ = retry.Do(action,
		retry.Attempts(0),
		retry.DelayType(func(n uint, err error, config *retry.Config) time.Duration {
			println("Delay with attempts == 0 -> ", n)
			return time.Second
		}),
		retry.LastErrorOnly(true),
		retry.Context(context.Background()))
}

It prints:

Delay with attempts != 0 ->  0
Delay with attempts != 0 ->  1

Delay with attempts == 0 ->  1
Delay with attempts == 0 ->  2
Delay with attempts == 0 ->  3

I would expect the retry.DelayType function to either always get n from 0,1,2,... or always from 1,2,3,... but not one or the other depending on the other config parameters.

haaawk added a commit to haaawk/retry-go that referenced this issue Jan 21, 2025
Without this change DelayType function gets n either from
0,1,2,... or from 1,2,3,... depending on the value of Attempts
config value.

This change unifies the behaviour and makes n always take values from
0,1,2,...

Fixes avast#124

Signed-off-by: Piotr Jastrzebski <haaawk@gmail.com>
haaawk added a commit to haaawk/retry-go that referenced this issue Jan 21, 2025
Without this change DelayType function gets n either from
0,1,2,... or from 1,2,3,... depending on the value of Attempts
config value.

This change unifies the behaviour and makes n always take values from
1,2,3,...

Fixes avast#124

Signed-off-by: Piotr Jastrzebski <haaawk@gmail.com>
@haaawk
Copy link
Author

haaawk commented Jan 21, 2025

It seems that n=1,2,3,... is the right thing to do based on the documentation -> https://github.com/avast/retry-go/blob/master/options.go#L16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant