Regression to Blob types with respect to SDKv2

rix0rrr commented 1 year ago

Describe the feature

It seems that input fields marked as "Blob" types used to permissively accept types in SDKv2, but no longer do so in SDKv3.

As an example, Kinesis PutRecord:

SDKv2

  export interface PutRecordInput {
    StreamName?: StreamName;

    Data: Data;

    // ....
  }
  export type Data = Buffer|Uint8Array|Blob|string;
}

https://github.com/aws/aws-sdk-js/blob/ecd7c9c2ad37560326e1645b83ad981dbee7f4f9/clients/kinesis.d.ts#L343

SDKv3

export interface PutRecordInput {
  StreamName?: string;

  Data: Uint8Array | undefined;

  // ...
}

https://github.com/aws/aws-sdk-js-v3/blob/4adc48a4ef86d0af396ee8f0861c6730f0d2a562/clients/client-kinesis/src/models/models_0.ts#L2201

Use Case

Why does this matter?

We are the CDK, migrating from SDKv2 to SDKv3. There are multiple places where we are driving the SDK not with our own, type-checked values, but using values we get from users and pass on to the SDK.

Users may have been passing us strings where BLOBs were expected, which works perfectly well with SDKv2, but is now all of a sudden broken for SDKv3.
Users may have set up AWS API calls from EventBridge events using a Lambda that uses the SDK to make an AWS API call, using some of the event JSON to form the payload to the AWS API call. Those used to work fine as strings, but are now broken. What's worse, the JSON events are going to contain the strings that they contain, and there's both no way to represent Uint8Arrays in JSON, as well as no ability for the user to control the format of the event in the first place.

If we knew that a Uint8Array was expected, it would be trivial for us to do a run-time conversion, but unfortunately we have no insight into the type that is expected at run time (remember, this is long past the TypeScript type checking stage). Therefore, we can't do the conversion, and have no other solution than to pass on the wrongly typed value and have the call fail.

Proposed Solution

Can the SDK not follow Postel's Law here, and also accept the other types you used to accept in SDKv2, doing an under-the-covers conversion as necessary?

Alternatively, provide us with a run-time inspectable table telling us the types of all fields, so that we can do that conversion as necessary?

Other Information

No response

Acknowledgements

[ ] I may be able to implement this feature request
[ ] This feature might incur a breaking change

SDK version used

latest

Environment details (OS name and version, etc.)

Nodejs 20

rix0rrr commented 1 year ago

Also, I'm not quite sure why PutRecord#Data and Invoke#Payload are being rendered differently here. Looking at the Smithy models, both are modeled as a type: "blob":

Kinesis:

https://github.com/aws/aws-sdk-js-v3/blob/515ea2e10b9906d15841f63bf22d56accfd49b0e/codegen/sdk-codegen/aws-models/kinesis.json#L334

Lambda:

https://github.com/aws/aws-sdk-js-v3/blob/515ea2e10b9906d15841f63bf22d56accfd49b0e/codegen/sdk-codegen/aws-models/lambda.json#L2084

trivikr commented 1 year ago

The smithy blob type is uninterpreted binary data, so we're not accepting string anymore in JS SDK v3.

In the past, users requested to get string type for output, like s3.getObject, and we had provided mixin in https://github.com/aws/aws-sdk-js-v3/pull/3977 which allows consumers to transform Unit8Array to relevant types.

We can add a similar fix for accepting string in input and do the conversion internally. We'll discuss this with the rest of the JS SDK team, and update this issue.

trivikr commented 1 year ago

The automatic conversion of blob types was implemented in https://github.com/aws/aws-sdk-js-v3/pull/4836, which was published in https://github.com/aws/aws-sdk-js-v3/releases/tag/v3.357.0

This is applicable to TypeScript types of blob. The SDK is not doing any conversion from string to Uint8Array. For output, we did add a helper method to transform to string.

Verified using the following code:

// invokeLambdaFn.ts
import { Lambda } from "@aws-sdk/client-lambda";

const client = new Lambda({ region: "us-west-2" });
client.invoke({
  FunctionName: "test-function-name",
  Payload: JSON.stringify({ foo: "bar" }),
});

$ npx tsc --version
Version 5.1.6

v3.356.0

TypeScript TS2322 error is thrown

$ npx tsc invokeLambdaFn.ts
invokeLambdaFn.ts:7:3 - error TS2322: Type 'string' is not assignable to type 'Uint8Array'.

7   Payload: JSON.stringify({ foo: "bar" }),
    ~~~~~~~

  node_modules/@aws-sdk/client-lambda/dist-types/models/models_0.d.ts:3373:5
    3373     Payload?: Uint8Array;
             ~~~~~~~
    The expected type comes from property 'Payload' which is declared here on type 'InvokeCommandInput'

Found 1 error in invokeLambdaFn.ts:7

v3.357.0

No error is thrown by TypeScript

$ npx tsc invokeLambdaFn.ts

Is there a specific API call in CDK which we can use for testing? One which fails on sending a string for a blob type.

rix0rrr commented 1 year ago

AwsApi and AwsCustomResource, for example. But there's not really anything to test for you right now.

Thanks for the effort, let's hope this makes it to Lambda NodeJS 18.x runtime soon!

kuhe commented 1 year ago

there may be a workaround involving providing a custom implementation to client.config.base64Encoder that converts strings to Uint8Array before calling the default base64Encoder.

kuhe commented 3 months ago

we updated the default base64 encoder to accept strings and not just byteArrays. Is this still an issue in the latest SDK version?

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

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