aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.42k stars 3.81k forks source link

:exclamation: NOTICE (aws-lambda-nodejs): Runtime.ImportModuleError: `Cannot find module 'module'` #30717

Open ykageyama-mondo opened 1 month ago

ykageyama-mondo commented 1 month ago

Please add your +1 👍 to let us know you have encountered this

Status: RESOLVED

Fixed in v2.147.3.

Overview:

With the v0.22 release of esbuild, the default bundling behavior has been changed to omit imported packages from the bundle by default when the platform is node. The recommendation is to set the packages option to bundle to get the desired bundling behavior.

Complete Error Message:

Cannot find module 'source-map-support/register'\nRequire stack:\n- /var/task/index.js\n- /var/runtime/index.mjs

Workaround:

We recommend pinning the version to 0.2.15

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

some_lambda = NodejsFunction(
    scope=self,
    ...
    bundling=nodejs_bundling_options,
)

Note: You may also add the flag --packages=bundle but this may result in other errors and is not recommended.

Solution:

We have pinned the version of esbuild used when no version is already installed to 0.2.15.


Original Issue Content:

Expected Behavior

When executing the lambda, import package from 'somePackage' imports the target package

Current Behavior

When executing the lambda, import package from 'somePackage' throws a Runtime.ImportModuleError with message Cannot find module 'somePackage'

Reproduction Steps

  1. Clone https://github.com/ykageyama-mondo/esbuild-issue-recreation
  2. Install and deploy
  3. Execute deployed lambda

Possible Solution

Add --packages=bundle to the default esbuildCommand created. Tried this on the reproduction lambda but seems to error with Must use "outdir" when there are multiple input files. I'm not familiar enough with esbuild to understand why this is happening and how to fix it.

Additional Information/Context

No response

CDK CLI Version

2.147.2

Framework Version

2.147.2

Node.js Version

18.17.1

OS

Ubuntu 22.04.4 LTS

Language

TypeScript

Language Version

5.5.2

Other information

No response

ykageyama-mondo commented 1 month ago

The temporary solution to this is to pin the version of esbuild to ~0.21.

ykageyama-mondo commented 1 month ago

The README section on modules will also need to be updated to reflect the change in default behaviour

barendb commented 1 month ago

We made the following change and that fixed the issue

Issue reported on esbuild gh as well

https://github.com/evanw/esbuild/issues/3817

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

some_lambda = NodejsFunction(
    scope=self,
    ...
    bundling=nodejs_bundling_options,
)
tobiberger commented 1 month ago

Another temporary solution is to add the now required parameter instead of specifying the version. This will keep the old bundling functionality working with the new esbuild version.

In JS/TS this would look somewhat like this:

new NodejsFunction(scope, "my-esbuild-lambda", {
  functionName: "awesome-function",
  ...
  bundling: {
    esbuildArgs: {
      "--packages": "bundle",
    },
  },
})
mrgrain commented 1 month ago

Everyone looking for a temporary fix, I recommend doing this until we have settled on a solution in CDK:

We made the following change and that fixed the issue

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

some_lambda = NodejsFunction(
    scope=self,
    ...
    bundling=nodejs_bundling_options,
)
kristof-kasa commented 1 month ago

@tobiberger solution worked for me. <3

tobiberger commented 1 month ago

As there are already some people liking my proposed solution, here's something for your consideration: I personally prefer this approach, because it avoids locking esbuild to an outdated version, which might also be easy to forget and stay like that for longer than intended. However, I expect that a proper fix of the underlying issue by the aws-cdk lib might very well be in conflict with this setting. I guess it will either include packages in the lambda bundle or (hopefully) it will add the --packages=bundle parameter by default, which might lead to errors in builds once we get the cdk update. This is all speculation of course, but if you value a high certainty of perfectly running pipelines over ensuring up to date dependencies, then you should probably go with @mrgrain's approach and hard-code the esbuild version. Just make sure you don't forget to revert when this issue is resolved.

One more thing you should know is that setting the esbuild version in cdk code only works when using docker for bundling. This is the default, so it should work for most people. However, if your cdk project is running in a node environment where esbuild is already installed and you haven't specified the forceDockerBundling bundling option, it will run with the installed version, no matter what you specify. Of course, then it's still your decision whether you lock the esbuild version or add the bundling option.

Edit: I just realized that the fact about using the esbuild version present in a node project can be very useful for large projects, as it provides the opportunity to set the esbuild version for the whole project in one place instead of setting it in every NodejsFunction constructor.

Ys-Zhou commented 1 month ago

Hey, aws guys. This is really a critical issue. I have a huge CDK project and I cannot add --packages=bundle to every NodejsFunction resource. So please set it as the default bundling behavior in CDK or provide us a global option for setting it. I will lock esbuild to 0.21.5 for now but I want to know your decision.

TheRealAmazonKendra commented 1 month ago

We are currently in the process of creating a patch release with the esbuild version pinned to 0.21.5 to get customers out of immediate pain. We will also look at adding the --packages=bundle flag in but we will not include that in the patch release in case there are unintended consequences that also be customer impacting.

mrgrain commented 1 month ago

As there are already some people liking my proposed solution, here's something for your consideration: I personally prefer this approach, because it avoids locking esbuild to an outdated version, which might also be easy to forget and stay like that for longer than intended. However, I expect that a proper fix of the underlying issue by the aws-cdk lib might very well be in conflict with this setting. I guess it will either include packages in the lambda bundle or (hopefully) it will add the --packages=bundle parameter by default, which might lead to errors in builds once we get the cdk update. This is all speculation of course, but if you value a high certainty of perfectly running pipelines over ensuring up to date dependencies, then you should probably go with @mrgrain's approach and hard-code the esbuild version. Just make sure you don't forget to revert when this issue is resolved.

Thank you for the balanced write-up @tobiberger ! My main concern right now is that --packages=bundle is not backwards compatible. So for anyone using a version < 0.22.0, just adding this in will fail the build.

$ touch index.js
$ npx esbuild@0.21.x index.js --packages=bundle
✘ [ERROR] Invalid value "bundle" in "--packages=bundle"

  The only valid value is "external".

1 error
image
elasticrash commented 1 month ago

my workaround for Lambdas functions was to pin the version of esbuild in the NodeJsFunction

this.lambda = new NodejsFunction(this,
    StateApi, {
        ....
        bundling: {
            esbuildVersion: "0.21.0",
    }
});
github-actions[bot] commented 1 month ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

ykageyama-mondo commented 1 month ago

I am not sure if this issue should be marked a closed. The fix to pin the esbuild version in the docker file does not resolve the issue where the local bundling is broken by default for esbuild version 0.22.

TheRealAmazonKendra commented 1 month ago

The closure happened automatically because the linked PR was merged. I'm not sure, however, if we will be able to support a touch-free upgrade to 0.22. We cannot update the default to include --packages=bundle because it will break people with certain configurations. Now that we have the immediate fix released, however, we can dig into this further.

ykageyama-mondo commented 1 month ago

Thanks for re-opening the issue @TheRealAmazonKendra. Would we be able to add the option conditionally when this.esbuildInstallation?.version >= '0.22'? Are there any concerns with this approach?

github-actions[bot] commented 1 month ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.

dreamorosi commented 1 month ago

Esbuild released a new version that reverts the change (0.23), so that it now behaves the same as pre-0.22.

Despite all this, I think CDK should consider pinning to a known working version at least in the docker-based build.

MightySepp666 commented 1 month ago

I would also like to share some of my expectations and opinions on this matter.

Despite all this, I think CDK should consider pinning to a known working version at least in the docker-based build.

Absolutely agree. ESBuild doesn't follow semantic versioning and uses minor versions for breaking changes, as stated by the maintainer in #3817:

The major version is 0 so the minor version is used for breaking changes.

In the release notes of 0.23 he also strongly advocates on pinning the version:

I've just been made aware that Amazon doesn't pin their dependencies in their "AWS CDK" product, which means that whenever esbuild publishes a new release, many people (potentially everyone?) using their SDK around the world instantly starts using it without Amazon checking that it works first. This change in version 0.22.0 happened to break their SDK. I'm amazed that things haven't broken before this point. This revert attempts to avoid these problems for Amazon's customers. Hopefully Amazon will pin their dependencies in the future.

However, I would also like to point out that any build dependencies have to be pinned to expected versions, in order to avoid any unexpected system outages like we had this time. There's nothing worse, than having a version deployed to a dev/staging system that works and deploying the same version without changes a day later to production and having an outage 😕.

For example, I saw this line in the Docker build logs, which suggests that TypeScript is also not pinned to any version (not even a major version):

RUN npm install --global typescript

Maybe other dependencies are affected as well, so it would be great, if you could check this.

We cannot update the default to include --packages=bundle because it will break people with certain configurations.

As long as the build/stack synth fails, I'm personally fine with this, as long as there's an expressive error message telling me what to do. For me it's much more preferable to see a build time error and a broken pipeline, than an unexpected runtime error in a production system 😉.

sholtomaud commented 4 weeks ago

Another comment.

a) NodejsFunction is cool. b) AWS is a $2 trillion stock value company.

Therefore, why not just buy ESBUILD, or create your own, and roll it into CDK CLI?

Also, the more AWS controls the code, the more it can control the DevSecOps pipeline, the (hopefully) more reliable the CDK, and the minimisation of these 3rd party derived issues. Right?

ajhool commented 3 weeks ago

I bumped to the latest aws-cdk version 2.148.0 and was still seeing the problem. I realized it is because I also had esbuild@0.21.0^ in my package.json

For those unaware, you can install esbuild (eg. yarn add esbuild) and CDK will use the NPM version instead of docker. I find the performance of the npm esbuild to be faster but I haven't looked closely at where that performance bump comes from.

So, if you've bumped cdk and are still seeing this issue, you might have esbuild in your package.json. You'll need to install the esbuild@0.23.0 and it should be pinned to the version.

#  Or whatever the latest working version is...
yarn add -D esbuild@0.23.0
tobiberger commented 3 weeks ago

For those unaware, you can install esbuild (eg. yarn add esbuild) and CDK will use the NPM version instead of docker. I find the performance of the npm esbuild to be faster but I haven't looked closely at where that performance bump comes from.

Docker bundling will run a docker build ever time you run it. If the layers aren't already built on the machine, this means installing yarn, pnpm, typescript, and esbuild. That might be where the performance difference comes from.