Skip to content

fix(aws): Ensure lambda layer uses default export from ImportInTheMiddle#12233

Merged
Lms24 merged 1 commit intodevelopfrom
lms/fix-aws-import-in-the-middle
May 27, 2024
Merged

fix(aws): Ensure lambda layer uses default export from ImportInTheMiddle#12233
Lms24 merged 1 commit intodevelopfrom
lms/fix-aws-import-in-the-middle

Conversation

@Lms24
Copy link
Copy Markdown
Member

@Lms24 Lms24 commented May 27, 2024

So this is a fun one:

Our lambda layer relies on one bundled Sentry SDK, which caused problems raised in #12089 and #12009 (comment). Specifically, by bundling import-in-the-middle code into one file, it seems like the library's way of declaring its exports conflict, causing the "ImportInTheMiddle is not a constructor" error to be thrown. While this should ideally be fixed soon in import-in-the-middle, for now this PR adds a small Rollup plugin to transform new ImportInTheMiddle(...) calls to new ImportInTheMiddle.default(...) to our AWS Lambda Layer bundle config.

Once the fixes land upstream and we updated our dependencies accordingly, we should remove this plugin again.

I tested this locally with a reproduction that failed with the exact same error message as reported in #12089. We should add an e2e test that tests the layer (as well as the AWS Serverless SDK). I'm gonna take a look at this today and open a follow up PR.

For now, this (in combination with #12232) hopefully

fixes #12089

@Lms24 Lms24 requested review from AbhiPrasad, lforst and mydea May 27, 2024 12:41
@Lms24 Lms24 self-assigned this May 27, 2024
@Lms24 Lms24 mentioned this pull request May 27, 2024
3 tasks
const bundleTypeConfigMap = {
standalone: standAloneBundleConfig,
addon: addOnBundleConfig,
node: nodeBundleConfig,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is node not used anywhere else? Nice!

Copy link
Copy Markdown
Member Author

@Lms24 Lms24 May 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, the only thing we bundled with this config on the node side is the aws lambda layer bundle

@Lms24 Lms24 changed the title fix(aws): Ensure lambda layer uses default export of ImportInTheMiddle fix(aws): Ensure lambda layer uses default export from ImportInTheMiddle May 27, 2024
Copy link
Copy Markdown
Contributor

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reasonable way for us to test this? maybe with an e2e test? I realize it's maybe too much because this fix is temporary based on the upstream stuff, but I'd like to make sure this doesn't regress again with any future OTEL changes.

@Lms24
Copy link
Copy Markdown
Member Author

Lms24 commented May 27, 2024

Is there a reasonable way for us to test this? maybe with an e2e test? I realize it's maybe too much because this fix is temporary based on the upstream stuff, but I'd like to make sure this doesn't regress again with any future OTEL changes.

Yes, my plan is to add an e2e test that more or less does exactly what our layer would do. Taking a look at this right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error using v8 lambda layer (243)

3 participants