aws / aws-sdk-js-v3

Modularized AWS SDK for JavaScript.
Apache License 2.0
3.04k stars 569 forks source link

DynamoDb no longer throwing an exception for undefined attributes on PUT (and `removeUndefinedValues` being ignored) #6433

Open peteraiken opened 1 week ago

peteraiken commented 1 week ago

Checkboxes for prior research

Describe the bug

In previous versions of the aws-sdk (specifically @aws-sdk/lib-dynamodb), the default behaviour when providing undefined values to DynamoDb PUT operations was to throw an exception with the message Pass options.removeUndefinedValues=true to remove undefined values from map/array/set.. This was the equivalent behaviour of initialising the DB Client with removeUndefinedValues: false.

Since release 3.429.0, this is no longer the case - undefined values provided to DynamoDb operations are instead just removed, which is the equivalent of removeUndefinedValues: true. Manually setting removeUndefinedValues: false has no effect.

I believe this has been caused by this change which was included as a bug fix for release 3.429.0.

SDK version number

@aws-sdk/lib-dynamodb@3.429.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v20.14.0

Reproduction Steps

Running the below code in @aws-sdk/lib-dynamodb@3.428.0 and below will fail with error message Pass options.removeUndefinedValues=true to remove undefined values from map/array/set.. Running the below code in @aws-sdk/lib-dynamodb@3.429.0 and above will succeed, removing the undefined attributes.

import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocumentClient, PutCommand } from "@aws-sdk/lib-dynamodb";

// Initialize DynamoDB client and Document client
const client = new DynamoDBClient({ region: "us-east-1" });
const ddbDocClient = DynamoDBDocumentClient.from(client);

const putItem = async () => {
  const params = {
    TableName: "YourTableName",
    Item: {
      PK: "123",           // Primary Key
      SK: "456",           // Sort Key
      someAttribute: undefined,  // Undefined attribute
    },
  };

  try {
    await ddbDocClient.send(new PutCommand(params));
    console.log("Item successfully put.");
  } catch (err) {
    console.error("Error putting item:", err);
  }
};

putItem();

Observed Behavior

In @aws-sdk/lib-dynamodb@3.428.0 and below, this operation would throw an exception with the error message Pass options.removeUndefinedValues=true to remove undefined values from map/array/set., unless removeUndefinedValues: true is set in the DynamoDb instance config. In @aws-sdk/lib-dynamodb@3.429.0 and above, the operation will succeed, and will have removed the undefined values from the record altogether - regardless of the removeUndefinedValues setting.

Expected Behavior

I would expect the behaviour to be consistent - that the operation would throw an exception with the error message Pass options.removeUndefinedValues=true to remove undefined values from map/array/set., unless removeUndefinedValues: true is set in the DynamoDb instance config.

Possible Solution

No response

Additional Information/Context

No response

aBurmeseDev commented 1 week ago

Hi @peteraiken - thanks for reaching out.

The behavior you're observing is actually the expected and intended behavior. Previously, the users were running into TypeError (in this issue) until the PR you mentioned corrected the behavior. As explained in the PR, the previous behavior was incorrect because an undefined column in a DynamoDB row is not the same as an undefined member in a map or set. An undefined column cannot be resolved to any specific AttributeValue type.

The fix implemented in the PR was to support top-level undefined AttributeValues and also validate nested values appropriately. This change ensures that undefined columns are handled correctly, aligning with the expected behavior of the DynamoDB service.

You can also refer to this comment by our team member from previous issue.

Let us know if you have further questions. Best, John

peteraiken commented 1 week ago

Thanks @aBurmeseDev - I appreciate this is definitely smoother behaviour, I raised it for us because we had some functionality that we intentionally wanted to fail if a value was undefined and on upgrading the package, this stopped happening. Of course we can get around this by adding our own null checks before the operation, but good to know this is an intentional change.

Just to clarify - does this mean that removeUndefinedValues now only applies (and should only apply) to internal properties of maps/sets, and not to top-level attributes?