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

S3: NotFound error of HeadObject isn't handled properly #2880

Open
3 tasks done
harai opened this issue Mar 10, 2021 · 6 comments
Open
3 tasks done

S3: NotFound error of HeadObject isn't handled properly #2880

harai opened this issue Mar 10, 2021 · 6 comments
Labels
bug This issue is a bug. p2 This is a standard priority issue service-api This issue is due to a problem in a service API, not the SDK implementation.

Comments

@harai
Copy link

harai commented Mar 10, 2021

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 harai added the bug This issue is a bug. label Mar 10, 2021
@skmcgrail
Copy link
Member

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 skmcgrail added service-api This issue is due to a problem in a service API, not the SDK implementation. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Mar 11, 2021
@harai
Copy link
Author

harai commented Mar 11, 2021

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

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 12, 2021
@shawnHartsell
Copy link

shawnHartsell commented May 10, 2021

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 😢

@oakad
Copy link

oakad commented Jun 28, 2021

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.

@vudh1 vudh1 transferred this issue from aws/aws-sdk-go-v2 May 23, 2022
@vudh1 vudh1 self-assigned this May 23, 2022
@vudh1 vudh1 assigned RanVaknin and unassigned vudh1 Sep 9, 2022
@RanVaknin RanVaknin assigned debora-ito and unassigned indrora Mar 1, 2024
@kellertk
Copy link
Contributor

P126358787

@tim-finnigan tim-finnigan transferred this issue from aws/aws-sdk Oct 30, 2024
@zshzbh zshzbh added the p2 This is a standard priority issue label Nov 27, 2024
@zshzbh
Copy link

zshzbh commented Nov 27, 2024

Contacted service team for followup, waiting for response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. p2 This is a standard priority issue service-api This issue is due to a problem in a service API, not the SDK implementation.
Projects
None yet
Development

No branches or pull requests