aws / aws-sdk-js

AWS SDK for JavaScript in the browser and Node.js
https://aws.amazon.com/developer/language/javascript/
Apache License 2.0
7.59k stars 1.55k forks source link

DocumentClient constructor in codebase takes in more arguments than mentioned in documentation #4489

Closed caoAZ closed 11 months ago

caoAZ commented 1 year ago

Describe the bug

Providing a maxRetries value and/or a timeout value to DocumentClient constructor will not have any effect.

The documentation [Link](https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/AWS/DynamoDB/DocumentClient.html#constructor-property:~:text=DynamoDB.updateItem().-,Constructor%20Details,-new%20AWS.DynamoDB) does not mention that it is possible to provide maxRetries or timeout as possible arguments, however they are still valid arguments according to the codebase Link (need to traverse down to ConfigurationOptions Link), which we think might be a mistake.

Expected Behavior

Expected to have some retry or timeout effect when provided into the DocumentClient constructor as it is possible to do so in the codebase.

Current Behavior

According to the codebase it is possible to have such constructor for DocumentClient, for example:

const dynamodb = new AWS.DynamoDB();
const documentClient = new AWS.DynamoDB.DocumentClient({
    service: dynamodb,
    maxRetries: 4,
    httpOptions: {
        timeout: 6
    }
})

Reproduction Steps

When providing such constructor for DocumentClient:

const dynamodb = new AWS.DynamoDB();
const documentClient = new AWS.DynamoDB.DocumentClient({
    service: dynamodb,
    maxRetries: 4,
    httpOptions: {
        timeout: 6
    }
})

The timeout/retries would not have any effect.

Possible Solution

Not include DynamoDB.Types.ClientConfiguration in DocumentClient constructor within codebase.

Additional Information/Context

No response

SDK version used

V2 (2.610.0)

Environment details (OS name and version, etc.)

Mac on Ventura 13.5.1

ajredniwja commented 12 months ago

Hi @caoAZ, thanks for opening this issue, when I try to run the code with a constructor like that I am able to see that it does take retires in account.

    const request = docClient.scan(params);
    request.on('httpHeaders', (statusCode, headers, response, request) => {
        console.log('Request:', request);
        console.log('Response:', response);
    });

Max retires is set to 4 when using a constructor like that otherwise it is set to 10.

github-actions[bot] commented 11 months 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.

caoAZ commented 11 months ago

Hi @ajredniwja are you positive? To test out our theory, we gave a timeout value of 4ms within the documentClient constructor the dynamodb call succeeded without fail for any operation (ie query/put) but when we put the same timeout value within the dynamodb constructor, we immediately see failures for any of the same operation.

Previously, we had a timeout value of 1.5s with 4 retries within the documentClient constructor and we would get timeout errors on a weekly basis b/c our API Gateway has a timed-out value of 30 sec and I believe DynamoDb has a default value of 2mins. But when we moved the same timeout and retries value to the dynamoDb constructor, from our logs we don't see any dynamoDB timeout errors anymore.

It could be something else, but I'm putting my experience here so it could be helpful to someone.