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

3.209.0 introduces marshalling issue #4222

Closed
3 tasks done
adrai opened this issue Nov 23, 2022 · 13 comments
Closed
3 tasks done

3.209.0 introduces marshalling issue #4222

adrai opened this issue Nov 23, 2022 · 13 comments
Assignees
Labels
bug This issue is a bug. closed-for-staleness workaround-available This issue has a work around available.

Comments

@adrai
Copy link

adrai commented Nov 23, 2022

Checkboxes for prior research

Describe the bug

Updating from 3.208.0 to 3.209.0 has issues when the retrieved object in dynamodb contain null values.

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

const ddbClient = DynamoDBDocumentClient.from(new DynamoDBClient({ region: 'eu-west-1' }), {
  marshallOptions: {
      removeUndefinedValues: true
  }
})

const org = (await ddbClient.send(new GetCommand({
  TableName: 'my-table-for-organisations',
  Key: { id }
}))).Item
console.log(org)
// org is looking like this in dynamodb:
// {
//  "id": "b512877f-e373-454f-87ee-7385c5ef51dc",
//  "address": {
//     "city": "Perugia",
//     "country": "IT",
//     "line1": "Via XYZ",
//     "postalCode": "06121",
//     "state": null
//   },
//   taxNumber: null
// }

generates:

TypeError: Cannot read properties of null (reading 'S')
    at AttributeValueFilterSensitiveLog (/var/task/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1328:13)
    at /var/task/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1414:128
    at Array.reduce (<anonymous>)

SDK version number

@aws-sdk/client-dynamodb@3.209.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v16.13.1

Reproduction Steps

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

const ddbClient = DynamoDBDocumentClient.from(new DynamoDBClient({ region: 'eu-west-1' }), {
  marshallOptions: {
      removeUndefinedValues: true
  }
})

const org = (await ddbClient.send(new GetCommand({
  TableName: 'my-table-for-organisations',
  Key: { id }
}))).Item
console.log(org)
// org is looking like this in dynamodb:
// {
//  "id": "b512877f-e373-454f-87ee-7385c5ef51dc",
//  "address": {
//     "city": "Perugia",
//     "country": "IT",
//     "line1": "Via XYZ",
//     "postalCode": "06121",
//     "state": null
//   },
//   taxNumber: null
// }

Observed Behavior

generates:

TypeError: Cannot read properties of null (reading 'S')
    at AttributeValueFilterSensitiveLog (/var/task/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1328:13)
    at /var/task/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1414:128
    at Array.reduce (<anonymous>)

Expected Behavior

should marshall without errors

Possible Solution

downgrade to 3.208.0

Additional Information/Context

see #4222 (comment)

maybe also related: #3846, #4237

@adrai adrai added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Nov 23, 2022
@bluepeter
Copy link

bluepeter commented Nov 24, 2022

Experiencing this issue as well. We are getting frustrated by the continuing cadence of serious bugs in v3... almost to the point of going back to v2.

@adrai
Copy link
Author

adrai commented Nov 24, 2022

fyi: changing this line seems to fix it

[key]: AttributeValueFilterSensitiveLog(value),

-        [key]: AttributeValueFilterSensitiveLog(value),
+        [key]: value ? AttributeValueFilterSensitiveLog(value) : undefined,

seems value can also be null

// cc: @kuhe @trivikr

@yenfryherrerafeliz yenfryherrerafeliz self-assigned this Nov 27, 2022
@tushar-sg
Copy link

facing the same issue in the latest "3.216.0".

@avegao
Copy link

avegao commented Nov 27, 2022

I can confirm that latest version 3.216.0 is still broken and with 3.208 works as expected

@yenfryherrerafeliz yenfryherrerafeliz added the investigating Issue is being investigated and/or work is in progress to resolve the issue. label Nov 27, 2022
@yenfryherrerafeliz
Copy link
Contributor

Hi @adrai, thanks for opening this issue. I can confirm this is a bug and seems to be happening due to the loggerMiddleware, we need to do a deeper investigation to understand what the problem is, however, as workaround we could either turn off the info logger by doing the following:
Just pass the following parameter to the client:

 logger: {
     info: undefined
 }

Example:

const ddbClient = DynamoDBDocumentClient.from(
    new DynamoDBClient({
        region: 'us-east-2',
        logger: {
            info: undefined
        }
    }), {
    marshallOptions: {
        removeUndefinedValues: true
    }
});

Or you could also use the DynamoDBClient without DynamoDBDocumentClient.

I will provide more updates as soon as get them.

Thanks!

@yenfryherrerafeliz yenfryherrerafeliz added needs-review This issue/pr needs review from an internal developer. 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. needs-triage This issue or PR still needs to be triaged. labels Nov 27, 2022
@yenfryherrerafeliz
Copy link
Contributor

Repro Steps:

  • Put a record with a null value into a dynamodb table:
import {DynamoDBClient, PutItemCommand} from '@aws-sdk/client-dynamodb'
import { DynamoDBDocumentClient, GetCommand } from '@aws-sdk/lib-dynamodb'

const ddbClient = DynamoDBDocumentClient.from(
    new DynamoDBClient({
        region: 'us-east-2'
    }), {}
);
const response = await ddbClient.send(new PutItemCommand({
    TableName: 'test-table',
    Item: {
        name: {
            NULL: true
        },
        id: {
            S: 'UID-10202020202021'
        }
    }
}))

console.log(response);
  • Then, lets try to get that same item:
import {DynamoDBClient, PutItemCommand} from '@aws-sdk/client-dynamodb'
import { DynamoDBDocumentClient, GetCommand } from '@aws-sdk/lib-dynamodb'

const ddbClient = DynamoDBDocumentClient.from(
    new DynamoDBClient({
        region: 'us-east-2',
    }), {
    marshallOptions: {
        removeUndefinedValues: true
    }
});
const response = await ddbClient.send(new GetCommand({
    TableName: 'test-table',
    Key: {
        id: 'UID-10202020202021'
    }
}))

console.log(response);
  • And we get:
PROJECT_DIR/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1328
    if (obj.S !== undefined)
            ^

TypeError: Cannot read properties of null (reading 'S')
    at AttributeValueFilterSensitiveLog (PROJECT_DIR/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1328:13)
    at PROJECT_DIR/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1414:128
    at Array.reduce (<anonymous>)
    at GetItemOutputFilterSensitiveLog (PROJECT_DIR/node_modules/@aws-sdk/client-dynamodb/dist-cjs/models/models_0.js:1414:40)
    at PROJECT_DIR/node_modules/@aws-sdk/middleware-logger/dist-cjs/loggerMiddleware.js:16:21
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async file://PROJECT_DIR/src/getItem.js:12:18

Node.js v18.3.0

@adrai
Copy link
Author

adrai commented Nov 29, 2022

@yenfryherrerafeliz any update on this? Isn't my proposed solution good enough? #4222 (comment)

@yenfryherrerafeliz
Copy link
Contributor

Hello @adrai, I have not updates yet. I have reviewed your proposed solution and I agree it could fix the problem, however, this still needs to be reviewed by the team. I will provide updates as soon as possible.

Thanks!

@RanVaknin
Copy link
Contributor

@yenfryherrerafeliz ,

This was brought up here and it is a known issue and assigned a p0. Someone is working on a fix for it so feel free to close this ticket in favor of tracking it on the initial ticket.

@avegao @adrai @bluepeter @tushar-sg Please comment on issue as well so it will be clear that this is impacting more people.

Thank you,
Ran~

@adrai
Copy link
Author

adrai commented Nov 29, 2022

@RanVaknin since the used SDK version is different, I don't thing the root cause of that issue is the same...
Would love to keep both open in that case.
3.209.0 introduced that issue for me... and the other issue is related to 3.142.0

@asnaeb
Copy link

asnaeb commented Nov 30, 2022

Facing this issue as well. A temporary solution while the cause has been found might as well be changing

if (obj.S !== undefined) return { S: obj.S };

and its siblings to

if (obj?.S !== undefined) return { S: obj.S };

@kuhe
Copy link
Contributor

kuhe commented Dec 2, 2022

opened a PR with a fix: #4249

@kuhe kuhe self-assigned this Dec 2, 2022
@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed needs-review This issue/pr needs review from an internal developer. labels Dec 6, 2022
@github-actions github-actions bot added closed-for-staleness and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Dec 7, 2022
@github-actions github-actions bot closed this as completed Dec 7, 2022
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. closed-for-staleness workaround-available This issue has a work around available.
Projects
None yet
Development

No branches or pull requests

8 participants