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

DynamoDB/AWS SDK error handling doesn't work properly #1773

Closed
yerke opened this issue Jul 23, 2022 · 7 comments
Closed

DynamoDB/AWS SDK error handling doesn't work properly #1773

yerke opened this issue Jul 23, 2022 · 7 comments
Assignees
Labels
bug This issue is a bug. closed-for-staleness response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days.

Comments

@yerke
Copy link

yerke commented Jul 23, 2022

Describe the bug

We are calling DynamoDB like follows:

_, err = d.database.PutItem(
			ctx,
			&dynamodb.PutItemInput{
				Item:                item,
				TableName:           aws.String(decisionEventsTableName),
				ConditionExpression: aws.String("attribute_not_exists(#Token)"), // prevents overwrite
				ExpressionAttributeNames: map[string]string{
					"#Token": "Token", // Token is a keyword in aws
				},
			})

Then we are trying to handle exception

	if err != nil {
		var ccf *types.ConditionalCheckFailedException
		if errors.As(err, &ccf) { some_stuff_here() }
		return err
	}

This way of handling exception in aws-sdk-go-v2 is suggested in migration guide: https://aws.github.io/aws-sdk-go-v2/docs/handling-errors/#api-error-responses

Expected Behavior

I expect that if DynamoDB returns ConditionalCheckFailedException, errors.As call as written above to return true.

Current Behavior

The problem is that it doesn't work. I can see that that err contains *smithy.OperationError: operation error DynamoDB: PutItem, https response error StatusCode: 400, RequestID: UVDSN7JDGHJFC7AGUH3UNQQ4IFVV4KQNSO5AEMVJF66Q9ASUAAJG, ConditionalCheckFailedException: The conditional request failed, but errors.As(err, &ccf) does not return true.

Reproduction Steps

Please see code above

Possible Solution

I had to use a workaround as suggested in https://github.com/aws/aws-sdk-go-v2/issues/1386.

Additional Information/Context

It looks similar to https://github.com/aws/aws-sdk-go-v2/issues/1386.

I have a support case open with Case ID 10424576731.

AWS Go SDK V2 Module Versions Used

	github.com/aws/aws-sdk-go-v2 v1.16.7
	github.com/aws/aws-sdk-go-v2/config v1.15.0
	github.com/aws/aws-sdk-go-v2/credentials v1.12.4
	github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue v1.9.2
	github.com/aws/aws-sdk-go-v2/feature/dynamodb/expression v1.4.1
	github.com/aws/aws-sdk-go-v2/service/dynamodb v1.15.5

Compiler and Version used

go1.17.6

Operating System and version

Linux, CentOS 7

@yerke yerke added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 23, 2022
@RanVaknin RanVaknin self-assigned this Jul 25, 2022
@RanVaknin
Copy link
Contributor

Hi, Thanks for opening the ticket.
I'll try to repro and see what I can find.

@RanVaknin
Copy link
Contributor

RanVaknin commented Jul 28, 2022

Hi @yerke ,

As I mentioned on the separate thread you referenced. This is not a known error with the DynamoDB API.

I am not able to reproduce your issue. Since your code is missing a lot of things like what your item looks like, the client initialization and more.. It's hard for me to get a concrete idea of what might be the problem on your end.

Here is my complete code

package main

import (
	"context"
	"errors"
	"fmt"
	"github.com/aws/aws-sdk-go-v2/aws"
	"github.com/aws/aws-sdk-go-v2/config"
	"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue"
	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
	"github.com/aws/aws-sdk-go-v2/service/dynamodb/types"
)

type Office struct {
	Season  int
	Episode int
	Foo     string
}

func main() {
	cfg, err := config.LoadDefaultConfig(
		context.Background(),
		config.WithRegion("us-east-1"),
	)
	if err != nil {
		panic(fmt.Sprintf("Error configuring AWS: %s", err))
	}

	db := dynamodb.NewFromConfig(cfg)

	item, err := attributevalue.MarshalMap(&Office{
		Season:  11,
		Episode: 11,
		Foo:     "bar",
	})
	if err != nil {
		fmt.Println(err)
	}

	putItemInput := dynamodb.PutItemInput{
		TableName:           aws.String("TEST_TABLE"),
		Item:                item,
		ConditionExpression: aws.String("attribute_not_exists(#Foo)"),
		ExpressionAttributeNames: map[string]string{
			"#Foo": "Foo",
		},
	}

	putItemOutput, err := db.PutItem(context.Background(), &putItemInput)
	if err != nil {
		var ccf *types.ConditionalCheckFailedException
		if errors.As(err, &ccf) {
			fmt.Println("this is a smithy generated error")
		}
		return
	}

	fmt.Printf("%v\n", putItemOutput)
}

The first time the code ran it created the record in DDB, after I changed the value of Foo and executed the file again the output was as expected:

This is a smithy generated error

Let me know if this helps.
Thanks

@RanVaknin RanVaknin added response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. and removed needs-triage This issue or PR still needs to be triaged. labels Jul 28, 2022
@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 the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jul 31, 2022
@yerke
Copy link
Author

yerke commented Jul 31, 2022

@RanVaknin I will try to make a smaller repro for you.

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Aug 1, 2022
@RanVaknin RanVaknin added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Aug 1, 2022
@github-actions
Copy link

github-actions bot commented Aug 3, 2022

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 Aug 3, 2022
@github-actions github-actions bot closed this as completed Aug 9, 2022
@DennisPing
Copy link

Commenting for any future readers. What tripped me up was "double pointer" thing as @RanVaknin showed.

// Doing an update. If Key doesn't exist, just add a new item. Return nothing.

_, err = d.Client.UpdateItem(ctx, &dynamodb.UpdateItemInput{
	Key: map[string]types.AttributeValue{
		"userId": &types.AttributeValueMemberN{Value: strconv.Itoa(userId)},
	},
	TableName:                 aws.String("myTableName"),
	ExpressionAttributeNames:  expr.Names(),
	ExpressionAttributeValues: expr.Values(),
	UpdateExpression:          expr.Update(),
	ReturnValues:              types.ReturnValueNone,
})
if err != nil {
	var opErr *smithy.OperationError
	if errors.As(err, &opErr) {
		switch opErr.Unwrap().(type) {
		case *types.ProvisionedThroughputExceededException:
			// handle update rate too high
		case *types.RequestLimitExceeded:
			// handle ran out of money
		default:
			// handle some other operational error
		}
	}
	return fmt.Errorf("unexpected error: %w", err)
}
return nil

@ilovejs
Copy link

ilovejs commented Dec 20, 2023

no, above example would not work for ConditionalCheckFailedException casting.
also, switch opErr.Unwrap().(type) is not so good piece of code..
what if err.Error(), it print without any err detail e.g. fields,
detail was truncated. as far as i can tell it ends with :

@RanVaknin

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. closed-for-staleness 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

4 participants