aws-lambda-nodejs: Overrides of handler always get prefixed with index.

[huntharo]

Describe the bug

Some lambda extensions require changing the handler to a specific value but aws-lambda-nodejs always prefixes this with index., breaking the intended values.

For example, the AWS Lambda Web Adapter requires run.sh:

https://github.com/awslabs/aws-lambda-web-adapter#lambda-functions-packaged-as-zip-package-for-aws-managed-runtimes

For example, an extension from NewRelic requiresnewrelic-lambda-wrapper.handler:

https://docs.newrelic.com/docs/serverless-function-monitoring/aws-lambda-monitoring/enable-lambda-monitoring/instrument-your-own/

Expected Behavior

If a handler is set to a value with a . in it then the index. prefix logic should not apply.

Current Behavior

Values like run.sh are turned into index.run.sh

Reproduction Steps

new lambdaNodejs.NodejsFunction(this, 'my-lambda-func', {
      functionName: 'my-lambda-name',
      architecture: lambda.Architecture.ARM_64,
      entry: './packages/my-lambda/src/index.ts',
      handler: 'run.sh',                                                             // <--- this will turn into `index.run.sh`
      logRetention: logs.RetentionDays.ONE_MONTH,
      memorySize: 1769,
      timeout: cdk.Duration.minutes(15),
      runtime: lambda.Runtime.NODEJS_16_X,
      bundling: {
        sourceMap: !shared.isTestBuild,
        minify: !shared.isTestBuild,
commandHooks: {
            beforeInstall: () => [],
            beforeBundling: () => [],
            afterBundling: (inputDir: string, outputDir: string) => {
              return [
                `${os.platform() === 'win32' ? 'copy' : 'cp'} ${path.join(
                  inputDir,
                  'packages',
                  'my-lambda',
                  'run.sh'                                                                  // <--- this won't be found because handler looks for `index.run.sh`
                )} ${outputDir}`,
              ];
            },
          },
      },
    });

Possible Solution

I have personally been using a patch for this for 15 months that does not add the index. prefix if the user intentionally sets the handler to a value that includes a .:

--- a/node_modules/@aws-cdk/aws-lambda-nodejs/lib/function.js
+++ b/node_modules/@aws-cdk/aws-lambda-nodejs/lib/function.js
@@ -44,7 +44,7 @@ class NodejsFunction extends lambda.Function {
                 depsLockFilePath,
                 projectRoot,
             }),
-            handler: `index.${handler}`,
+            handler: handler.indexOf('.') !== -1 ? `${handler}` : `index.${handler}`,
         });
         // Enable connection reuse for aws-sdk
         if ((_g = props.awsSdkConnectionReuse) !== null && _g !== void 0 ? _g : true) {

The code is unchanged, essentially, even today:

aws-cdk/packages/@aws-cdk/aws-lambda-nodejs/lib/function.ts

Line 111 in 0ebbf58

handler: `index.${handler}`,

Additional Information/Context

aws-lambda-nodejs is intended to the an easy button for bundling Node.js apps, and it largely performs well at that task.

But if the user intentionally overrides the handler value then the easy button quickly becomes more difficult as you either have to patch this behavior with patch-package after tracing it through in a debugger, or you have to remove all usage of aws-lambda-nodejs just so that you can set the handler and not have it get mangled. Neither of these turn out to be that easy :)

CDK CLI Version

2.66.1

Framework Version

2.66.1

Node.js Version

18

OS

Mac

Language

Typescript

Language Version

n/a

Other information

I can submit a pull request if the solution is thought to be adequate.

If the solution works but needs a feature flag to be enabled I can submit it that way too. Let me know if there is an alternative approach and I'll see if I can submit it that way.

[pahud]

entry: './packages/my-lambda/src/index.ts',
handler: 'run.sh',     

This sounds like an interesting use case. Looks like you are planning to bundle your nodejs app with NodejsFunction wrapped with a shell script to run this with the lambda adapter?

[huntharo]

entry: './packages/my-lambda/src/index.ts',
handler: 'run.sh',     

This sounds like an interesting use case. Looks like you are planning to bundle your nodejs app with NodejsFunction wrapped with a shell script to run this with the lambda adapter?

Oh yeah I already use this in a few internal projects (I made a patch the Lambda Web Adapter to add gzip support recently) where we use NodejsFunction to handle the esbuild invoke and then we use the Lambda Web Adapter to run these simple apps. This might seem a little weird but it ends up being a lot less documentation for me to write, enables the developer to run the app locally with express, and makes it so we don't have to maintain various wrapper apps for running the code locally or in a Lambda. Additionally it allows us to gzip the response in Rust code not in Node.js (which is inefficient at gzip due to the number of copies and base64 encode/decodes in the process). In the case of apps that use this approach we just cheat and rename run.sh to index.run.sh on disk since we know how the modification will happen and the handler will end up being set to index.run.sh.

But in other projects where we're using an external adapter that wants us to set the handler to a specific value we have to have a patch instead that prevents the index. prefix.

[pahud]

@huntharo This is awesome! If you have capacity, we'd love to review your PR when it's ready and we appreciate more feedback around this use case from the community. Thank you!

[huntharo]

@pahud - It looks like the PR #24406 is ready in that all the tests are passing... but let me know if you like the approach or not. I don't think there is a case where using a . in the handler name works with the current automatic prefixing of index..

I'm I am correct then it means no one can be using a . today in the handler name and turning off the prefix in that case is safe. Even if someone was using a . in the handler name they would only have to add back index. to be back in business.

Is there a case where a handler name without a filename can have a dot for a Node.js app? Such as, can you have the handler be a static method of a class and use ClassName.StaticMethod as the handler name? If not then we should be 100% fine with this approach.

Also... please check the documentation. There was not much discussion around the handler parameter before so I'm not sure if I added it in the correct places.

[pahud]

Hi @huntharo

Thank you for your PR contribution. I've seen the CI passed with integ testing update, which is great. I will not be able to review your PR but the CDK core team maintainers will. Feel free to discuss more details with the reviewer in the PR comments. Good luck!

[github-actions]

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