Skip to content

Possible to allow custom ShouldRetry rule over internal ones for retry handler? #207

@ningshanli

Description

@ningshanli

Hi!

Hope you are doing well, and thanks for contributing to this package :)

While I was going over retry_handler.go, I realize the handler only retries on three transient errors

const tooManyRequests = 429
const serviceUnavailable = 503
const gatewayTimeout = 504

Even if we set ShouldRetry option, it won't apply the rule unless the original error and request is a defined transient error.

func (middleware RetryHandler) retryRequest(ctx context.Context, pipeline Pipeline, middlewareIndex int, options retryHandlerOptionsInt, req *nethttp.Request, resp *nethttp.Response, executionCount int, cumulativeDelay time.Duration, observabilityName string) (*nethttp.Response, error) {
	if middleware.isRetriableErrorCode(resp.StatusCode) &&
		middleware.isRetriableRequest(req) &&
		executionCount < options.GetMaxRetries() &&
		cumulativeDelay < time.Duration(absoluteMaxDelaySeconds)*time.Second &&
		options.GetShouldRetry()(cumulativeDelay, executionCount, req, resp) {
                      ...
                 }
)

func (middleware RetryHandler) isRetriableErrorCode(code int) bool {
	return code == tooManyRequests || code == serviceUnavailable || code == gatewayTimeout
}

Wondering is it reasonable to make ShouldRetry a higher prioritized rule over the internal retryable check on error code and request? (like if the custom retry logic is defined use that, if not fallback to the internal ones). This would give client more flexibility while taking advantage of the internal retry handler.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    In Progress 🚧

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions