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

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

Closed
ykageyama-mondo opened this issue Jul 1, 2024 · 23 comments · Fixed by #30726 or codu-code/codu#969 · May be fixed by NOUIY/aws-solutions-constructs#111, NOUIY/aws-solutions-constructs#112 or codu-code/codu#987
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/medium Medium work item – several days of effort in-progress This issue is being actively worked on. management/tracking Issues that track a subject or multiple issues p0

Comments

@ykageyama-mondo
Copy link

ykageyama-mondo commented Jul 1, 2024

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 ykageyama-mondo added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 1, 2024
@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Jul 1, 2024
@ykageyama-mondo
Copy link
Author

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

@ykageyama-mondo
Copy link
Author

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

@barendb
Copy link

barendb commented Jul 1, 2024

We made the following change and that fixed the issue

Issue reported on esbuild gh as well

evanw/esbuild#3817

nodejs_bundling_options = NodejsBundlingOptions(esbuild_version="0.21.5")

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

@tobiberger
Copy link

tobiberger commented Jul 1, 2024

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
Copy link
Contributor

mrgrain commented Jul 1, 2024

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
Copy link

@tobiberger solution worked for me. <3

@tobiberger
Copy link

tobiberger commented Jul 1, 2024

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
Copy link

Ys-Zhou commented Jul 1, 2024

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.

@pahud pahud added p1 effort/medium Medium work item – several days of effort p0 and removed needs-triage This issue or PR still needs to be triaged. p1 labels Jul 1, 2024
@TheRealAmazonKendra
Copy link
Contributor

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.

@TheRealAmazonKendra TheRealAmazonKendra pinned this issue Jul 1, 2024
@TheRealAmazonKendra TheRealAmazonKendra changed the title aws-cdk-lib/aws-lambda-nodejs: default packages esbuildArg causes package imports to not be bundled ❗ NOTICE (aws-lambda-nodejs): default packages esbuildArg causes package imports to not be bundled Jul 1, 2024
@TheRealAmazonKendra TheRealAmazonKendra changed the title ❗ NOTICE (aws-lambda-nodejs): default packages esbuildArg causes package imports to not be bundled ❗ NOTICE (aws-lambda-nodejs): Runtime.ImportModuleError: Cannot find module 'module' Jul 1, 2024
@TheRealAmazonKendra TheRealAmazonKendra added management/tracking Issues that track a subject or multiple issues in-progress This issue is being actively worked on. @aws-cdk/aws-lambda-nodejs and removed @aws-cdk/aws-lambda Related to AWS Lambda labels Jul 1, 2024
@mrgrain
Copy link
Contributor

mrgrain commented Jul 1, 2024

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 [email protected] index.js --packages=bundle
✘ [ERROR] Invalid value "bundle" in "--packages=bundle"

  The only valid value is "external".

1 error
image

@elasticrash
Copy link

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",
	}
});

mergify bot pushed a commit that referenced this issue Jul 1, 2024
…rror (#30726)

### Issue # (if applicable)

Closes #30717.

### Reason for this change

esbuild introduced a breaking change in v0.22 which caused the build error in `aws-lambda-nodejs` module.

### Description of changes

Pin the esbuild version to 0.21 in Dockerfile

### Description of how you validated changes



### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@mergify mergify bot closed this as completed in #30726 Jul 1, 2024
Copy link

github-actions bot commented Jul 1, 2024

⚠️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.

mergify bot pushed a commit to cdklabs/aws-cdk-notices that referenced this issue Jul 1, 2024
@ykageyama-mondo
Copy link
Author

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
Copy link
Contributor

TheRealAmazonKendra commented Jul 2, 2024

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
Copy link
Author

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?

@mergify mergify bot closed this as completed in 06c14b1 Jul 2, 2024
Copy link

github-actions bot commented Jul 2, 2024

⚠️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
Copy link
Contributor

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
Copy link

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
Copy link

sholtomaud commented Jul 6, 2024

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
Copy link

ajhool commented Jul 9, 2024

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 [email protected]^ 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 [email protected] and it should be pinned to the version.

#  Or whatever the latest working version is...
yarn add -D [email protected]

@tobiberger
Copy link

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.

@xazhao
Copy link
Contributor

xazhao commented Aug 11, 2024

Closing this issue because the error has been mitigated. We will create a separate Github issue to pin the version to prevent this happening in the future.

@xazhao xazhao closed this as completed Aug 11, 2024
Copy link

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 11, 2024
@xazhao xazhao unpinned this issue Aug 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.