Incorrect interface types in Amazon CloudFront clients

piotrekwitkowski commented 3 weeks 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

Many of the required strings in the client's models are incorrectly typed as string | undefined, but required in practice. They should be typed as string instead.

This applies for all commands of the client:

There are 13 occurrences of such bug across these 6 methods.

SDK version number

@aws-sdk/client-cloudfront@3.590.0 @aws-sdk/client-cloudfront-keyvaluestore@3.590.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Node v18.19.0

Reproduction Steps

In the @aws-sdk/client-cloudfront:

import { CloudFrontClient, DescribeKeyValueStoreCommand } from "@aws-sdk/client-cloudfront"; 
const client = new CloudFrontClient({ region: "us-east-1" });
const input = { Name: undefined }; // <-- this field should be required, `string` only, not `string | undefined`
const command = new DescribeKeyValueStoreCommand(input); <- I expect linting error here, because the `Name` field is actually required by the API

Similar for the other client, from @aws-sdk/client-cloudfront-keyvaluestore

import { CloudFrontKeyValueStoreClient, DescribeKeyValueStoreCommand } from "@aws-sdk/client-cloudfront-keyvaluestore"; 
const client = new CloudFrontKeyValueStoreClient({ region: "us-east-1" });
const input = { Name: undefined }; // <-- this field should be required, `string` only, not `string | undefined`
const command = new DescribeKeyValueStoreCommand(input); <- I expect linting error here, because the `Name` field is actually required by the API

Observed Behavior

These types should be strings - these fields are required by the API, and there is a runtime error if the value is not set:

    "ValidationException: 1 validation error detected: Value null at 'ifMatch' failed to satisfy constraint: Member must not be null",
    "    at de_ValidationExceptionRes (/var/runtime/node_modules/@aws-sdk/client-cloudfront-keyvaluestore/dist-cjs/index.js:585:21)",
    "    at de_CommandError (/var/runtime/node_modules/@aws-sdk/client-cloudfront-keyvaluestore/dist-cjs/index.js:502:19)",
...

Expected Behavior

1/ SDK type mentioning string only, 2/ my IDE / TypeScript able to detect this behavior at lint/compile time.

Possible Solution

Fix autogenerated types

Additional Information/Context

I may be able to fix the bug, please just tell me where to start.

aBurmeseDev commented 3 weeks ago

Hi @piotrekwitkowski - thanks for reaching out.

I'm not able to reproduce with the code below. Can you share reproducible code for the error?

import { CloudFrontClient, DescribeKeyValueStoreCommand } from "@aws-sdk/client-cloudfront"; 
const client = new CloudFrontClient({region: "us-west-1"});
const input = { 
  Name: "STRING_VALUE", 
};
const command = new DescribeKeyValueStoreCommand(input);
const response = await client.send(command);

Can you also share your Node & SDK version as well?

piotrekwitkowski commented 3 weeks ago

Hi, updated Node and SDK version above. The issue is not about the good case, when you provide the Name value. It is about the case where I don't provide the value, which is a problem that should be caught way earlier, to improve developer experience.

aBurmeseDev commented 3 weeks ago

Thanks for your response. A few things to address here:

Name value is a param required by service API (CloudFront) and it throws error when not provided, which is correct behavior.
Which param required in every client call is defined in service models that are generated upstream by service teams and SDK doesn't have control over it.
As far as why we display type as string | undefined vs only string, it's for forward-compatibility with service models and has been addressed a few times on this repo (https://github.com/aws/aws-sdk-js-v3/issues/5992).

Hope it clears things up, let me know if there's any further questions!

github-actions[bot] commented 1 week 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