(aws-lambda-nodejs): treat @smithy scope identically to @aws-sdk scope if excluding packages

[kuhe]

Describe the bug

In this readme section, https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-lambda-nodejs/README.md#externals, it states that

When passing a runtime that is known to include a version of the aws sdk, it will be excluded by default. For example, when passing NODEJS_16_X, aws-sdk is excluded. When passing NODEJS_18_X, all @aws-sdk/* packages are excluded.

However, the core runtime of the AWS SDK for JavaScript (v3) is contained in the scope @smithy/* as can be seen in e.g. client-s3/package.json. If a user application has a certain version of the AWS SDK with both core runtime @smithy/ and AWS packages under @aws-sdk/, deleting only the @aws-sdk/ packages can create a fatal version mismatch between the two sets of dependencies.

From the AWS SDK team's perspective, we recommend that the SDK is not removed from user applications due to the AWS Lambda provided SDK having an unpredictable version update cadence. A sudden change in version not initiated by the user may cause errors due to changes in behavior even if technically no "breaking" interface changes occur.

However, if continuing to remove the @aws-sdk node_modules folder, the @smithy folder should be treated identically since they are tightly coupled.

Regression Issue

• Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

If deleting @aws-sdk/ folder, also delete @smithy/ folder. Both NPM scopes are fully controlled by AWS teams.

Current Behavior

Only @aws-sdk/ is deleted, causing mismatch between those and the @smithy/ packages.

Reproduction Steps

Use NODEJS_18_X runtime and bundle the @aws-sdk/client-s3 package as an example.

Inspect the node_modules folder for the existence of @smithy/ packages.

Possible Solution

If deleting @aws-sdk/ folder, also delete @smithy/ folder.

Additional Information/Context

No response

CDK CLI Version

n/a

Framework Version

No response

Node.js Version

NODEJS_18_X, possibly others except NODEJS_LATEST

OS

n/a

Language

TypeScript

Language Version

No response

Other information

No response

[pahud]

@kuhe Thank you. We'll bring this up to the team immediately.

[GavinZZ]

Thanks for creating this issue. We had a team discussion and we believe this is a valid issue and I'll kick off a PR to fix this issue.

[GavinZZ]

Discussed with @kuhe offline, and here's the update.

Context

Node 16 or below would by default use AWS SDK v2 and Lambda provides the SDK by default so CDK removed @aws-sdk modules. See this line.

Similarly for Node 18+ runtimes, since AWS Lambda includes AWS SDK v3 by default, and CDK excludes all the @aws-sdk/* packages because they’re expected to already be present. However, the CDK currently removes only the @aws-sdk/* packages when bundling for Node 18+ runtimes, but it does not remove the @smithy/* packages. This can cause a mismatch in versions between the @smithy/* packages and the AWS SDK packages that AWS Lambda provides.

The mismatch can happen in the following scenarios. This is a pretty edge case but customers did encounter this issue.

/user-app/node_modules/
  - /@smithy/* (v123) <-- this gets used because it wasn't deleted
  - /@aws-sdk/*  (v123) <-- CDK removes `@aws-sdk/*` currently
/lambda-hidden-folder/node_modules
  - /@smithy/* (v456)
  - /@aws-sdk/* (v456) <-- this gets used as fallback since the module is removed from node_modules by CDK

The reason that we don't need to remove smithy models for AWS SDK v2 is because that AWS SDK v3 is built on top of Smithy models and SDK v2 is not.

Proposed Solutions

There are two solutions to the issue.

1. We remove smithy as well from the bundling. This is an acceptable but not the best change from AWS SDK's perspective. Reason being that

let's say the user deploys the SDK at version v123, but it gets removed so they're actually on version v456, then Lambda does an update of the provided SDK and now they're suddenly on v789, it could cause minor issues 
1. it's hard to troubleshoot because they think they're on v123
2. services can rarely change things like endpoints between versions which cause breaking change

2. We change the default behaviour to not remove @aws-sdk/*. This is not the best change from CDK's perspective as changing default behaviour may bring unexpected change. Although this doesn't seem and shouldn't cause any regression, this is not the safest change from CDK side.

[github-actions]

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.