(aws-lambda-nodejs): Wrong @aws-sdk bundling when using format: OutputFormat.ESM and externalModules: []

[WtfJoke]

Describe the bug

We have a project using esModules. We want to utilize esModules also for our Lambdas.
When using NodeJsFunction and provide the following custom bundling options: bundling: { externalModules: [], format: OutputFormat.ESM }, the resulting .mjs file contains wrong imports (dist-cjs instead of dist-es).

Here is the section of the bundled .mjs file

2024-02-29T13:52:13.280Z	undefined	ERROR	Uncaught Exception 	
{
    "errorType": "Error",
    "errorMessage": "Dynamic require of \"os\" is not supported",
    "stack": [
        "Error: Dynamic require of \"os\" is not supported",
        "    at file:///var/task/index.mjs:12:9",
        "    at node_modules/@smithy/shared-ini-file-loader/dist-cjs/getHomeDir.js (file:///var/task/index.mjs:1877:16)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/shared-ini-file-loader/dist-cjs/index.js (file:///var/task/index.mjs:1991:29)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/node-config-provider/dist-cjs/index.js (file:///var/task/index.mjs:2152:41)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/middleware-endpoint/dist-cjs/adaptors/getEndpointFromConfig.js (file:///var/task/index.mjs:2231:34)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/middleware-endpoint/dist-cjs/index.js (file:///var/task/index.mjs:2516:40)"
    ]
}

When setting only format option bundling: { format: OutputFormat.ESM },, everything works as expected, because the provided aws-sdk is loaded (instead of the bundled one).

We set externalModules: [] because that leads to lower cold start times (reference: #25492), which means the @aws-sdk is bundled.

Expected Behavior

I expect that no CommonJS will be used when setting OutputFormat.esm in the bundling options.

Current Behavior

The resulting .mjs file contains commonJs references, which leads to the function crashing:

// node_modules/@aws-sdk/util-user-agent-node/dist-cjs/index.js
var require_dist_cjs42 = __commonJS({
  "node_modules/@aws-sdk/util-user-agent-node/dist-cjs/index.js"(exports, module) {

The function crashes with the following exception:

2024-02-29T13:52:13.280Z	undefined	ERROR	Uncaught Exception 	
{
    "errorType": "Error",
    "errorMessage": "Dynamic require of \"os\" is not supported",
    "stack": [
        "Error: Dynamic require of \"os\" is not supported",
        "    at file:///var/task/index.mjs:12:9",
        "    at node_modules/@smithy/shared-ini-file-loader/dist-cjs/getHomeDir.js (file:///var/task/index.mjs:1877:16)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/shared-ini-file-loader/dist-cjs/index.js (file:///var/task/index.mjs:1991:29)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/node-config-provider/dist-cjs/index.js (file:///var/task/index.mjs:2152:41)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/middleware-endpoint/dist-cjs/adaptors/getEndpointFromConfig.js (file:///var/task/index.mjs:2231:34)",
        "    at __require2 (file:///var/task/index.mjs:18:50)",
        "    at node_modules/@smithy/middleware-endpoint/dist-cjs/index.js (file:///var/task/index.mjs:2516:40)"
    ]
}

Reproduction Steps

1. Run cdk synth on that project: https://github.com/WtfJoke/cdk-nodejs-esm-bug-reproducer
2. Inspect the resulting .mjs file

Alternatively:

1. Deploy that project: https://github.com/WtfJoke/cdk-nodejs-esm-bug-reproducer
2. Run the CdkWorkshopStack-MyLambda lambda and inspect the error

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

10.2.4

Framework Version

No response

Node.js Version

v20.11.0

OS

windows (wsl)

Language

TypeScript

Language Version

5.3.3

Other information

No response

[pahud]

This would need further investigation. I'm making it a p1 but we welcome any pull requests from the community.

[Tietew]

Following settings works for me:
For details, see https://esbuild.github.io/api/#main-fields

const banner =
  "const require = (await import('node:module')).createRequire(import.meta.url);const __filename = (await import('node:url')).fileURLToPath(import.meta.url);const __dirname = (await import('node:path')).dirname(__filename);";
const externalModules = [
  '@aws-sdk/credential-provider-cognito-identity',
  '@aws-sdk/credential-provider-http',
  '@aws-sdk/credential-provider-ini',
  '@aws-sdk/credential-provider-process',
  '@aws-sdk/credential-provider-sso',
  '@aws-sdk/credential-provider-web-identity',
  '@smithy/credential-provider-imds',
];

new nodejs.NodejsFunction(this, 'Function', {
  entry: 'function.ts',
  runtime: lambda.Runtime.NODEJS_20_X,
  bundling: {
    banner,
    externalModules,
    format: nodejs.OutputFormat.ESM,
    mainFields: ['module', 'main'],
  },
});

The externalModules above prevents to bundle unneeded credential providers on Lambda.

[WtfJoke]

Specifying mainFields: ['module', 'main'] works.

The cdk docs to mainFields outlines that too:

How to determine the entry point for modules.
Try ['module', 'main'] to default to ES module versions.

It seems it would make sense to specify mainFields to that value when you use OutputFormat.ESM.

It seems also an issue with aws-sdk bundling as well, according to evanw/esbuild#2692 and aws/aws-sdk-js-v3#4217. If I understand the issues correctly, when these would be fixed its not needed to overwrite mainFields.

[Vandita2020]

Hey @WtfJoke, this is a dependency issue. ESBuild converts import statements in bundled packages to require but not vice versa. So when you emit to esm you'll get this error if any of your code is commonjs.

There are workarounds for this,

1. As mentioned by @Tietew using mainFields: ['module', 'main'] where we tell esbuild to use the module entry point.
2. Another one is to set banner to inject require into the script using banner: "import { createRequire } from 'module';const require = createRequire(import.meta.url)

[WtfJoke]

Hey @Vandita2020

Yeah I'm currently using the mentioned workarounds (mainfields and banner), but I would have expected that this workarounds shouldn't be necessary when using only esModules compatible libraries like in this case.

Have you seen my reproducer here: https://github.com/WtfJoke/cdk-nodejs-esm-bug-reproducer?
The code in question:

import { SSM } from "@aws-sdk/client-ssm";

export const handler = async () => {
  new SSM().putParameter({
    Name: "my-param",
    Value: "my-value",
    Overwrite: true,
  });
};

It only uses @aws-sdk v3 as dependency, which is also ESM. So for me it looks like either issue with aws-sdk (not correct way of distributing esmodules code) or aws-cdk (not correct esbuild bundling settings).

Do you think that this issue should be closed in favour of aws/aws-sdk-js-v3#4217 or a new issue in aws-sdk?

[Vandita2020]

Hey @WtfJoke,
I have replicated your code and worked with it. I also made modifications to simplify it by bundling the code without using CDK. Additionally, I have discussed this matter with AWS-SDK team, and it appears that everything points this to be an issue with ESBuild.

[WtfJoke]

Hey @Vandita2020

Thanks for taking the time, bundle the code without using CDK and discussing this also with the AWS-SDK team. Appreciate that.

Currently I do not understand why your mentioned issue is the cause for that (as far as I see, there is also no response from the esbuild or the aws sdk maintainers there). Is it possible to get a bit more of an explanation here?

There exists a previous issue in esbuild for the aws sdk, where the esbuild maintainer responded, concluding that there is an issue in the aws sdk.

For me it looks a bit like a deadlock here 🙈

Any ideas how we can progress from here?

P.S. For cdk/NodeJsFunction a solution could be to default the mainFields to ['module', 'main'] so ESM is forced when specifying OutputFormat.ESM. I would be open for creating this PR, if thats a reasonable solution.