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

lib-dynamodb does not marshall binary in v3.515.0 when using AWS Lambda provided SDK #5868

Open
3 tasks done
grant-d opened this issue Mar 7, 2024 · 9 comments
Open
3 tasks done
Assignees
Labels
bug This issue is a bug. p2 This is a standard priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet. workaround-available This issue has a work around available.

Comments

@grant-d
Copy link

grant-d commented Mar 7, 2024

Checkboxes for prior research

Describe the bug

Summary: In recent versions of JS packages, Dynamo marshalls Uint8Array as List instead of Binary

I redeployed a Node20 lambda today (in eu-central-1) that had no changes to the data layer. But as of that deploy, marshalling Uint8Array and/or Buffer serializes to a List and not Binary. I have rolled back entirely without success, which leads me to believe the latest SDKs are deployed to lambda already.

The problem seems suspiciously timeous with this recent PR, which changes how data is marshalled. Perhaps unrelated, but it does have changed logic around creating Lists instead of Binary (and also changes re convertClassInstanceToMap which would seem related to the symptoms below):
e1ba507#diff-8853e9557dcc53577bcb4bd057ea592df1aa7d4bddf478a16024fcfac3e7ce85

Here is a basic repro:

import { DynamoDBClient } from '@aws-sdk/client-dynamodb'
import { DynamoDBDocumentClient, TranslateConfig } from '@aws-sdk/lib-dynamodb'
import { UpdateCommand, UpdateCommandInput } from '@aws-sdk/lib-dynamodb'

const client = new DynamoDBClient({ apiVersion: 'latest', region: 'eu-central-1` })

const config: TranslateConfig = {
  marshallOptions: { 
    convertClassInstanceToMap: true, // There are relevant changes in PR related to this option
    removeUndefinedValues: true
  }
}

const dbClient = DynamoDBDocumentClient.from(client, config)

const foo = new Uint8Array([1, 2, 3])
await client.send(new UpdateCommand({
   ... "SET #foo = :foo"
   ... // Eliding idiomatic values
  ["#foo] = "foo",
  [":foo"] = foo
})`

Here is the previous result (still working on stamps that I have not re-deployed yet)
They were built using 3.513.0
image

Here is the problematic result:
Built using 3.523.0
image
Notice how it's a List of Maps
image

As a matter of diligence - even though our code has not changed - I logged the type of the value at the last possible moment before calling into DynamoDb, to ensure that we are passing the expected value through:

console.debug('type', { name, type: typeof value, length: value?.length, ctor: value?.constructor?.name })
image

SDK version number

"@aws-sdk/lib/util/client-dynamodb" "^3.523.0"

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Node 20 on AWS Lambda

Reproduction Steps

See main comments

Observed Behavior

Binary is serialized as a List of Maps.

Expected Behavior

Binary is serialized as a Binary

Possible Solution

No response

Additional Information/Context

No response

@grant-d grant-d added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 7, 2024
@grant-d grant-d changed the title DynamoDb does not marshall binary since recent PR DynamoDb does not marshall binary since circa 3.513.0 Mar 7, 2024
@grant-d
Copy link
Author

grant-d commented Mar 7, 2024

Update:
I made a single change which 'fixed' the problem, meaning Uint8Array now serializes correctly (as Binary). I would infer that the problem is thus in the SDK and not in 'my' code, since (as previously stated) we've had no changes in this data layer code for several weeks, and either way a Buffer or Uint8Array should be treated as Binary and not as a Class (using specific meaning of Class in marshalling context)

However, this is not a suitable fix, even temporarily, since we need the relevant option's behavior elsewhere

    marshallOptions: {
      convertClassInstanceToMap: false, // Was true
      removeUndefinedValues: true
    }
image

@RanVaknin
Copy link
Contributor

Hi @grant-d,

Thanks for the in depth analysis.

which leads me to believe the latest SDKs are deployed to lambda already.

You are correct. We will look into it with priority.

Thanks,
Ran~

@RanVaknin RanVaknin self-assigned this Mar 7, 2024
@RanVaknin RanVaknin added investigating Issue is being investigated and/or work is in progress to resolve the issue. p1 This is a high priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Mar 7, 2024
@grant-d grant-d changed the title DynamoDb does not marshall binary since circa 3.513.0 lib-dynamodb does not marshall binary since circa 3.513.0 Mar 7, 2024
@kuhe kuhe self-assigned this Mar 8, 2024
@kuhe
Copy link
Contributor

kuhe commented Mar 8, 2024

The affected version is the version supplied by AWS Lambda, which you may be seeing in v3.515.0 of the SDK's packages.

The fix in AWS SDKs was rolled out in 3.523.0, which is not deployed to AWS Lambda automatically. Are you able to include the dynamodb packages in your function deployment as a workaround?

@kuhe
Copy link
Contributor

kuhe commented Mar 8, 2024

Depending on how you are deploying, there may be a step that removes AWS SDK from your uploaded code under the assumption that it is provided by Lambda, so even if you have v3.523.0 in your code it may be running v3.515.0 on Lambda.

If you could shut off this step, or otherwise ensure v3.523.0 or higher is deployed to your Lambda function to override the default, it should bring in the fix.

@kuhe
Copy link
Contributor

kuhe commented Mar 8, 2024

To confirm the desired behavior, this is the test code I'm running:

(using TypeScript with a mix of import/require)

import { DynamoDB } from "@aws-sdk/client-dynamodb";
import { DynamoDBDocument } from "@aws-sdk/lib-dynamodb";
const client_metadata = require("@aws-sdk/client-dynamodb/package.json");
const util_metadata = require("@aws-sdk/util-dynamodb/package.json");
const lib_metadata = require("@aws-sdk/lib-dynamodb/package.json");

(async () => {
  console.log("versions:", {
    client: client_metadata.version,
    util: util_metadata.version,
    lib: lib_metadata.version,
  });

  const ddb = new DynamoDB({ region: "us-west-2" });
  const doc = DynamoDBDocument.from(ddb, {
    marshallOptions: {
      convertClassInstanceToMap: true,
    },
  });

  await doc.put({
    TableName: "test",
    Item: {
      id: "bits",
      data: new Uint8Array([1, 2, 3, 4]),
    },
  });

  const get = await doc.get({
    TableName: "test",
    Key: {
      id: "bits",
    },
    ConsistentRead: true,
  });

  console.log(get);
  console.log("get.Item.data should be Uint8Array(4) [ 1, 2, 3, 4 ].");
})();

@grant-d
Copy link
Author

grant-d commented Mar 8, 2024

Awesome, thanks @kuhe, I will try deploy the specific version of the sdk and let you know.

[Edit]. That worked. For anyone else who happens to hit this issue, and you are deploying a Node 18+ function using CDK and built-in bundling, make sure to do this:

// package.json
{
  "devDependencies": {
    "@aws-sdk/client-dynamodb": "^3.529.1", // Delete this and all its friends from here...
    "@aws-sdk/lib-dynamodb": "^3.529.1",
    "@aws-sdk/util-dynamodb": "^3.529.1"
  },
  "dependencies": {
    "@aws-sdk/client-dynamodb": "^3.529.1", // ... and add them here instead
    "@aws-sdk/lib-dynamodb": "^3.529.1",
    "@aws-sdk/util-dynamodb": "^3.529.1"
  }
}

// lambda.ts
const lambda = new lmb.NodejsFunction(this, 'Lambda123', {
  ...
  bundling: {
    ...
    externalModules: [
      ...
      // Use the 'aws-sdk' available in the Lambda runtime
      'aws-sdk', // Comment this line out
      // Use the 'aws-sdk-v3' available in the Lambda runtime (ONLY on Node 18+)
      '@aws-sdk' // Comment this line out
    ]
  }
}

@kuhe kuhe added workaround-available This issue has a work around available. and removed investigating Issue is being investigated and/or work is in progress to resolve the issue. labels Mar 8, 2024
@kuhe kuhe changed the title lib-dynamodb does not marshall binary since circa 3.513.0 lib-dynamodb does not marshall binary in v3.515.0 when using AWS Lambda provided SDK Mar 8, 2024
@kuhe
Copy link
Contributor

kuhe commented Mar 8, 2024

We will work on applying the fix to AWS Lambda provided AWS SDK for JavaScript v3, but I do not have a time estimate for releasing the fix.

The recommendation remains to supply your own AWS SDK version in uploaded Lambda functions https://docs.aws.amazon.com/lambda/latest/operatorguide/sdks-functions.html.

@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Mar 8, 2024
@grant-d
Copy link
Author

grant-d commented Mar 15, 2024

@kuhe has this been deployed to lambda yet ?

@kuhe
Copy link
Contributor

kuhe commented Mar 18, 2024

The Lambda deployment is in progress but not completed yet. It can take a few weeks.

@RanVaknin RanVaknin added p2 This is a standard priority issue and removed p1 This is a high priority issue labels Apr 22, 2024
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. p2 This is a standard priority issue pending-release This issue will be fixed by an approved PR that hasn't been released yet. workaround-available This issue has a work around available.
Projects
None yet
Development

No branches or pull requests

3 participants