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

aws/retry: Allows use of closed network connection to be retryable #1826

Closed
wants to merge 1 commit into from

Conversation

satorunooshie
Copy link

@satorunooshie satorunooshie commented Aug 31, 2022

handled the use of closed network connection error as same as https://github.com/aws/aws-sdk-go.
ref: aws/aws-sdk-go#3476

#1825

For changes to files under the /model/ folder, and manual edits to autogenerated code (e.g. /service/s3/api.go) please create an Issue instead of a PR for those type of changes.

If there is an existing bug or feature this PR is answers please reference it here.

@satorunooshie satorunooshie requested a review from a team as a code owner August 31, 2022 05:39
@RanVaknin
Copy link
Contributor

We cannot accept this PR to override the Retryer behavior because not every API operation may be idempotent. The service API models don't have information to determine if an operation is idempotent, thus the SDKs cannot assume if it's safe to retry.

Proposed solution:
Just take your solution and implement in on the client level instead of the SDK level by overriding the default implementation of RetryableConnectionError

// your implementaion:
type RetryableConnectionError2 struct{}

func (r RetryableConnectionError2) IsErrorRetryable(err error) aws.Ternary {
	if err == nil {
		return aws.UnknownTernary
	}
	var retryable bool
	var conErr interface{ ConnectionError() bool }
	var tempErr interface{ Temporary() bool }
	var timeoutErr interface{ Timeout() bool }
	var urlErr *url.Error
	var netOpErr *net.OpError
	switch {
	case errors.As(err, &conErr) && conErr.ConnectionError():
		retryable = true

	case strings.Contains(err.Error(), "connection reset"),
		strings.Contains(err.Error(), "use of closed network connection"):
		retryable = true

	case errors.As(err, &urlErr):

		// Refused connections should be retried as the service may not yet be
		// running on the port. Go TCP dial considers refused connections as
		// not temporary.
		if strings.Contains(urlErr.Error(), "connection refused") {
			retryable = true
		} else {
			return r.IsErrorRetryable(errors.Unwrap(urlErr))
		}
	case errors.As(err, &netOpErr):
		// Network dial, or temporary network errors are always retryable.
		if strings.EqualFold(netOpErr.Op, "dial") || netOpErr.Temporary() {
			retryable = true
		} else {
			return r.IsErrorRetryable(errors.Unwrap(netOpErr))
		}
	case errors.As(err, &tempErr) && tempErr.Temporary():
		// Fallback to the generic temporary check, with temporary errors
		// retryable.
		retryable = true
	case errors.As(err, &timeoutErr) && timeoutErr.Timeout():
		// Fallback to the generic timeout check, with timeout errors
		// retryable.
		retryable = true
	default:
		return aws.UnknownTernary
	}
	return aws.BoolTernary(retryable)
}

func main(){
	cfg, err := config.LoadDefaultConfig(context.TODO(), config.WithRegion("us-east-1"), config.WithClientLogMode(aws.LogRequestWithBody|aws.LogResponseWithBody))
	if err != nil {
		log.Fatalf("unable to load SDK config, %v", err)
	}

	svc := dynamodb.NewFromConfig(cfg, func(options *dynamodb.Options) {
			options.Retryer = retry.NewStandard(func(opts *retry.StandardOptions) {
				opts.Retryables = append([]retry.IsErrorRetryable{RetryableConnectionError2{}}, opts.Retryables...)
			})
	})
}

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