service/s3: HeadObject waiter always retries on error, regardless of type

[goro9]

Acknowledgements

• I have searched (https://github.com/aws/aws-sdk/issues?q=is%3Aissue) for past instances of this issue
• I have verified all of my SDK modules are up-to-date (you can perform a bulk update with go get -u github.com/aws/aws-sdk-go-v2/...)

Describe the bug

It seems that the ObjectExistsStateRetryable used by default in NewObjectExistsWaiter is being retried for errors other than NotFound.

aws-sdk-go-v2/service/s3/api_op_HeadObject.go

Lines 915 to 930 in 3cc2195

	func objectExistsStateRetryable(ctx context.Context, input *HeadObjectInput, output *HeadObjectOutput, err error) (bool, error) {
	if err == nil {
	return false, nil
	}
	if err != nil {
	var errorType *types.NotFound
	if errors.As(err, &errorType) {
	return true, nil
	}
	}
	return true, nil
	}

Regression Issue

• Select this option if this issue appears to be a regression.

Expected Behavior

For NotFound errors: retry HeadObject
For errors other than NotFound: return err as is

Current Behavior

Always retry when an error occurs, regardless of the error type.

Reproduction Steps

options.Retryable = func(_ context.Context, _ *s3.HeadObjectInput, _ *s3.HeadObjectOutput, err error) (bool, error) {
	if err == nil {
		return false, nil
	}

	var errorType *types.NotFound
	if errors.As(err, &errorType) {
		return true, nil
	}

	return false, err
}

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

	github.com/aws/aws-sdk-go-v2/service/s3 v1.71.1

Compiler and Version used

go version go1.23.4 darwin/arm64

Operating System and version

macOS Sonoma 14.1

[adev-code]

Hello @goro9, thanks for reaching out and reporting the issue. I have replicated from my side and yes Go SDK is not retrying as it should. Meanwhile trying with Python SDK, it does make a retry. In this regard, I have brought this issue to the team for further review. I will let you know as soon as I get any updates. If you have any question let me know. Thanks.

[Madrigal]

Based on waiter workflow it seems like we're not doing the right thing. We need to address this and figure out if it's safe to roll out.

Need to also do some investigation around waiters on other services

[lucix-aws]

Waiter workflow step 4:

If none of the acceptors are matched and an error was encountered while calling the operation, then transition to the failure state and stop waiting.

Our implementation, for reasons probably lost to time, is just not doing this.

We can change it, trivially, but that comes with risk. Basically, any scenario where a waiter is called, some transient non-matching errors come back, and then a match does eventually come back would be broken. They would now fail on the unmatched error, as the spec says they should, instead of ignoring it. Whether or not anyone in the wild depends on that behavior, it's impossible to say.

The other option would be to let users enable the "on-spec" behavior through a new flag. Not at all a fan of having what would essentially be doThisCorrectly bool but it's the only way to avoid the aforementioned risk.