GetObjectOutput.Body is incorrectly typed with unrelated types across frameworks

[gh-andre]

Checkboxes for prior research

• I've gone through Developer Guide and API reference
• I've checked AWS Forums and StackOverflow.
• I've searched for previous similar issues and didn't find any solution.

Describe the bug

This bug is incorrectly tracked as a "improvement" in this issue.

#4720

The reason it's a bug and not something to improve is because Readable is valid only for Node.js and ReadableStream and Blob are valid only for browser frameworks, so when one wants to narrow this properly in TypeScript for Node, the code cannot reliably reference the latter because they don't exist. In fact, Node does have both names, except that they don't match the types intended by this library at all, which creates further confusion (i.e. one in node:stream/web and the other in node:buffer). On top of it, both types are declared in serde.d.ts, so it compiles if these types are referenced, but won't run.

SDK version number

@aws-sdk/[email protected]

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

Node v16.13.1

Reproduction Steps

This is what would typically be used to avoid compiler errors, but it will fail at run time because ReadableStream and Blob are declared in serde.d.ts, but not defined anywhere. Node's native interfaces with the same names cannot be used because they don't match the layout of what's expected.

    response = await this.s3_client.send(get_object_command);
    if(response.Body == undefined || response.Body instanceof ReadableStream || response.Body instanceof Blob)
        throw Error("S3 response body other than Readable is invalid");
    let inStream: Readable = response.Body;

Observed Behavior

See above

Expected Behavior

AWS S3 targeting a specific framework, such as Node.js, shall provide only framework-specific types in a type union, which for Node is only Readable. Putting types for unrelated frameworks in the same union is a design bug.

Possible Solution

The only way to get current code working is to use the not with the single correct type for Node, like this:

    response = await this.s3_client.send(get_object_command);
    if(response.Body == undefined || !(response.Body instanceof Readable))
        throw Error("S3 response body other than Readable is invalid");
    let inStream: Readable = response.Body;

This is not a solution for the problem, but just the work-around to allow current code compile and run despite the malformed type union.

The actual solution would be to split types cleanly across different frameworks.

Additional Information/Context

I'm sure this will be closed as "working as designed", but if you do want to save time for developers using AWS S3 framework/module, add a note against Body on this page:

https://docs.aws.amazon.com/AWSJavaScriptSDK/v3/latest/Package/-aws-sdk-client-s3/Interface/GetObjectOutput/

, that only Readable will appear only in Node and ReadableStream or Blob will appear in web frameworks, so people don't waste time hunting down how to handle alternative types in their actual frameworks, like I did today until I stumbled on a couple of threads suggesting to just "ignore" those alternative types.

[kuhe]

I have a PR in progress to replace the streaming payload union with a type alias that describes why the union is comprised of that many types, you can see it in smithy-lang/smithy-typescript#835.

We're not going to offer a version of the S3 Client that contains pre-narrowed blob payload types because the blob payload type must be inferred from the client's request handler, and that would mean passing the parametric type across too many levels.

You can declare a narrowed client with no runtime overhead like this:

import { GetObjectCommand, GetObjectCommandOutput, S3Client } from "@aws-sdk/client-s3";
import { Command, MetadataBearer, SdkStream } from "@smithy/types";
import { IncomingMessage } from "node:http";

// This is the current streaming payload blob type used in S3.
type StreamingBlobPayloadOutputTypes = GetObjectCommandOutput["Body"];

/**
 * Declare a custom narrowed type for your runtime environment.
 */
interface S3ClientNodeJs extends S3Client {
  send<CI extends object, I extends CI, CO extends MetadataBearer, O extends CO, R>(
    command: Command<CI, I, CO, O, R>
  ): Promise<Transform<O, StreamingBlobPayloadOutputTypes | undefined, SdkStream<IncomingMessage>>>;
}

// declare the type of the client.
const s3 = new S3Client({}) as S3ClientNodeJs;

const res = await s3.send(
  new GetObjectCommand({
    Bucket: "...",
    Key: "...",
  })
);

// streaming blob payloads are now the narrowed type.
const body: SdkStream<IncomingMessage> = res.Body!;
body.transformToString();

The type narrowed declaration asserts a replacement for the output type, using a transform helper:

// The Transform type helper is a work in progress and
// may be included in future versions of the AWS SDK.
export type Transform<T, FromType, ToType> = ConditionalRecursiveTransformExact<T, FromType, ToType>;
type TransformExact<T, FromType, ToType> = [T] extends [FromType] ? ([FromType] extends [T] ? ToType : T) : T;
type RecursiveTransformExact<T, FromType, ToType> = T extends Function
  ? T
  : T extends object
  ? {
      [key in keyof T]: [T[key]] extends [FromType]
        ? [FromType] extends [T[key]]
          ? ToType
          : ConditionalRecursiveTransformExact<T[key], FromType, ToType>
        : ConditionalRecursiveTransformExact<T[key], FromType, ToType>;
    }
  : TransformExact<T, FromType, ToType>;
type ConditionalRecursiveTransformExact<T, FromType, ToType> = [T] extends [
  RecursiveTransformExact<T, FromType, ToType>
]
  ? [RecursiveTransformExact<T, FromType, ToType>] extends [T]
    ? T
    : RecursiveTransformExact<T, FromType, ToType>
  : RecursiveTransformExact<T, FromType, ToType>;

[gh-andre]

@kuhe Thank you for the detailed response and work-arounds.

I suppose these work-arounds are warranted for larger apps that use many S3Client calls throughout the app, but for those that use just core S3 functionality in some app storage service, it's probably simpler/cleaner to just filter out those non-existing types, with appropriate comments that some of these types don't exist in Node.

I can also appreciate not being able to offer a framework-specific package because of a large codebase. While unfortunate, it's manageable, as long as it is documented, that things like ReadableStream in Node are just placeholders for alternative frameworks.

[kuhe]

Update for anyone reading this, the optional streaming payload type constraints are available via @smithy/types.

Example:

import { S3 } from "@aws-sdk/client-s3";
import { NodeJsClient } from "@smithy/types";

const s3 = new S3({}) as NodeJsClient<S3>;

const getObject = await s3.getObject({
  Bucket: "",
  Key: "",
});

const body = getObject.Body; // the type will resolve as SdkStream<IncomingMessage>, a platform-specific type.
const bodyString = await body?.transformToString();

[gh-andre]

@kuhe Thank you. Much appreciated.

[github-actions]

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.