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

Improve TypeScript experience for GetObjectCommandOutput Body / add SdkStreamMixin::transformToNodeSream #4720

Closed
1 of 2 tasks
nwalters512 opened this issue May 11, 2023 · 20 comments
Assignees
Labels
closed-for-staleness feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog

Comments

@nwalters512
Copy link
Contributor

nwalters512 commented May 11, 2023

Describe the feature

I want to read an object from S3 and pipe it to a file with TypeScript code. With the v2 SDK, this would look like the following:

import * as AWS from 'aws-sdk';
import * as fs from 'node:fs';

const s3 = new AWS.S3();

const s3Stream = s3.getObject({ Bucket: 'demo', Key: 'file.txt' }).createReadStream();
s3Stream.pipe(fs.createWriteStream('file.txt'));

Here's what I attempted to do with the v3 SDK

import * as fs from 'node:fs';
import { S3Client, GetObjectCommand } from '@aws-sdk/client-s3';

const s3 = new S3Client({});

const res = await s3.send(new GetObjectCommand({ Bucket: 'demo', Key: 'file.txt' }));
res.Body?.pipe(fs.createWriteStream('file.txt'));

However, TypeScript complains that there is no pipe method on Body:

Screenshot 2023-05-11 at 14 32 40

This seems to be because the type definition for Body is too broad.

Use Case

I understand that the current type of Body (Readable | ReadableStream | Blob) probably exists so that the SDK can use the same types across web and Node. However, this makes it unergonomic to use in Node. I don't want to litter my code with as stream.Readable every time I have to get an object from S3. The current type also incorrectly suggests that one may sometimes get a Blob or a ReadableStream back on Node, when AFAICT one will only ever get a stream.Readable.

Proposed Solution

Body already has some helper members like transformToString() and transformToWebStream() that produce a narrowly-typed value. However, there's no equivalent to get a Node readable stream. I'm proposing something like transformToNodeSream() that will return a stream.Readable.

I recognize that this function wouldn't be able to do anything useful in the browser, but I think that's an acceptable compromise. transformToWebStream() will already error in unsupported Node environments; it's fine if transformToNodeSream() does the same on the web.

I don't love baking Node into the method name, as Node-style streams may be supported in other runtimes (e.g. Bun). However, I think that "Node-style streams" as a concept are reasonably-well understood, so this name should work. I also considered transformToReadableStream or transformToReadStream, which are reasonable alternatives.

Other Information

I'm lifting this out of #1877. The problem was brought up there, but wasn't resolved before the issue was closed and locked.

Note that in theory, one might expect the following to work:

stream.Readable.fromWeb(res.Body.transformToWebStream());

However, this also produces a TypeScript error:

Screenshot 2023-05-11 at 14 54 29

Acknowledgements

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

SDK version used

3.332.0

Environment details (OS name and version, etc.)

macOS + Node 18.12.0

@nwalters512 nwalters512 added feature-request New feature or enhancement. May require GitHub community feedback. needs-triage This issue or PR still needs to be triaged. labels May 11, 2023
@RanVaknin RanVaknin self-assigned this May 12, 2023
@yenfryherrerafeliz
Copy link
Contributor

Hi @nwalters512, thanks for opening this feature request. I will mark this feature request to be reviewed so we can define next steps on this. I do not guarantee this will be looked at soon, but, we prioritize bug fixes and feature requests based on community reactions and comments, which means the priority of this feature request may increase based on the criteria I just mentioned.

Thanks!

@yenfryherrerafeliz yenfryherrerafeliz added needs-review This issue/pr needs review from an internal developer. p2 This is a standard priority issue and removed needs-triage This issue or PR still needs to be triaged. labels May 19, 2023
@nwalters512
Copy link
Contributor Author

Thanks @yenfryherrerafeliz! I was hoping to cross-link this issue on #1877 where this was first discussed so that folks subscribed to that issue could come here and 👍-react, but this issue is locked and limited to collaborators. Would you be able to either unlock it or post something on my behalf there?

@RanVaknin RanVaknin added p3 This is a minor priority issue queued This issues is on the AWS team's backlog and removed p2 This is a standard priority issue needs-review This issue/pr needs review from an internal developer. labels Jun 13, 2023
@yenfryherrerafeliz
Copy link
Contributor

Hi @nwalters512, would work for you to do the following:

import {GetObjectCommand, S3Client} from "@aws-sdk/client-s3";
import * as fs from "fs";
import { NodeJsClient } from "@smithy/types";

const client = new S3Client({
    region: "us-east-2"
}) as NodeJsClient<S3Client>;
const response = await client.send(new GetObjectCommand({
    Bucket: process.env.TEST_BUCKET,
    Key: process.env.TEST_KEY
}));
response.Body.pipe(fs.createWriteStream('./test-file.txt'));

By doing this the type in the Body will be narrowed down to NodeJS.

Please let me know if that helps!

Thanks!

@yenfryherrerafeliz yenfryherrerafeliz added the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Oct 21, 2023
@nwalters512
Copy link
Contributor Author

While that does work, I don't think it's meaningfully better than doing (response.Body as stream.Readable).pipe() as I still have to remember to do ... as ... every time. This is exactly the kind of unergonomic code I want to avoid:

However, this makes it unergonomic to use in Node. I don't want to litter my code with as stream.Readable every time I have to get an object from S3.

And with your proposed version, I also have to rely on some other package (what is Smithy?) with no obvious relationship to @aws-sdk/client-s3.

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to \"closing-soon\" in 7 days. label Oct 24, 2023
@andrewatwood
Copy link

Also having this issue, very bizarre to be written in this way with no way out by default.

@Vulpeslnculta
Copy link

I have also encountered the same issue, this time trying to pass transformToWebStream output with pipe that returns Property 'pipe' does not exist on type 'ReadableStream<any>'

@AnthonyDugarte
Copy link

While that does work, I don't think it's meaningfully better than doing (response.Body as stream.Readable).pipe() as I still have to remember to do ... as ... every time. This is exactly the kind of unergonomic code I want to avoid:

However, this makes it unergonomic to use in Node. I don't want to litter my code with as stream.Readable every time I have to get an object from S3.

And with your proposed version, I also have to rely on some other package (what is Smithy?) with no obvious relationship to @aws-sdk/client-s3.

I know this is kind of old but here's the relationship between Smithy and @aws-sdk/client-s3:

Why did you develop Smithy?

Smithy is based on an interface definition language that has been widely used within Amazon >and AWS for over a decade. We decided to release Smithy publicly to let other developers use >Smithy for their own services and benefit from our years of experience with building tens of >thousands of services. By releasing the Smithy specification and tooling, we also hope to >make it easier for developers to maintain open source AWS SDKs.

https://smithy.io/2.0/index.html#why-did-you-develop-smithy

@bsshenrique
Copy link

If someone still looking for it, you can go with something like:

import { Readable } from 'node:stream';

const response = await client.send(command);

// Readable -> ...
return response.Body as Readable;

// A bad way for who don't care about memory
// File in memory -> Buffer -> Readable -> ...
const file = await response.Body?.transformToString('encoding');
return Readable.from(Buffer.from(file as string, 'encoding'));

// You also can pipe it to a writable
(response.Body as Readable).pipe(dest)

@vladimirm85
Copy link

maybe this will help

@danielrsantana-humaitrix
Copy link

danielrsantana-humaitrix commented Jun 6, 2024

Hey, AWS Team. Are there any updates on this? In the meantime, I'm using any, but it breaks the TypeScript purpose.

For those trying the same, I managed without the agent Smith suggested by @yenfryherrerafeliz and questioned by @AnthonyDugarte. Instead, I found an alternative solution that is a simplified version of what @bsshenrique suggested.

  downloadFromS3: async (
    key: string,
    awsCredentials: ITenantAwsCredentials
  ): Promise<any | undefined> => {
    const bucketName = process.env.AWS_STORAGE_BUCKET_NAME!;
    const command = new GetObjectCommand({
      Bucket: bucketName,
      Key: key
    });

    return (await s3Client(awsCredentials).send(command)).Body;
  },
  
    app.post(
    '/api/v2/download-media/:media_id',
    async (request: Request, response: Response) => {
      const key = 'path/inside_your_bucket/file.extension';
      const credentials = {
        region: 'your-region',
        credentials: {
          accessKeyId: 'your_access_key_id',
          secretAccessKey: 'your_secret_access_key'
      };

      const data = await downloadFromS3(key, credentials );

      if (!data) {
        return response.status(204).send();
      }

      response.setHeader('Content-Type', 'audio/mp3');
      response.setHeader(
        'Content-Disposition',
        `attachment; filename="${media_id}.${media_extension}"`
      );

      //This is the magic:
      (data as any).pipe(response);
    }
  );

In your front end, set either Axios or Fetch to GET/POST/PUT (get if you don't send parameters), add this:

  postRequestMedia = async (url: string, data?: any) => {
    const config = { responseType: 'blob', headers: this.config.headers };

    return this.axiosInstance.post(url, data, config);
  };
  
  // inside of your component ...
  const getMessageAudio = async (): Promise<Blob | undefined> => {
      try {
        const url = `https://your-api-server.domain.com/api/v2/download-media/${props.media_id}`;
        const data = { parameter: 'some data you might need to pass to your API' };
        const response = await downloadBlob(url, data);

        return (await response).data;
      } catch {
        console.log('Not expected');
      }
    }
  };
  
  useEffect(() => {
    if (props.media_id) {
      (async () => {
        setMediaBlob(await getMessageAudio());
      })();
    }
  }, [props.media_id]);

  useEffect(() => {
    if (mediaBlob?.size) {
      const url = URL.createObjectURL(mediaBlob);
      setAudioUrl(url);

      return () => {
        URL.revokeObjectURL(url);
      };
    }
  }, [mediaBlob?.size]);

  useEffect(() => {
    if (mediaBlob?.size) {
      const url = URL.createObjectURL(mediaBlob);
      setAudioUrl(url);

      return () => {
        URL.revokeObjectURL(url);
      };
    }
  }, [mediaBlob?.size]);

  (...)

  return (
    // ...
    <audio preload='metadata' controls>
      {audioUrl && <source src={audioUrl} type='audio/mpeg' />}
      Your browser does not support the audio element.
    </audio>
  )

JIT: This code got simplified to make it easier to understand what to do. If you need more detailed info, you can check this StackOverflow answer, which is how I ended up here and realised pipe was the way to go. Linke here

@kuhe kuhe changed the title Improve TypeScript experience for GetObjectCommandOutput Body Improve TypeScript experience for GetObjectCommandOutput Body / add SdkStreamMixin::transformToNodeSream Jul 18, 2024
@kuhe
Copy link
Contributor

kuhe commented Jul 18, 2024

For typing, as Yenfry stated above this following type transform narrows the body to a very specific NodeJS Readable type called IncomingMessage.

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

const client = new S3Client({
    region: "us-east-2"
}) as NodeJsClient<S3Client>;

const response = await client.send(new GetObjectCommand({
    Bucket: process.env.TEST_BUCKET,
    Key: process.env.TEST_KEY
}));

// response.Body is IncomingMessage instance

For adding the new runtime transform to Readable method, that is in the backlog.

@kuhe kuhe added p2 This is a standard priority issue and removed p3 This is a minor priority issue labels Jul 18, 2024
@cisox
Copy link

cisox commented Jul 24, 2024

For anyone looking for the types they can use for passing the input and output around, I use these:

type NarrowedInputType<I> = Transform<
    I,
    StreamingBlobPayloadInputTypes | undefined,
    NodeJsRuntimeStreamingBlobPayloadInputTypes
>;

type NarrowedOutputType<O> = Transform<
    O,
    StreamingBlobPayloadOutputTypes | undefined,
    NodeJsRuntimeStreamingBlobPayloadOutputTypes
>;

and then:

export function putS3Object(input: NarrowedInputType<PutObjectCommandInput>)
export function getS3Object(): Promise<NarrowedOutputType<GetObjectCommandOutput>>

or

type NarrowedPutObjectCommandInput = NarrowedInputType<PutObjectCommandInput>;
type NarrowedGetObjectCommandOutput = NarrowedOutputType<GetObjectCommandOutput>;

export function putS3Object(input: NarrowedPutObjectCommandInput)
export function getS3Object(): Promise<NarrowedGetObjectCommandOutput>

@RanVaknin RanVaknin removed their assignment Aug 5, 2024
@etaoins
Copy link

etaoins commented Aug 19, 2024

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

const client = new S3Client({
    region: "us-east-2"
}) as NodeJsClient<S3Client>;

const response = await client.send(new GetObjectCommand({
    Bucket: process.env.TEST_BUCKET,
    Key: process.env.TEST_KEY
}));

// response.Body is IncomingMessage instance

This no longer works on @aws-sdk/[email protected]:

No overload matches this call.
  Overload 1 of 4, '(command: Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>, options?: HttpHandlerOptions | undefined): Promise<...>', gave the following error.
    Argument of type 'GetObjectCommand' is not assignable to parameter of type 'Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { ...; } | ... 92 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>'.
      Types of property 'resolveMiddleware' are incompatible.
        Type '(stack: MiddlewareStack<ServiceInputTypes, ServiceOutputTypes>, configuration: S3ClientResolvedConfig, options: any) => Handler<...>' is not assignable to type '(stack: MiddlewareStack<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, ServiceOutputTypes>, configuration: SmithyResolvedConfiguration<...>, options: any) => Handler...'.
          Types of parameters 'stack' and 'stack' are incompatible.
            Type 'MiddlewareStack<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { ...; } | ... 92 more ... | { ...; }, ServiceOutputTypes>' is not assignable to type 'MiddlewareStack<ServiceInputTypes, ServiceOutputTypes>'.
              Types of property 'add' are incompatible.
                Type '{ (middleware: InitializeMiddleware<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, ServiceOutputTypes>, options?: (InitializeHandlerOptions & AbsoluteLocation) | und...' is not assignable to type '{ (middleware: InitializeMiddleware<ServiceInputTypes, ServiceOutputTypes>, options?: (InitializeHandlerOptions & AbsoluteLocation) | undefined): void; (middleware: SerializeMiddleware<...>, options: SerializeHandlerOptions & AbsoluteLocation): void; (middleware: BuildMiddleware<...>, options: BuildHandlerOptions & ...'.
                  Types of parameters 'middleware' and 'middleware' are incompatible.
                    Types of parameters 'next' and 'next' are incompatible.
                      Type 'InitializeHandler<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { ...; } | ... 92 more ... | { ...; }, ServiceOutputTypes>' is not assignable to type 'InitializeHandler<ServiceInputTypes, ServiceOutputTypes>'.
                        Type 'ServiceInputTypes' is not assignable to type '{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { Bucket: string | undefined; ... 11 more ...; SSECustomerKeyMD5?: string | undefined; } | ... 92 more ... | { ...; }'.
  Overload 2 of 4, '(command: Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>, options?: HttpHandlerOptions | undefined, cb?: ((err: unknown, data?: { ...; } | undefined) => void) | undefined): void | Promise<...>', gave the following error.
    Argument of type 'GetObjectCommand' is not assignable to parameter of type 'Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { ...; } | ... 92 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>'.

@cisox
Copy link

cisox commented Aug 20, 2024

@etaoins I seem to remember having a similar problem. Make sure your packages are all up-to-date and matching versions.

@kuhe kuhe self-assigned this Aug 20, 2024
@lexctk
Copy link

lexctk commented Aug 22, 2024

@cisox I'm having the exact same issue as @etaoins, it was working in @aws-sdk/[email protected], however, in the latest version:

"@aws-sdk/client-s3": "^3.635.0"
"@smithy/types": "^3.3.0"
import {GetObjectCommand, S3Client} from "@aws-sdk/client-s3";
import { NodeJsClient } from "@smithy/types";

const client = new S3Client() as NodeJsClient<S3Client>;

const response = await client.send(new GetObjectCommand({
    Bucket: "bucket",
    Key: "key"
}));
TS2769: No overload matches this call.
Overload 1 of 4, '(command: Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>, options?: HttpHandlerOptions | undefined): Promise<...>', gave the following error.
Argument of type GetObjectCommand is not assignable to parameter of type
Command<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | { ...; } | ... 92 more ... | { ...; }, GetObjectCommandInput, ServiceOutputTypes, GetObjectCommandOutput, SmithyResolvedConfiguration<...>>
Types of property resolveMiddleware are incompatible.
Type '(stack: MiddlewareStack<ServiceInputTypes, ServiceOutputTypes>, configuration: S3ClientResolvedConfig, options: any) => Handler<...>' is not assignable to type '(stack: MiddlewareStack<{ Bucket: string | undefined; Key: string | undefined; UploadId: string | undefined; RequestPayer?: "requester" | undefined; ExpectedBucketOwner?: string | undefined; } | ... 93 more ... | { ...; }, ServiceOutputTypes>, configuration: SmithyResolvedConfiguration<...>, options: any) => Handler...'.

@jlarmstrongiv
Copy link

The last working version with @smithy/types is 3.631.0

@webjay
Copy link

webjay commented Aug 26, 2024

Who at AWS thinks you're doing a great job with this?

Who would think that it's so difficult in 2024 to download a file?!

@kuhe kuhe added the pending-release This issue will be fixed by an approved PR that hasn't been released yet. label Sep 5, 2024
@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed closing-soon This issue will automatically close in 4 days unless further comments are made. labels Sep 9, 2024
@kuhe
Copy link
Contributor

kuhe commented Sep 9, 2024

A fix is available in https://www.npmjs.com/package/@smithy/types/v/3.4.0. Most recent versions of AWS SDK clients have a ^3.x range dependency on Smithy types and are compatible with the fix.

Clients releasing from tomorrow onwards will have the fix in their minimum required version of @smithy/types.

The fix is for compilation errors related to the SDK Client type transforms from Smithy types, e.g.

import type { AssertiveClient, BrowserClient, NodeJsClient, UncheckedClient } from "@smithy/types";

@kuhe kuhe added closing-soon This issue will automatically close in 4 days unless further comments are made. and removed pending-release This issue will be fixed by an approved PR that hasn't been released yet. labels Sep 9, 2024
@etaoins
Copy link

etaoins commented Sep 10, 2024

This seems to work again now 🙇

@github-actions github-actions bot removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Sep 11, 2024
@kuhe kuhe added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Sep 12, 2024
@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 Sep 15, 2024
Copy link

github-actions bot commented Oct 3, 2024

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 Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
closed-for-staleness feature-request New feature or enhancement. May require GitHub community feedback. p2 This is a standard priority issue queued This issues is on the AWS team's backlog
Projects
None yet
Development

No branches or pull requests