aws / aws-lambda-builders

Python library to compile, build & package AWS Lambda functions for several runtimes & framework
Apache License 2.0
334 stars 139 forks source link

NodejsNpmEsbuildBuilder:NpmInstall - NPM Failed: npm WARN config production Use `--omit=dev` instead #553

Open yegorpetrov opened 11 months ago

yegorpetrov commented 11 months ago

Description:

It looks like at some point npm deprecated the --production parameter and started WARNing about it in stderr, which seems to break sam build.

I'm not sure which version of npm introduced that but the relevant range in npm code git-blames back to a couple years ago.

I found --production still in unconditional use in the corresponding builder {actions.py:111}. I think it needs to be upgraded to --omit=dev depending on the npm version.

Steps to reproduce:

sam build with a node_modules layer and npm version >= 9.6.7 Alternatively, just run npm i --production and observe the same warning. Clearly, it's not supposed to be called with --production anymore.

Observed result:

 Running NodejsNpmEsbuildBuilder:CopySource
 Running NodejsNpmEsbuildBuilder:CopySource
 Running NodejsNpmEsbuildBuilder:NpmInstall
 Running NodejsNpmEsbuildBuilder:NpmInstall
 Running NodejsNpmEsbuildBuilder:NpmInstall
 Running NodejsNpmEsbuildBuilder:EsbuildBundle
 Running NodejsNpmEsbuildBuilder:EsbuildBundle
 Running NodejsNpmEsbuildBuilder:EsbuildBundle
Build Failed
Error: NodejsNpmEsbuildBuilder:NpmInstall - NPM Failed: npm WARN config production Use `--omit=dev` instead.

Expected result:

Wouldn't expect to see the warning, and wouldn't expect it to be treated as an error.

Additional environment details (Ex: Windows, Mac, Amazon Linux etc)

  1. OS: image: node:18
  2. If using SAM CLI, sam --version: 1.98.0
  3. AWS region: us-east-1

Add --debug flag to any SAM CLI commands you are running

So the CI/CD pipeline failed, and I went to create this bug report, and then I read about --debug and added it to sam commands in the pipeline... But it did not fail this time! I'm confused and I cannot reproduce this issue reliably. Still, just looking at the code of aws-lambda-builders and npm, the problem seems to be present.

yegorpetrov commented 11 months ago

Ok, it looks like the failure isn't really about the "--production" switch. I now have logs with --debug enabled and I'm seeing this:

2023-10-05 05:47:17,074 | NodejsNpmEsbuildBuilder:NpmInstall failed
Traceback (most recent call last):
  File "/root/.local/pipx/venvs/aws-sam-cli/lib/python3.11/site-packages/aws_lambda_builders/workflows/nodejs_npm/actions.py", line 116, in execute
    self.subprocess_npm.run(command, cwd=self.install_dir)
  File "/root/.local/pipx/venvs/aws-sam-cli/lib/python3.11/site-packages/aws_lambda_builders/workflows/nodejs_npm/npm.py", line 84, in run
    raise NpmExecutionError(message=err.decode("utf8").strip())
aws_lambda_builders.workflows.nodejs_npm.npm.NpmExecutionError: NPM Failed: npm WARN config production Use `--omit=dev` instead.
npm ERR! code 1
npm ERR! path /tmp/tmp6vedj3ok/node_modules/esbuild
npm ERR! command failed
npm ERR! command sh -c node install.js
npm ERR! node:internal/errors:865
npm ERR!   const err = new Error(message);
npm ERR!               ^
npm ERR! 
npm ERR! Error: Command failed: /tmp/tmp6vedj3ok/node_modules/esbuild/bin/esbuild --version
npm ERR!     at checkExecSyncError (node:child_process:890:11)
npm ERR!     at Object.execFileSync (node:child_process:926:15)
npm ERR!     at validateBinaryVersion (/tmp/tmp6vedj3ok/node_modules/esbuild/install.js:98:28)
npm ERR!     at /tmp/tmp6vedj3ok/node_modules/esbuild/install.js:285:5 {
npm ERR!   status: null,
npm ERR!   signal: 'SIGSEGV',
npm ERR!   output: [ null, Buffer(0) [Uint8Array] [], Buffer(0) [Uint8Array] [] ],
npm ERR!   pid: 1891,
npm ERR!   stdout: Buffer(0) [Uint8Array] [],
npm ERR!   stderr: Buffer(0) [Uint8Array] []
npm ERR! }
npm ERR! 
npm ERR! Node.js v18.18.0

So actually it's esbuild crashing with a SIGSEGV in its own postinstall action. The "--production" error is there only because the npm builder shows stderr upon non-zero exit code from npm, but essentially it's unrelated.

I now removed esbuild from package.json and left only its globally installed version. And I also call "esbuild --version" just before "sam build" in the CI/CD script to make sure that esbuild is fully installed. Guess what, I'm yet to see any failures.

sriram-mv commented 11 months ago

Is the issue that npm exits with a non zero exit code, but the relevant work that it needed to do is done anyway?

yegorpetrov commented 10 months ago

@sriram-mv

Is the issue that npm exits with a non zero exit code, but the relevant work that it needed to do is done anyway?

I would say that the issue is NodejsNpmEsbuildBuilder "lying" about the error. It shows...

Build Failed
Error: NodejsNpmEsbuildBuilder:NpmInstall - NPM Failed: npm WARN config production Use `--omit=dev` instead.

...which is misleading, because the actual reason is esbuild crashing with a segmentation violation without any traces in stderr. In other words, NodejsNpmEsbuildBuilder should not blindly show the latest stderr output as the reason for build failure. I had to use --debug to find out the true reason after spending hours speculating why "npm WARN config production" could break the build (it didn't).

As for esbuild dying with a SIGSEGV, my workaround was to install and test it (esbuild --version) before running sam build in my CI/CD script. But that's a whole different story.