aws-sdk-go-v2 icon indicating copy to clipboard operation
aws-sdk-go-v2 copied to clipboard

S3: `NotFound` error of `HeadObject` isn't handled properly

Open harai opened this issue 4 years ago • 6 comments

Confirm by changing [ ] to [x] below to ensure that it's a bug:

Describe the bug

	res, err := s3Client.HeadObject(ctx, &s3.HeadObjectInput{Bucket: &s3Bucket, Key: &s3Key})
	if err != nil {
		var notFoundError *type.NotFound
		if errors.As(err, &notFoundError) {
			// not executed
		}
	}

When the specified key doesn't exist, HeadObject returns Not Found. However, the error can't be handled properly using the code above.

Version of AWS SDK for Go?

1.2.0

Version of Go (go version)?

go version go1.15.3 linux/amd64

To Reproduce (observed behavior)

The code described above should be enough.

Expected behavior

errors.As(err, &notFoundError) returns true when the specified key doesn't exist.

harai avatar Mar 10 '21 11:03 harai

The HeadObject operation is special in that you can't use the the modeled NotFound exception type due to the limitations of the HTTP protocol. HTTP HEAD method responses can't contain response bodies per specification. To handle this error specifically in the SDK for this operation you will need to use the ResponseError type from smithy-go instead, and use the HTTPStatusCode method to determine if the error was a 404.

skmcgrail avatar Mar 11 '21 17:03 skmcgrail

Thanks. That inconsistency should be fixed. Until then, detailed documentation about this behavior will help us.

harai avatar Mar 11 '21 23:03 harai

Thanks. That inconsistency should be fixed. Until then, detailed documentation about this behavior will help us.

I am porting a service over from V1 to V2 and noticed that other function calls return ResponseError from smithy/http such as GetObject and CopyObject. In addition, the exact error type seems to be OperationError. I discovered this by trying to write a switch based type assertion.

This would be a breaking change, but I would consider not leaking types that come from a dependent package. This prevents callers from having to explicitly import the smithy package in their code. It also ties the SDK to a particular version of smithy. If a breaking change was made to the dependent type, all callers would need to upgrade 😢

shawnHartsell avatar May 10 '21 22:05 shawnHartsell

Smithy error type is not helpful if we need to distinguish between failure types (403 vs 404) and such. The only option right now is to cast to http.ResponseError and go from there.

This behavior may result in very non obvious bugs in cases where GetObject and HeadObject error paths get merged (not an uncommon use case) and thus must be documented in bold letters.

oakad avatar Jun 28 '21 10:06 oakad

P126358787

kellertk avatar Apr 16 '24 20:04 kellertk

Contacted service team for followup, waiting for response.

zshzbh avatar Nov 27 '24 18:11 zshzbh