[retry] Add centralized retry/backoff abstraction

[Pranav-d33]

cc @leecalcote Introduces retry/ — a context-aware, exponential-backoff retry package wrapping cenkalti/backoff/v4. Provides a reusable abstraction to replace ad-hoc retry loops across Meshery components.

Changes:

• Add retry/{retry,options,errors,retry_test}.go
  • Do(ctx, op, ...opts) with production-safe defaults (500 ms initial, x1.5 growth, +/-30% jitter, 2 min cap)
  • Permanent(err) escape hatch for non-retryable errors
  • IsPermanent(err) using errors.As for proper chain unwrapping
  • WithLogNotifier bridges retries to MeshKit logger.Handler
  • 15 table-driven tests, race-detector clean
• Promote cenkalti/backoff/v4 v4.3.0 from indirect to direct in go.mod
  fixes Feature Request: Centralized Retry/Backoff Helper in MeshKit #1008

[welcome]

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, you can find updates in the #github-notifications channel in the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while performing a commit.

[gemini-code-assist]

Code Review

This pull request introduces a centralized, context-aware exponential backoff retry package wrapping github.com/cenkalti/backoff/v4. It includes a configurable Do function, functional options for tuning backoff parameters, and helpers for handling permanent errors. Feedback suggests upgrading to backoff/v5 for consistency, improving the documentation examples to use context-aware HTTP calls, and refining the Operation signature to include a context. Additionally, suggestions were made to optimize the Do function by checking the context before the first attempt, simplifying the internal retry call, and tightening test assertions for permanent errors.

[Pranav-d33]

cc @leecalcote Wanted to flag this is ready for review when you get a chance — happy to walk through the approach or adjust based on feedback.

[icoderarely]

LGTM, might need a maintainers eye on it now for this to be merged if this is a fix they want. Good work @Pranav-d33

[Pranav-d33]

@icoderarely Can you point me to the right person to ping ???

[icoderarely]

hey, did you not create an issue or forgot to link this pr to an issue you are working on?

and the one who assigned you might be the first person you could tag.

Also have some patience, it's a big project, alot of work is happening in parallel and maintainers/members need time too to get back to each. So best bet would be to tag, wait for a day, if there is no response msg this on slack community that you need this pr to be reviewed/merged.

[Pranav-d33]

The issue is in Meshery/meshery and lee was the one who assigned it to me, thats why i pinged him in slack...
meshery/meshery#19275

[icoderarely]

oh we gotta tag someone else then (some exceptions mate)

[icoderarely]

@Omkar-Ugal who could be the right person to tag?

[Omkar-Ugal]

please link the pr with the issue

[Pranav-d33]

please link the pr with the issue

it is linked, meshery/meshery#19275 , the issue is in meshery actually, do i create a new issue in meshkit as well?

[matrixkavi]

The change for go.mod is not required.

While majority of the code looks ok to me, I was wondering what would happen if a caller called WithMultiplier or WithJitter with negative values for instance. Isn't it good to validate and set the values to default rather than blindly setting them in options.go?

[Pranav-d33]

Added validation guards:

• WithMultiplier: values <= 0 now fall back to DefaultMultiplier (1.5)
• WithJitter: values < 0 or > 1 now fall back to DefaultRandomizationFactor (0.3)

Also reverted the unnecessary go.mod change.

[lekaf974]

Just a quick commen, LGTM otherwise

[Pranav-d33]

@matrixkavi , Let me know if any changes are needed before merging.

[icoderarely]

LGTM for

retry: drop unnecessary uint64->uint cast by aligning Config.MaxAttempts type with backoff library

[lekaf974]

LGTM

[leecalcote]

There is the smallest amount of discourse on this topic in the community Slack - https://mesheryio.slack.com/archives/CFGG6U10E/p1779755449286469?thread_ts=1776923231.353449&cid=CFGG6U10E

[leecalcote]

@Pranav-d33 this might be something that we can wire into the remote provider availability tests that occur concurrently onload of the Provider UI.

[Pranav-d33]

MeshKit Retry Abstraction Test Matrix

Category 1: Basic Success Paths

Scenario	What it tests	Result
TestRetrySucceedsFirstAttempt	Operation succeeds on the first call with no retries.	PASS (0.00s) — operation called exactly once
TestRetrySucceedsAfterTransientErrors	Operation fails 3 times and succeeds on the 4th attempt.	PASS (0.01s) — operation called exactly 4 times
TestRetryDefaultsAreApplied	Verifies default retry settings are applied when no options are provided.	PASS (0.36s) — at least 2 calls made

Category 2: Permanent Error Handling

Scenario	What it tests	Result
TestRetryPermanentErrorStopsImmediately	Permanent errors stop retries immediately.	PASS (0.00s) — exactly 1 call
TestIsPermanentReturnsFalseForTransient	Transient errors are not classified as permanent.	PASS (0.00s)
TestIsPermanentReturnsTrueForPermanentWrapped	Wrapped permanent errors are correctly detected.	PASS (0.00s)
TestIsPermanentHandlesDoublyWrappedErrors	Nested wrapped permanent errors still unwrap correctly.	PASS (0.00s)

Category 3: Context Cancellation

Scenario	What it tests	Result
TestRetryContextCancellationStopsLoop	Retry loop stops when context is cancelled mid-backoff.	PASS (0.01s)
TestRetryContextAlreadyCancelledBeforeFirstAttempt	Retry exits immediately if context is already cancelled.	PASS (0.00s)

Category 4: Budget Enforcement

Scenario	What it tests	Result
TestRetryMaxAttemptsEnforced	Retry count respects WithMaxAttempts(4).	PASS (0.01s) — exactly 4 calls
TestRetryMaxElapsedTimeEnforced	Retry loop respects wall-clock elapsed time budget.	PASS (0.08s)
TestRetryZeroMaxAttemptsMeansUnlimited	Unlimited retries stop only via elapsed time budget.	PASS (0.04s)
TestRetryWithMaxAttemptsOneNoRetry	WithMaxAttempts(1) performs exactly one attempt.	PASS (0.00s)

Category 5: Notifier Callback Behavior

Scenario	What it tests	Result
TestRetryNotifierCalledOnEachRetry	Notifier invoked once per transient retry failure.	PASS (0.01s)
TestRetryNotifierNotCalledOnImmediateSuccess	Notifier is not called on immediate success.	PASS (0.00s)
TestRetryNotifierNotCalledOnPermanentError	Notifier is not called for permanent errors.	PASS (0.00s)

Summary

• Total scenarios executed: 16
• Passed: 16/16
• Total runtime: 0.36s
• Coverage includes:
  • transient retry behavior
  • exponential backoff defaults
  • permanent error semantics
  • context cancellation
  • retry budget enforcement
  • notifier callback behavior
  • wrapped error handling

[reaper8055]

I have reviewed the changes, please check the comments.

[Pranav-d33]

@reaper8055 ready for re-review.

[reaper8055]

The config validation is a good addition, but the returned errors are currently string-only.

Right now callers can only distinguish invalid retry configuration from an operation failure by inspecting the error message. That is brittle. Since retry.Do can now fail before the operation even runs, callers should have a reliable way to detect that class of failure.

I’d suggest exposing a sentinel error, for example:

var ErrInvalidConfig = errors.New("retry: invalid config")

Then wrap it from validateConfig:

return fmt.Errorf("%w: InitialInterval must be > 0, got %v", ErrInvalidConfig, cfg.InitialInterval)

This lets consumers write:

if errors.Is(err, retry.ErrInvalidConfig) {
    // fail startup / reject bad config
}

[reaper8055]

The classifier API has good tests, but it also needs a compiled documentation example.

WithErrorClassifier is now an important part of the public contract because it lets callers separate retry-loop mechanics from domain-specific error classification. However, the current ExampleDo demonstrates retry.Permanent inside the operation, not the classifier path.

I’d add an example like ExampleWithErrorClassifier that shows the intended usage:

err := retry.Do(ctx, op,
    retry.WithErrorClassifier(func(err error) retry.ErrorDecision {
        var status statusError
        if errors.As(err, &status) {
            if status.Code >= 500 || status.Code == http.StatusTooManyRequests {
                return retry.DecisionRetry
            }
            return retry.DecisionStop
        }
        return retry.DecisionRetry
    }),
)

This matters because otherwise users may keep embedding permanent/transient decisions inside every operation body. The cleaner contract is: the operation returns the domain error, and the classifier decides whether that error is retryable.

A compiled example also protects the public API from drift because go test will fail if the documented usage stops compiling.

IMPORTANT: check with @leecalcote / @lekaf974 on repo policy to where to include code examples for such use case!

[reaper8055]

IMPORTANT: check with @leecalcote / @lekaf974 on repo policy to where to include code examples / demos for such use case!

[reaper8055]

Everything looks good here apart from small things, check the comments.

[lekaf974]

Not sure that demo is needed

files added after my approval, I'll take another look

[lekaf974]

Please return meshkit errors from retry package

[banana-three-join]

Nothing major really sticks out to me. I think it looks good!