The getSignedUrl after migration to SDK V3 needs await which requires us to change sync funtion to async function.

[abhirpat]

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

The getSignedUrl after migration to SDK V3 needs await which requires us to change sync funtion to async function. This changes application behavior and thus requires us to make many functions.

SDK version number

@aws-sdk/client-s3@3.456.0

Which JavaScript Runtime is this issue in?

Node.js

Details of the browser/Node.js/ReactNative version

v18.18.0

Reproduction Steps

Convert below code to SDK V3 which is requires await before getSignedUrl

import AWS from "aws-sdk";

// Bucket and object should be present in your AWS account.
const BUCKET = "aws-sdk-js-test";
const KEY = "hello-world.txt";

const s3 = new AWS.S3({ region: "us-east-1" });

const syncFunc = () => {
  return s3.getSignedUrl("getObject", { Bucket: "bucket", Key: "key" });
};

const url = syncFunc();
console.log(url);

Observed Behavior

After converting it to SDK V3, it is requires await before getSignedUrl which forces us to change sync function to async function. This complicates the migration to SDK V3 as many functions requires update.

Expected Behavior

After migrating, it should allow us to keep using sync function.

Possible Solution

Allow to use it without await

Additional Information/Context

No response

[RanVaknin]

Hi @abhirpat ,

Thanks for reaching out. In AWS SDK v2, there were two methods for generating a pre-signed URL – getSignedUrl and getSignedUrlPromise. In v3, we have consolidated this to a single asynchronous method. This change aligns with the asynchronous nature of JavaScript in Node.js.

This is documented in our UPGRADING.md doc.

While we understand this transition requires adaptations for those used to v2, it's a deliberate step to keep the SDK in line with modern JavaScript practices and prepare for potential future complexities. We recognize the efforts involved in migrating and thank you for your understanding as we continue to evolve the SDK.

All the best, Ran~

[RanVaknin]

Hi @abhirpat ,

Thanks for the context. I did speak about this with the team. Ill keep it open as a feature request and wait to see if it gets more traction.

All the best, Ran~

[zaheerbabarkhan]

The getSignedUrl will be needed in v3 too.

In standard practice, this function is used in standard getters functions if ORMs and ODMs, the problem is that those standard getter functions do not support async activity and only allow sync code.

As mentioned by @abhirpat this will need a lot of wrapping around the code or the use of unnecessary third-party libraries for that wrapping purpose.

thanks

[BuistvoPloti]

100% agree with @zaheerbabarkhan

[BuistvoPloti]

the most convenient solution to use something like this: remove get() that you have, rename it to whatever you want for example async_get (we just need an async fn on the object definition level)

name: {
    type: type.VIRTUAL,
    async_get: async function () {
    const doc = await fn(your async call)
    return {...}
  }

and then you can apply it somewhere globally

// on global sequelize hooks 
async afterFind(instances) {
        await run_async_getter.call(this, instances);
    }

async function run_async_getter(instances) {
    for (const [type, attribute] of Object.entries(this.rawAttributes)) {
        const { async_get: get } = attribute;

        if (get && get.constructor.name === "AsyncFunction") {
            if (Array.isArray(instances)) {
                await Promise.all(
                    instances.map(async instance => {
                        instance.setDataValue(type, await get.call(instance));
                    })
                );
            } else if (instances) {
                instances.setDataValue(type, await get.call(instances));
            }
        }
    }
}

version to make "include" also trigger hooks:

async function run_async_getter(instance, options) {
    if (Array.isArray(instance))
        return await Promise.all(
            instance.map(async instance => {
                run_async_getter.call(this, instance, options);
            })
        );

    for (const [type, attribute] of Object.entries(this.rawAttributes)) {
        const { async_get: get } = attribute;

        if (get && get.constructor.name === "AsyncFunction") {
            if (instance) {
                instance.setDataValue(type, await get.call(instance));
            }
        }
    }

    if (instance && options && options.includeMap) {
        await Promise.all(
            Object.entries(options.includeMap)
                .map(([key, value]) => {
                    if (instance && instance[key]) {
                        return run_async_getter.call(value.model, instance[key], value);
                    }
                })
                .filter(Boolean)
        );
    }
}

[klesgidis]

👍 +100

[nikosd23]

If you want better adoption of aws v3 yοu should consider merging this. We have in our roadmap to update it but the task is blocked due to this.

[charlesritchea]

I really hate the c word (consistency) It's caused no end of misery. A pure function that does not use external IO systems, but is a raw algorithm has no business being only async. This is literally the only use of v2 that we are stuck with. I'm considering re-inventing the wheel as a workaround

[dangerbacon]

This is blocking my project from adopting V3 as well. This change would require completely restructuring a significant portion of my product to convert synchronous code to asynchronous code, and then downstream projects would have to be restructured to deal with those changes as well.

It's all for no real benefit, either. I've walked through the code of the entire call chain of getSignedUrl through all the dependencies, and it's just dozens of async/await wrappers around what is ultimately synchronous code underneath. Like charlesritchea said, this is raw algorithm and requires no external IO systems. Making this async-only and forcing people to restructure their entire projects for no good reason other than "consistency" is absurd.

[abhirpat]

@RanVaknin could you please provide update?

[charlesritchea]

Glad you found a workaround for your scenario, but this is still async. You can't wait for the value to be returned, so it can't be used in a getter. This is how all async code used to be done before promises

On Mon, Jul 1, 2024, 2:36 PM Abhishek @.***> wrote:

@RanVaknin https://github.com/RanVaknin could you please provide update?

Meanwhile, I can share my workaround to avoid refactoring large no. of code. Hope it it helps while SDK team is reviewing this!

1. Synchronous function signS3URL https://github.com/aws-solutions/qnabot-on-aws/blob/main/source/lambda/es-proxy-layer/lib/signS3URL.js
2. An example of importing the exports defined in above function https://github.com/aws-solutions/qnabot-on-aws/blob/main/source/lambda/es-proxy-layer/lib/bedrock/bedrockAgents.js

— Reply to this email directly, view it on GitHub https://github.com/aws/aws-sdk-js-v3/issues/5509#issuecomment-2200781220, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMUMHI6UFVOSCLVWDODD4LZKGOTNAVCNFSM6AAAAAA7VGM2M2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBQG44DCMRSGA . You are receiving this because you commented.Message ID: @.***>

[RanVaknin]

Hi everyone,

I have revisited this issue for further discussion with the team. The v3 SDK relies on underlying cryptographic libraries that inherently use asynchronous signing methods. For this reason, introducing synchronous getSignedUrl method is not feasible.

I understand that this is a contentious issue, given the substantial feedback and community interest. However, at this point, we are unable to proceed with implementing this feature. If you have additional insights or data points you would like to share, please feel free to comment below.

Thank you, Ran~

[dangerbacon]

For anyone who discovers this ticket in the future:

There is an NPM package called "aws4" that can sign requests synchronously using the new AWS Signature Version 4 standard. Here's the URL for it: https://www.npmjs.com/package/aws4

It not only can do this synchronously, but in my benchmarks, it does it about 20x faster than AWS's official package does it.

Here's how to synchronously sign a simple S3 get request in only 4 lines of code using that package:

var aws4  = require('aws4');
var opts = { host: 's3.amazonaws.com', path: `${YOUR_BUCKET}/${YOUR_KEY}`, service: 's3', region: YOUR_REGION, signQuery: true };
const result = aws4.sign(opts, { accessKeyId: YOUR_ACCESS_KEY, secretAccessKey: YOUR_SECRET_ACCESS_KEY })
const url = `https://${result.host}/${result.path}`;

It's MIT licensed and is an extremely popular package - as of right now, it has 17 million downloads in the past week - so it's got a good deal of support behind it, too.

I hate resorting to third-party libraries like this, but it's a far better solution than restructuring my entire code base to work around this one change in the official SDK. I hope this helps someone in the future.

[charlesritchea]

Or having to, I dunno, fork cesium. Thank you so much for this workaround

On Thu, Aug 8, 2024, 11:42 AM dangerbacon @.***> wrote:

For anyone who discovers this ticket in the future:

There is an NPM package called "aws4" that can sign requests synchronously using the new AWS Signature Version 4 standard. Here's the URL for it: https://www.npmjs.com/package/aws4

It not only can do this synchronously, but in my benchmarks, it does it about 20x faster than AWS's official package does it.

Here's how to synchronously sign a simple S3 get request in only 4 lines of code using that package:

var aws4 = require('aws4'); var opts = { host: 's3.amazonaws.com', path: ${YOUR_BUCKET}/${YOUR_KEY}, service: 's3', region: YOUR_REGION, signQuery: true }; const result = aws4.sign(opts, { accessKeyId: YOUR_ACCESS_KEY, secretAccessKey: YOUR_SECRET_ACCESS_KEY }) const url = https://${result.host}/${result.path};

It's MIT licensed and is an extremely popular package - as of right now, it has 17 million downloads in the past week - so it's got a good deal of support behind it, too.

I hate resorting to third-party libraries like this, but it's a far better solution than restructuring my entire code base to work around this one change in the official SDK. I hope this helps someone in the future.

— Reply to this email directly, view it on GitHub https://github.com/aws/aws-sdk-js-v3/issues/5509#issuecomment-2276137731, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAMUMHMVTWUFSYC44XGF53TZQOGXVAVCNFSM6AAAAAA7VGM2M2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENZWGEZTONZTGE . You are receiving this because you were mentioned.Message ID: @.***>

[github-actions[bot]]

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.