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

lambda-nodejs: Bundling fails with recent pnpm version #25612

Closed
cchanche opened this issue May 16, 2023 · 3 comments · Fixed by #26386
Closed

lambda-nodejs: Bundling fails with recent pnpm version #25612

cchanche opened this issue May 16, 2023 · 3 comments · Fixed by #26386
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/medium Medium work item – several days of effort p1

Comments

@cchanche
Copy link

cchanche commented May 16, 2023

Describe the bug

When bundling a lambda function using lambda_nodejs with no dependencies and with pnpm version 8.4.0 or higher, the script fails to delete the node_modules/.modules.yaml file.

Expected Behavior

The bundling succeeds (no need to delete this file).

Current Behavior

The bundling fails with error :

Bundling asset stack/lambda-fn/Code/Stage...

  ...dk.out/bundling-temp-d1f50916a16a3a6ba493bb8442e9fc9680e83df6b64d7a5183903277f0923fc7/index.js  2.1kb

⚡ Done in 8ms
Already up to date
Done in 314ms
rm: [..]/cdk.out/bundling-temp-d1f50916a16a3a6ba493bb8442e9fc9680e83df6b64d7a5183903277f0923fc7/node_modules/.modules.yaml: No such file or directory

...

Error: Failed to bundle asset stack/lambda-fn/Code/Stage, bundle output is located at [...] exited with status 1

Reproduction Steps

Simple lambda construct :

new lambda.NodejsFunction(this, 'MyFunction', {
  runtime: lambda.Runtime.NODEJS_16_X,
  entry: path.join(__dirname, 'lambda', 'src/index.ts'),
});

/lambda/package.json

{
  "dependencies": {
    "aws-lambda": "^1.0.7"
  },
}

/lambda/src/index.ts

import { DefineAuthChallengeTriggerHandler } from 'aws-lambda';

export const handler: DefineAuthChallengeTriggerHandler = async (event) => {
  return event;
};

/package.json

{
  "dependencies": {
    "aws-cdk": "^2.70.0",
    "aws-cdk-lib": "2.67.0",
    "constructs": "^10.0.0",
  },
  "devDependencies": {
    "esbuild": "^0.17.11",
    "ts-node": "^10.0.0",
    "typescript": "4.9.5"
  }
}

pnpm version :

  • v8.4.0 -> bundling fails
  • below v8.4.0 -> bundling succeeds

Possible Solution

In the bundling.ts source file :

  • add -f flag to the rm command
  • don't perform the rm command if pnpm -v >= 8.4.0

Additional Information/Context

The pnpm v8.0.4 release note specifies clearly :

Do not create a node_modules folder with a .modules.yaml file if there are no dependencies inside node_modules.

Since esbuild bundles the dependencies (unless specified otherwise through lambda_nodejs.NodejsFunctionProps['bundling]['nodeModules']), this will break synthesis of most lambda functions.

CDK CLI Version

2.70.0 (build c13a0f1)

Framework Version

No response

Node.js Version

v16.19.1

OS

macOs 13.3.1 (22E261)

Language

Typescript

Language Version

Typescript (4.9.5)

Other information

Source file is located at packages/aws-cdk-lib/aws-lambda-nodejs/lib/bundling.ts

@cchanche cchanche added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 16, 2023
@pahud pahud added p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels May 16, 2023
@pahud pahud changed the title @aws-cdk/aws-lambda-nodejs: Bundling fails with recent pnpm version lambda-nodejs: Bundling fails with recent pnpm version May 16, 2023
@pahud
Copy link
Contributor

pahud commented May 16, 2023

Thank you for your report. Your proposed solution sounds like a good workaround.

@willgriffin
Copy link

Due to your insight, I was able to figure out why I was getting a corrupted archive invalid crc error when trying to deploy with sst. Thanks!

damienhill added a commit to damienhill/aws-cdk that referenced this issue Jul 17, 2023
…n’t exist (aws#25612)

Added -f flag when removing node_modules/.modules.yaml from output dir as part of ES Build bundling step to support both pnpm 8.3.1 and below behaviour where this file is always generated and 8.4.0 and above where this file is not generated if no node_modules exist.

Signed-off-by: Damien Hill <damien@damienhill.com>
@mergify mergify bot closed this as completed in #26386 Jul 17, 2023
mergify bot pushed a commit that referenced this issue Jul 17, 2023
Fix issue with esbuild bundling step using pnpm 8.4.0 and above where `{outputDir}/node_modules/.modules.yaml` does not always exist and current `rm` command fails bundling process when file is not present.

Relevant change that in [pnpm 8.4.0 release notes](https://github.com/pnpm/pnpm/releases/tag/v8.4.0): 

> Do not create a node_modules folder with a .modules.yaml file if there are no dependencies inside node_modules.

Solved by following prior rejected pull request #25617 and suggestion in original issue #25612 of adding `-f` param to `rm` command to succeed even if file doesn't exist. Updated relevant unit test to expect this flag when using pnpm.

Closes #25612.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

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

colifran pushed a commit that referenced this issue Jul 17, 2023
Fix issue with esbuild bundling step using pnpm 8.4.0 and above where `{outputDir}/node_modules/.modules.yaml` does not always exist and current `rm` command fails bundling process when file is not present.

Relevant change that in [pnpm 8.4.0 release notes](https://github.com/pnpm/pnpm/releases/tag/v8.4.0): 

> Do not create a node_modules folder with a .modules.yaml file if there are no dependencies inside node_modules.

Solved by following prior rejected pull request #25617 and suggestion in original issue #25612 of adding `-f` param to `rm` command to succeed even if file doesn't exist. Updated relevant unit test to expect this flag when using pnpm.

Closes #25612.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
bmoffatt pushed a commit to bmoffatt/aws-cdk that referenced this issue Jul 29, 2023
Fix issue with esbuild bundling step using pnpm 8.4.0 and above where `{outputDir}/node_modules/.modules.yaml` does not always exist and current `rm` command fails bundling process when file is not present.

Relevant change that in [pnpm 8.4.0 release notes](https://github.com/pnpm/pnpm/releases/tag/v8.4.0): 

> Do not create a node_modules folder with a .modules.yaml file if there are no dependencies inside node_modules.

Solved by following prior rejected pull request aws#25617 and suggestion in original issue aws#25612 of adding `-f` param to `rm` command to succeed even if file doesn't exist. Updated relevant unit test to expect this flag when using pnpm.

Closes aws#25612.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda-nodejs bug This issue is a bug. effort/medium Medium work item – several days of effort p1
Projects
None yet
3 participants