DataDog / datadog-lambda-js

The Datadog AWS Lambda Library for Node
Apache License 2.0
113 stars 35 forks source link

feat: Explicitly try/catch require to appease esbuild #409

Closed astuyve closed 1 year ago

astuyve commented 1 year ago

What does this PR do?

Hopefully fixes #408

Motivation

I'm still testing this, want to get it on a PR for integration tests. But apparently our callpath try/catch isn't enough to convince esbuild that this library is provided externally.

Testing Guidelines

I build this project and then tested it locally with npx esbuild dist/index.js --bundle --minify --platform=node and did not receive any errors.

For developers finding this in the future, you can test this is error-free by:

  1. Pulling this project
  2. Running yarn install
  3. removing node_modules/@aws-sdk and node_modules/aws-sdk entirely! <- This step is KEY.
  4. Running yarn build
  5. Running npx esbuild dist/index.js --bundle --minify --platform=node and confirming you do not receive any build errors.

An esbuild error looks like this:

npx esbuild dist/index.js --bundle --minify --platform=node
✘ [ERROR] Could not resolve "aws-sdk/clients/kms"

    dist/metrics/kms-service.js:57:38:
      57 │                         kms = require("aws-sdk/clients/kms");
         ╵                                       ~~~~~~~~~~~~~~~~~~~~~

  You can mark the path "aws-sdk/clients/kms" as external to exclude it from the bundle, which will
  remove this error. You can also surround this "require" call with a try/catch block to handle this
  failure at run-time instead of bundle-time.

1 error

Additional Notes

Types of Changes

Check all that apply

codecov-commenter commented 1 year ago

Codecov Report

Merging #409 (2fa841a) into main (3764dad) will decrease coverage by 0.03%. The diff coverage is 57.14%.

:exclamation: Current head 2fa841a differs from pull request most recent head cebab54. Consider uploading reports for the commit cebab54 to get more accurate results

@@            Coverage Diff             @@
##             main     #409      +/-   ##
==========================================
- Coverage   80.93%   80.91%   -0.03%     
==========================================
  Files          38       38              
  Lines        1983     1986       +3     
  Branches      460      460              
==========================================
+ Hits         1605     1607       +2     
- Misses        319      320       +1     
  Partials       59       59              
Impacted Files Coverage Δ
src/metrics/kms-service.ts 82.14% <57.14%> (-1.86%) :arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more