Fix missing headers for fetch

[berndfuhrmann]

Issue #679:

Description of changes:

• For integration tests, added a small HTTP server that receives requests made via fetch. This offers the ability to check headers that were actually supplied in the request.
• I reverted a changed introduced recently, so X-Amzn-Trace-Id headers are sent again
• Exporting enableCapture function. This provides the ability to use this without changing globals. It also provides the ability to use different Request classes, e.g. undici.Request instead of globalThis.Request. This should provide a way for anyone who wants to use undici with dispatcher.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[berndfuhrmann]

Also added this bug to undici:
nodejs/undici#3630

[jj22ee]

Thanks for the fix and improvements for this and the undici package!

The changes lgtm. We can remove package-lock.json in fetch package as this is a monorepo, I suppose your changes to package.json should affect the root package-lock.json instead.

Not sure why the codecov is failing suddenly on the Node16.x/Ubuntu test, I'll look into this (and also #688). It should be unrelated to your PR.

• Randomly got it to work in a different PR somehow, took a couple of tries: https://github.com/aws/aws-xray-sdk-node/actions/runs/10929993247/job/31109948820?pr=686

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.56%. Comparing base (9552603) to head (5470288).

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #687   +/-   ##
=======================================
  Coverage   83.56%   83.56%           
=======================================
  Files          36       36           
  Lines        1813     1813           
=======================================
  Hits         1515     1515           
  Misses        298      298           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jj22ee]

LGTM 🚢

[jj22ee]

I've fixed the failing codecov in #692 and merged the fix into your branch, along with some minor fixes to the package-lock files.

[lennart-land]

@jj22ee with this PR our production code broke, since we use this behind a proxy.
May i ask why the test resolves to response through proxy when fetch options are supplied and also the implementation to support proxies was removed?
At least a minor version release does not look correct here, since it introduced a breaking change to us.

[berndfuhrmann]

@lennart-land
The problem with test was this line:
stubFetch.should.have.been.calledOnceWith(request, {dispatcher: proxyStub});
Expecting this makes the test brittle, because the way adding the header works is to clone the request. When doing that, the dispatcher is "baked" into that request object, so stubFetch would just be called with that request. It works after a fix in undici.
This was the relevant fix on their side: nodejs/undici#3630 (comment)
So with that undici patch, things should work. However, that patch seems to be included only in undici 7, which is in alpha stage currently. That is unfortunate timing.

[jj22ee]

Hi @lennart-land , the previous change to introduce support for proxies in fetch introduced a regression (X-Amzn-Trace-Id header not propagated to downstream when not using a request object in Fetch), so we reverted the change in this PR. That said, you are right that a minor version release was incorrect, that is an error on my end.

For now, the current workaround is to pin aws-xray-sdk-fetch to v3.10.1. In the mean time, when the undici patch is released, that is an alternative option. We'll also look into an alternative solution for proxy support that doesn't break X-Amzn-Trace-Id propagation.

[jj22ee]

Re-opening #650