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

What's the best way to handle not found error? #1110

Closed
2 tasks done
csandanov opened this issue Feb 4, 2021 · 14 comments
Closed
2 tasks done

What's the best way to handle not found error? #1110

csandanov opened this issue Feb 4, 2021 · 14 comments
Assignees
Labels
closed-for-staleness guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@csandanov
Copy link

csandanov commented Feb 4, 2021

I found the following example in CHANGELOG:

// To get a specific API error
var notFoundErr *types.ResourceNotFoundException
if errors.As(err, &notFoundErr) {
    log.Printf("scan failed because the table was not found, %v", notFoundErr.ErrorMessage())
}

So I tried this with RDS:

_, err := client.DescribeDBInstances(ctx, &rds.DescribeDBInstancesInput{
    DBInstanceIdentifier: aws.String(identifier),
})
if err != nil {
    var notFoundErr *types.DBInstanceNotFoundFault
    // Doesn't work.
    // I also tried using types.ResourceNotFoundFault.
    if errors.As(err, &notFoundErr) {
         // 
    }
}

This works:

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
     //
}

Does this work as intended? Should I get 404 from response status code if I want to identify not found error?

@csandanov csandanov added the guidance Question that needs advice or information. label Feb 4, 2021
@csandanov csandanov changed the title Handling API not found error from examples doesn't work What's the best way to handle not found error? Feb 4, 2021
@csandanov
Copy link
Author

I ended up using this:

if opErr, ok := err.(*smithy.OperationError); ok {
	if httpErr, ok := opErr.Err.(*awsHttp.ResponseError); ok {
		if httpErr.HTTPStatusCode() == http.StatusNotFound {
			// 
		}
	}
}

@KaibaLopez
Copy link
Contributor

Hi @csandanov sorry, I'm not clear on what it is you were asking for but it looks like you were able to find your answer or is there another question you've got?

@KaibaLopez KaibaLopez self-assigned this Feb 4, 2021
@KaibaLopez KaibaLopez added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Feb 4, 2021
@skotambkar
Copy link
Contributor

Hi @csandanov, thanks for reaching out!

From your provided sample code, I think the problem is use of &apiErr, instead of &notFoundErr when using errors.As.

    var notFoundErr *types.DBInstanceNotFoundFault
    if errors.As(err, &notFoundErr) {
         // 
    }
 

Let us know if that solves the issue.

@csandanov
Copy link
Author

csandanov commented Feb 5, 2021

@skotambkar I'm sorry that's just a typo in the example I wrote here, it doesn't work. Now fixed

@KaibaLopez I was just looking for a simple way to handle not found errors, I saw example in the CHANGELOG and thought this was really neat but in the end it didn't work, I thought maybe there's another correct way to handle it since there's a types.DBInstanceNotFoundFault. Right now I check error for smithy.OperationError, fetch http error from operation error and check its code for 404, it works but it doesn't feel right

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

jasdel commented Mar 3, 2021

Hi @csandanov could you provide log output of the error message your application receiving that you want to capture? Also log the error value returned by the operation method.

// Add logging for a operation to stdout
resp, err := client.client.DescribeDBInstances(ctx, &rds.DescribeDBInstancesInput{
    DBInstanceIdentifier: aws.String(identifier),
    func (o *dynamodb.Options) {
         o.ClientLogMode: aws.LogResponseBody
    }
})

// Log error in detail
log.Printf("DescribeDBInstances error, %#v", err)

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

This issue has not received a response in 1 week. If you want to keep this issue open, please just leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added closing-soon This issue will automatically close in 4 days unless further comments are made. closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Mar 11, 2021
@csandanov
Copy link
Author

I got a similar issue with IAM, that's what the print gave me:

operation error IAM: GetRole, https response error StatusCode: 404,

@d122y
Copy link

d122y commented Nov 18, 2021

First of all, a huge shout out to @csandanov for posting solution! I know it's a closed issue but because it's one of the top google results for how to handle errors in aws-sdk-go-v2, and I think @csandanov enquiry was very valid due to lack of official documentation (not counting this, as it's somewhat incomplete), this is what I came up with... If anyone has a better way to implement switch for actual error cause please let me know, as I'm not quite happy with complexity of this solution.

if err != nil {
	if opErr, ok := err.(*smithy.OperationError); ok {
		if httpErr, ok := opErr.Err.(*awshttp.ResponseError); ok {
			switch httpErr.Err.(type) {
			case *types.ProvisionedThroughputExceededException:
			...
			case *types.RequestLimitExceeded:
			...
			}
		}
	}
}

@jasdel
Copy link
Contributor

jasdel commented Feb 28, 2022

Thanks for posting the example. Another way you could identify the underlying API error would be to use errors.As on smithy-go.APIError type. Since all API error types generated by the SDK will match this interface.

if err != nil {
	var apiErr smithy.APIError
	if errors.As(err, &apiErr) {
		switch apiErr.(type) {
		case *types.ProvisionedThroughputExceededException:
		...
		case *types.RequestLimitExceeded:
		...
		}
	}
}

Generally the SDK will layer API response error for operation calls in the following pattern:

  • smithy.OperationError - provides metadata about the operation call and client. Always present in failed operation call errors.
  • if s3 or s3control, a ResponseError type with a ServiceHostID() string method.
  • awshttp.ResponseError - provides metadata about the HTTP response, adds RequestID. Only present for HTTP request/response errors. error.As(err, ...) can match both awshttp.ResponseError and smithyhttp.ResponseError, if present
    • smithyhttp.ResponseError - embedded within awshttp.ResponseError.
  • type implementing smithy.APIError interface - concrete API response error values. Types matching APIError will only be present for API response error message received from service.

Each one of these layers can be searched for via the top level via the errors.As.

var apiErr smithy.APIError
if errors.As(err, &apiErr) {
      fmt.Println("APIError:", apiErr.ErrorCode())
}

var httpResponseErr *awshttp.ResponseError
if errors.As(err, &httpResponseErr) {
     fmt.Println("HTTP Response error requestID:", httpResponseErr.ServiceRequestID())
     fmt.Println("HTTP Response error status code: ", httpResponseErr.HTTPStatusCode())
}

@esdandreu
Copy link

Just for the reference, in order to use @jasdel example:

var httpResponseErr *awshttp.ResponseError

Assumes

import awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"

@Schachte
Copy link

Schachte commented Jan 6, 2023

This is fully working for me. I feel v1 SDK was a bit more clear, but nonetheless.

// Upload will upload an object and automatically create the bucket if the upload fails with NoSuchBucket
func Upload(client s3.Client, object *s3.PutObjectInput) error {
	bucket := *object.Bucket

	bucketCreation := &s3.CreateBucketInput{
		Bucket: aws.String(bucket),
	}

	// Attempt to upload the object, this will work in the majority of cases for
	// pre-existing customers and fail otherwise.
	putObjResponse, err := client.PutObject(context.Background(), object)

	if err != nil {
		var apiErr smithy.APIError
		if errors.As(err, &apiErr) {
                         // The comment above walks through retrieving the error code text. This is a constant 'NoSuchBucket'
			if apiErr.ErrorCode() == NO_SUCH_BUCKET {
				_, err = client.CreateBucket(context.Background(), bucketCreation)
				if err != nil {
					return err
				}
			}
		}
	}

	// If the bucket created without fail, we reattempt object upload
	if putObjResponse == nil {
		_, err = client.PutObject(context.Background(), object)
	}

	return err
}

@aajtodd
Copy link
Contributor

aajtodd commented Jan 10, 2023

@Schachte

Your code can be simplified to look more like v1 as pointed out above.

	var apiErr smithy.APIError
	if errors.As(err, &apiErr) {
		switch apiErr.(type) {
		case *s3types.NoSuchBucket:
			log.Fatalf("no such bucket: %v\n", err)
		default:
			log.Fatalf("unknown error: %v\n", err)
		}
	}

Unwrapping the error looks nearly identical as branching on error code but has the advantage of getting a concrete error type and access to any specific fields of that error. You also don't need to know specific error codes. Hope that helps.

@Schachte
Copy link

@aajtodd Looks much cleaner, cheers!

@milosgajdos
Copy link

milosgajdos commented Apr 28, 2023

As far as I can see in my code, I can't switch type smithy.APIError into either of *s3types.NoSuchKey, *s3types.NotFound when using HeadObject:

		_, err := s3Client.HeadObject(ctx, keyPath)
		if err != nil {
			logger.Errorf("%T", err) // *smithy.OperationError

			var apiErr smithy.APIError
			if errors.As(err, &apiErr) {
				logger.Errorf("S3 error: %T", apiErr) //  *smithy.GenericAPIError

				switch apiErr.(type) {
				case *s3types.NoSuchKey, *s3types.NotFound:
                                        logger.Errorf("%T", err) // NEVER GETS LOGGED
				default:
					logger.Errorf("%T", err) // *smithy.OperationError
				}
		}

Edit:
I just discovered #2084

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-for-staleness guidance Question that needs advice or information. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.
Projects
None yet
Development

No branches or pull requests

9 participants