Skip to content

Conversation

@StounhandJ
Copy link
Contributor

This MR addresses an issue where a delay is still applied after the final attempt, even when the maximum number of attempts for a specific error type has been reached.

Problem:
When using the retry.AttemptsForError() option to define a custom attempt limit for a specific error type, the retry logic still applies the configured delay after the final permitted attempt for that error. This results in unnecessary wait time and inconsistent behavior.

Steps to Reproduce:

pacakge main

var count uint64 = 0
var currentTime = time.Now()

type TestError struct {
}

func (TestError) Error() string {
	return ""
}

func action() error {
	count += 1
	fmt.Printf("Attempts: %v; Time: %v\n", count, time.Since(currentTime).Round(time.Second))
	currentTime = time.Now()

	return TestError{}
}

func main() {
	_ = retry.Do(action,
		retry.Attempts(3),
		retry.Delay(2*time.Second),
		retry.DelayType(retry.FixedDelay),
		retry.AttemptsForError(2, TestError{}),
		retry.LastErrorOnly(true),
		retry.Context(context.Background()))

	fmt.Printf("End Time: %v\n", time.Since(currentTime).Round(time.Second))
}

Observed Output:

Attempts: 1; Time: 0s
Attempts: 2; Time: 2s
End Time: 2s

Expected Output:

Attempts: 1; Time: 0s
Attempts: 2; Time: 2s
End Time: 0s

tstromberg added a commit to codeGROOVE-dev/retry that referenced this pull request Jul 28, 2025
tstromberg added a commit to codeGROOVE-dev/retry that referenced this pull request Jul 28, 2025
merge: No delay after final retry on max attempts avast#129
@JaSei
Copy link
Collaborator

JaSei commented Oct 14, 2025

Thanks for this PR. I apologize for the delayed response. To be honest, I switched jobs a while back and moved to a completely different tech stack, so I wasn't able to give this project the attention it deserved. I'm catching up now, though, and I'm happy to merge this. Thanks again for your work!

@JaSei JaSei merged commit 9e5d0d6 into avast:master Oct 14, 2025
17 checks passed
@codecov
Copy link

codecov bot commented Oct 14, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (cbf4dbf) to head (e330bce).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #129       +/-   ##
=========================================
- Coverage   90.86%      0   -90.87%     
=========================================
  Files           2      0        -2     
  Lines         230      0      -230     
=========================================
- Hits          209      0      -209     
+ Misses         17      0       -17     
+ Partials        4      0        -4     
Flag Coverage Δ
unittest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@StounhandJ
Copy link
Contributor Author

Thanks for this PR. I apologize for the delayed response. To be honest, I switched jobs a while back and moved to a completely different tech stack, so I wasn't able to give this project the attention it deserved. I'm catching up now, though, and I'm happy to merge this. Thanks again for your work!

I am very happy for you with your new job. I hope I can still help with this or another library.

project-mirrors-bot-tu bot pushed a commit to project-mirrors/forgejo-runner that referenced this pull request Oct 15, 2025
This PR contains the following updates:

| Package | Change | Age | Confidence |
|---|---|---|---|
| [github.com/avast/retry-go/v4](https://github.com/avast/retry-go) | `v4.6.1` -> `v4.7.0` | [![age](https://developer.mend.io/api/mc/badges/age/go/github.com%2favast%2fretry-go%2fv4/v4.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/go/github.com%2favast%2fretry-go%2fv4/v4.6.1/v4.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>avast/retry-go (github.com/avast/retry-go/v4)</summary>

### [`v4.7.0`](https://github.com/avast/retry-go/releases/tag/v4.7.0)

[Compare Source](avast/retry-go@4.6.1...4.7.0)

#### What's Changed

- support context cancel cause by [@&#8203;NivKeidan](https://github.com/NivKeidan) in [#&#8203;117](avast/retry-go#117)
- Bump github.com/stretchr/testify from 1.10.0 to 1.11.1 by [@&#8203;dependabot](https://github.com/dependabot)\[bot] in [#&#8203;136](avast/retry-go#136)
- Add FullJitterBackoffDelay  by [@&#8203;amirrezafahimi](https://github.com/amirrezafahimi) in [#&#8203;128](avast/retry-go#128)
- No delay after final retry on max attempts by [@&#8203;StounhandJ](https://github.com/StounhandJ) in [#&#8203;129](avast/retry-go#129)
- BackOffDelay multiplies attempts from zero by [@&#8203;StounhandJ](https://github.com/StounhandJ) in [#&#8203;130](avast/retry-go#130)
- add Go version 1.25 to test matrix for expanded compatibility testing by [@&#8203;JaSei](https://github.com/JaSei) in [#&#8203;142](avast/retry-go#142)

#### New Contributors

- [@&#8203;NivKeidan](https://github.com/NivKeidan) made their first contribution in [#&#8203;117](avast/retry-go#117)
- [@&#8203;amirrezafahimi](https://github.com/amirrezafahimi) made their first contribution in [#&#8203;128](avast/retry-go#128)
- [@&#8203;StounhandJ](https://github.com/StounhandJ) made their first contribution in [#&#8203;129](avast/retry-go#129)

**Full Changelog**: <avast/retry-go@4.6.1...v4.7.0>

</details>

---

### Configuration

📅 **Schedule**: Branch creation - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC), Automerge - Between 12:00 AM and 03:59 AM ( * 0-3 * * * ) (UTC).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS4xNDYuMCIsInVwZGF0ZWRJblZlciI6IjQxLjE0Ni4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJLaW5kL0RlcGVuZGVuY3lVcGRhdGUiLCJydW4tZW5kLXRvLWVuZC10ZXN0cyJdfQ==-->

Reviewed-on: https://code.forgejo.org/forgejo/runner/pulls/1092
Reviewed-by: earl-warren <[email protected]>
Co-authored-by: Renovate Bot <[email protected]>
Co-committed-by: Renovate Bot <[email protected]>
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 this pull request may close these issues.

2 participants