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

Error handling is STILL completely unreliable and unusable #5333

Closed
3 tasks done
Cyberax opened this issue Aug 20, 2021 · 12 comments
Closed
3 tasks done

Error handling is STILL completely unreliable and unusable #5333

Cyberax opened this issue Aug 20, 2021 · 12 comments
Assignees
Labels
bug This issue is a bug. needs-review This issue or pull request needs review from a core team member. service-api This issue is due to a problem in a service API, not the SDK implementation. workaround-available

Comments

@Cyberax
Copy link

Cyberax commented Aug 20, 2021

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

Describe the bug
Error handling for S3 is unreliable and useless.

Version of AWS SDK for Go?
Example: v1.8.1

Version of Go (go version)?
1.16

To Reproduce (observed behavior)
Run GetBucketLocation on a non-existing bucket:

	output, err := s3Conn.GetBucketLocation(ctx, &s3.GetBucketLocationInput{
		Bucket: aws.String("DoesNotExist1231232113425"),
	})
	var errNoSuchBucket *s3types.NoSuchBucket
	if err != nil && errors.As(err, &errNoSuchBucket) {
		return err
	}
        panic("SHOULD HAVE WORKED")

Expected behavior
The error should have been properly interpreted.

Additional context
The generated deserializer is not even reached and doesn't have any provisions to deserialize NoSuchBucket errors: https://github.com/aws/aws-sdk-go-v2/blob/3ddb4d670277f173ec60e271ced9a07b605fbc0c/service/s3/deserializers.go#L3467

The only mentions of NoSuchBucket handling is in ListObjects and ListObjectsV2 error handling.

@Cyberax Cyberax added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Aug 20, 2021
@KaibaLopez
Copy link
Contributor

Hi @Cyberax ,
Sorry for the lack of response here, but yea I can reproduce the error. Thanks for bringing it up to us.

@KaibaLopez KaibaLopez added needs-review This issue or pull request needs review from a core team member. and removed needs-triage This issue or PR still needs to be triaged. labels Sep 10, 2021
@MichaelCombs28
Copy link

This needs to be addressed for multiple services, not just s3.

@guseggert
Copy link

Given that AWS Go SDK V2 is GA now, is there a plan for fixing this while preserving backwards compat? This is a major shortcoming of V2 and I don't want my code to break when (if?) this is fixed, so I am inclined to stay on AWS Go SDK V1.

@Cyberax
Copy link
Author

Cyberax commented Feb 17, 2022

You can work around this issue by having a helper function to check the error codes. I've given up on waiting for AWS to fix this.

Here's my method:

type ErrCoder interface {
	ErrorCode() string
}

func CheckAwsError(err error, tgt ErrCoder) bool {
        if err == nil {
                return false;
        }
	var apiErr smithy.APIError
	if !errors.As(err, &apiErr) {
		return false
	}
	return apiErr.ErrorCode() == tgt.ErrorCode()
}

Using:

if CheckAwsError(err, &types.ConditionalCheckFailedException{}) { 
...
}

@itinance
Copy link

itinance commented May 7, 2022

Thank you @Cyberax ! You saved my day :)

@yerke
Copy link

yerke commented Jul 21, 2022

I am running into the same issue, but with DynamoDB APIs. It's a shame that this issue is not prioritized considering that this way of handling errors is suggested in migration guide.
Thanks to @Cyberax for figuring out a workaround.

@skmcgrail skmcgrail added the service-api This issue is due to a problem in a service API, not the SDK implementation. label Jul 21, 2022
@RanVaknin RanVaknin self-assigned this Jul 22, 2022
@RanVaknin
Copy link
Contributor

Hi @Cyberax and all.

Just to give an update. This is not GO SDK, or any AWS SDK specific issue. This is an S3 modeling issue that causes errors to not be mapped correctly from S3 to the respective SDKs.

This is still on the backlog, but requires the coordination of two major teams so takes time. I will raise this again with the service team in hopes that they can address it.

These two issues are somewhat related and stem from the same reason:
aws/aws-sdk#63
aws/aws-sdk-go-v2#2880

@RanVaknin
Copy link
Contributor

RanVaknin commented Jul 22, 2022

@yerke ,

Dynamo's API shouldn't have this behavior. Please cut a separate, detailed ticket, with clear reproduction steps and I will address it.
Thanks you.

@RanVaknin RanVaknin transferred this issue from aws/aws-sdk-go-v2 Jul 22, 2022
@yerke
Copy link

yerke commented Jul 23, 2022

@RanVaknin Here you go: aws/aws-sdk-go-v2#1773. Thanks for looking into it.

@RanVaknin
Copy link
Contributor

Created an internal ticket with the S3 team to add NoSuchBucket to the list of modeled errors for GetBucketLocation #P126361627.

Thanks,
Ran~

@lucix-aws
Copy link
Contributor

#5270

@lucix-aws lucix-aws closed this as not planned Won't fix, can't repro, duplicate, stale Oct 30, 2024
Copy link

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

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. needs-review This issue or pull request needs review from a core team member. service-api This issue is due to a problem in a service API, not the SDK implementation. workaround-available
Projects
None yet
Development

No branches or pull requests

10 participants