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 - PutItem, returns (nil, err) on ConditionalCheckFailedException. #2183

Closed
heathedavid opened this issue Jul 11, 2023 · 5 comments
Closed
Assignees
Labels
bug This issue is a bug. needs-review This issue or pull request needs review from a core team member. p2 This is a standard priority issue workaround-available

Comments

@heathedavid
Copy link

Describe the bug

As of Jun 30 2023 the press release:

https://aws.amazon.com/about-aws/whats-new/2023/06/amazon-dynamodb-cost-failed-conditional-writes/

It appears if PutItem fails with a ConditionalCheckFailedException a copy of the items wil be returned (if the appopriate flag is set on the input).

Upon testing, PutItem return (nil, err) on ConditionalCheckFailedException

Example is not provided (but can be upon request) as inspection of the latest source code shows:

https://github.com/aws/aws-sdk-go-v2/blob/service/dynamodb/v1.20.0/service/dynamodb/api_op_PutItem.go#L34

result, metadata, err := c.invokeOperation(ctx, "PutItem", params, optFns, c.addOperationPutItemMiddlewares)
if err != nil {
return nil, err
}

Namley on ALL errors nil is returned.

Expected Behavior

PutItem to return the Item upon a ConditionalCheckFailedException

Current Behavior

PutItem returns nil upon a ConditionalCheckFailedException

Reproduction Steps

uidRec := struct {
	PK  string `dynamodbav:"pk"`
	SK  string `dynamodbav:"sk"`
	UID string `dynamodbav:"uid"`
}{
	PK:  naturalKeyPartitionKey(groupID),
	SK:  naturalKey,
	UID: uid,
}

uidItem, err := attributevalue.MarshalMap(uidRec)
if err != nil {
	return "", fmt.Errorf("marshalling ID: %w", err)
}

// PK condition is sufficient - as it first tries and finds a row which matches
// the primary key - if it exits it will have a PK and SK attribute.
cond := expression.AttributeNotExists(expression.Name(xdb.PK))

expr, err := expression.NewBuilder().WithCondition(cond).Build()
if err != nil {
	return "", fmt.Errorf("failed to create Builder: %w", err)
}
// Create item - using conflict

response, err := dbMan.db.Client().PutItem(ctx, &dynamodb.PutItemInput{
	Item:                                uidItem,
	ConditionExpression:                 expr.Condition(),
	ExpressionAttributeNames:            expr.Names(),
	ExpressionAttributeValues:           expr.Values(),
	ReturnValues:                        types.ReturnValueAllOld,
	ReturnValuesOnConditionCheckFailure: types.ReturnValuesOnConditionCheckFailureAllOld,
	TableName:                           aws.String(dbMan.table),
})

if err != nil {
	var err1 *types.ConditionalCheckFailedException
	if errors.As(err, &err1) {
		if response != nil { //BUG - Support case opened with AWS
			uid, ok := response.Attributes[UID].(*types.AttributeValueMemberS)
			if !ok || len(uid.Value) == 0 {
				return "", err
			}
			log.Printf("UID for natural key %s already exists", naturalKey)
			return uid.Value, nil
		}
	}
	return "", err
}
log.Printf("UID for natural key %s created", naturalKey)

Possible Solution

No response

Additional Information/Context

No response

AWS Go SDK V2 Module Versions Used

SDK Version/Language: github.com/aws/aws-sdk-go-v2/service/dynamodb v1.20.0

Compiler and Version used

go version go1.19.6 darwin/arm64

Operating System and version

Mac OS

@heathedavid heathedavid added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 11, 2023
@RanVaknin RanVaknin self-assigned this Jul 15, 2023
@RanVaknin
Copy link
Contributor

Hi @heathedavid,

Thanks for bringing this to our attention.

The value isn't printed because of a deserialization issue.
While we investigate this, you can get around this for now by inspecting the raw http response sent back from the service by enabling logs:

	_, err = client.PutItem(context.TODO(), input, func(options *dynamodb.Options) {
		options.ClientLogMode = aws.LogResponseWithBody
	})

Thanks,
Ran~

@RanVaknin RanVaknin added p2 This is a standard priority issue needs-review This issue or pull request needs review from a core team member. workaround-available and removed needs-triage This issue or PR still needs to be triaged. labels Jul 17, 2023
@RanVaknin
Copy link
Contributor

Hi,

After looking a bit into this, there is no deserialization issue.

Namley on ALL errors nil is returned.

This is the intended behavior - this follows the golang idiom of a function's returned values and errors being mutually exclusive.

So in this particular case, the failed items were added as a field on the error response itself - see the new definition of ConditionalCheckFailedException:

type ConditionalCheckFailedException struct {
	Message *string
	ErrorCodeOverride *string
	Item map[string]AttributeValue // this is new
}

In practice, this means the new field Item is accessible as follows:

if err != nil {
	var err1 *types.ConditionalCheckFailedException
	if errors.As(err, &err1) {
	    // err1.Item is usable here
	}
}

Thanks,
Ran~

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

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.

@heathedavid
Copy link
Author

heathedavid commented Jul 25, 2023 via email

@heathedavid
Copy link
Author

heathedavid commented Jul 28, 2023 via email

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. p2 This is a standard priority issue workaround-available
Projects
None yet
Development

No branches or pull requests

2 participants