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 #305

Open
3 tasks done
Cyberax opened this issue Aug 20, 2021 · 10 comments
Open
3 tasks done

Error handling is STILL completely unreliable and unusable #305

Cyberax opened this issue Aug 20, 2021 · 10 comments
Assignees
Labels
bug Something isn't working needs-review s3 service-api This issue pertains to the AWS API 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 Something isn't working needs-triage labels Aug 20, 2021
@KaibaLopez
Copy link

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

@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 pertains to the AWS API label Jul 21, 2022
@RanVaknin RanVaknin self-assigned this Jul 22, 2022
@RanVaknin
Copy link

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:
#63
#255

@RanVaknin
Copy link

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

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

Thanks,
Ran~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-review s3 service-api This issue pertains to the AWS API workaround-available
Projects
None yet
Development

No branches or pull requests

10 participants