Cannot call `batchGetItem` with a table name containing a dot (.)

DomDerrien commented 1 year ago

Checkboxes for prior research

[X] I've gone through Developer Guide and API reference
[X] I've checked AWS Forums and StackOverflow.
[X] I've searched for previous similar issues and didn't find any solution.

Describe the bug

I have a series of DynamoDB tables with a dot in their name: game.Quiz, game.ChartRider, etc.

The name of the table seems used as-is in a string used for validation and the dot causes an invalid indirection. The first of the validation reports:

ValidationException: 1 validation error detected: Value '{}' at 'requestItems' failed to satisfy constraint: Member must have length greater than or equal to 1

Changing the dot by a dash allows to pass the validation but the request fails b/c the table does not exist.

If I pass the name with the dash and set an empty array for { RequestItems: { 'game-Quiz': { Keys: [] }}, I get the error message:

ValidationException: 1 validation error detected: Value '[]' at 'requestItems.game-Quiz.member.keys' failed to satisfy constraint: Member must have length greater than or equal to 1

I suspect there's some literal parsing that does not take into account table names with a dot!

SDK version number

@aws-sdk/util-dynamodb@3.417.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.0.0

Reproduction Steps

The code is meant to run in a NodeJS Lambda function.

    async batchGet(ids: string[], accumulator: any[] = [], projectionExpression?: string): Promise<any[]> {
        const command = new BatchGetItemCommand({
            RequestItems: {
                [this.tableName]: {
                    Keys: ids.map((id: string): Record<string, AttributeValue> => {
                        const { pk, sk } = decomposeId(id);
                        return marshall({ pk, sk}, { convertEmptyValues: true, removeUndefinedValues: true });
                    }),
                    ProjectionExpression: projectionExpression,
                },
            },
        });

        const { Responses } = await getDocumentClient().send(command);

        for (const record of Responses[this.tableName]) {
            accumulator.push(this.entityClass.getInstance().fromDdb(unmarshall(record)));
        }

        return accumulator;
    }

The code above is simplified and does not show how the UnprocessedKeys payload is processed. It does not show neither how the array of identifiers is sliced to send multiple queries of a maximum of 100 identifiers.

The code is part of a generic class with produced typed entities (thanks to this.entityClass.getInstance().fromDdb(...)). Outside this characteristic, it basically uses the AWS NodeJS SDV v3 for DynamoDB.

In my situation, this.tableName receives values with a dot (like game.Quiz).

Observed Behavior

With a table where I replace the dot by a dash, I can see an error message as

ValidationException: 1 validation error detected: Value '[]' at 'requestItems.game-Quiz.member.keys' failed to satisfy constraint: Member must have length greater than or equal to 1

With a dot, I suspect it produces a pattern like requestItems.game.Quiz and this is why the validation of the requestItems.game is reported as invalid when it should look at requestItems['game.Quiz']...

Expected Behavior

As table names with a dot is a valid format, I expect the validation to let my request go. All other CRUD operations are fine so far.

Possible Solution

I would expect the code validating the content of requestItems to use the key game.Quiz (retrieved with Object.keys()) as-is instead of parsing the generated path requestItems.game.Quiz and reporting a miss for request.game...

Additional Information/Context

I haven't got the time to look at the SDK code just yet. I hope my understanding of the issue based on the seen error messages is accurate.

In the meantime, I go with individual GetItemCommand calls but that very inefficient.

RanVaknin commented 1 year ago

Hi @DomDerrien ,

Thanks for reaching out. Unfortunately Im not able to reproduce your issue. I'm able to run a BatchGetItemCommand on a table name game.Quiz successfully:

import { DynamoDBClient, BatchGetItemCommand } from "@aws-sdk/client-dynamodb";
import { marshall, unmarshall } from "@aws-sdk/util-dynamodb";

const client = new DynamoDBClient({ region: "us-east-1" });

async function batchGetItems() {

  const keys = [
    { pk: "Quiz1", sk: "Question1" },
    { pk: "Quiz1", sk: "Question2" },
  ];

  try {
    const { Responses } = await client.send(new BatchGetItemCommand({
        RequestItems: {
          "game.Quiz": {
            Keys: keys.map(key => marshall(key))
          }
        }
      }));
    const items = Responses["game.Quiz"].map(item => unmarshall(item));
    console.log(items);
  } catch (error) {
    console.log(error)
  }
}

batchGetItems()

/*
[
  {
    question: 'baz',
    sk: 'Question1',
    pk: 'Quiz1',
    answer: 'biz'
  },
  {
    question: 'foo',
    sk: 'Question2',
    pk: 'Quiz1',
    answer: 'bar'
  }
]
*/

One thing I've noticed is that you might be calling the document client in conjunction with BatchGetItemCommand here:

        const { Responses } = await getDocumentClient().send(command); // command = BatchGetItemCommand

Since you didnt include a full code snippet with imports and client initialization, I can only speculate that getDocumentClient() will return an instance of the document client (@aws-sdk/lib-dynamodb) which does not have a BatchGetItemCommand.

If my suspicion is correct, you need to either stick with the base dynamodb client, or use the BatchGetCommand from the Document client, but you cannot mix the two.

Side Note: I recommend that you initialize the SDK client once, in the global scope instead of initializing it per API call. Each client instance maintains its own connection pool, and in high workload environment or environments with shared state like Lambda, you might reach file descriptor limits.

Please let me know if this helps. Thanks, Ran~

github-actions[bot] commented 1 year ago

This issue has not received a response in 1 week. If you still think there is a problem, please leave a comment to avoid the issue from automatically closing.

DomDerrien commented 1 year ago

Hi @RanVaknin, thanks for the feedback.

Here is the code of my getDocumentClient():

import { DynamoDBClient } from '@aws-sdk/client-dynamodb';

let ddbClient: DynamoDBClient;

export function getDocumentClient(region = process.env.AWS_REGION): DynamoDBClient {
    if (!ddbClient) {
        ddbClient = new DynamoDBClient({
            region,
            // @ts-ignore - prop is not recognized
            profile: process.env.AWS_PROFILE_NAME,
        });
    }
    return ddbClient;
}

It seems I followed the same steps you provided. I wonder why you don't have the validation error though...

I'll try to reproduce it in a standalone test case as you did. I'm just swamped now because we have a release pushed to the app stores at the end of the week (a big milestone from alpha to beta).

RanVaknin commented 1 year ago

Hi @DomDerrien ,

I'm still having a hard time understanding what the issue is because your code snippet is partial. You are not importing BatchGetItemCommand from client-dyanmodb. My guess is your code is de-structured across multiple files?

The best way to help you debug this is for you to upload a sample repository and share it with us so we can clone and help you debug it.

If I had to guess, based on the validation exception, your Keys parameter might include empty values. I would do a null check before marshalling.

Thanks, Ran~

DomDerrien commented 1 year ago

HI @RanVaknin,

Tl;dr: my mistake due to a misuse of the returned UnprocessedKeys value!

In a standalone version inspired by your code above, I was able to get the results I expected. I verified too that I could pass a ProjectExpression and got partial results as expected.

While going further in reviewing my code, I found the issue's root cause:

I was misusing the UnprocessedKeys value!
I trusted the Typescript definition mentioning it as optional and only checked with if (!UnprocessedKeys) { ...}
However, the documentation for that field mentions: If there are no unprocessed keys remaining, the response contains an empty UnprocessedKeys map.
I modified the test to be if (Object.keys(UnprocessedKeys) === 0) { ... }
Now, the submission of a second request with an empty RequestItems map is prevented!

So the error message is valid, it just wasn't issued for my first request, it was related to a ghost request my code thought it had to submit... Because I saw an error message with 'requestItems.game.Quiz.member.keys' for a table game.Quiz, I got stuck on the wrong root cause (tunnel vision).

Thanks for your support and patience. Dom

github-actions[bot] commented 1 year ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.

aws / aws-sdk-js-v3