Conversation
| let response; | ||
| try { | ||
| response = await baseFetchFunction(requestClone); | ||
| response = await baseFetchFunction(requestClone, fetchOptions); |
There was a problem hiding this comment.
My first idea was to pass down the entire args again but this would overwrite the changes we do to the request like adding the trace header as the headers from the options take priority over the headers that are already set for a request.
There was a problem hiding this comment.
Consider this code:
const agent = new Agent({maxSockets: 1234});
const request = new Request('someurl', { dispatcher: agent });
fetch(request);
Here, fetch is only called with one parameter. The dispatcher is "baked in" into the request. For undici, it's actually a symbol property. When you call baseFetchFunction(requestClone, fetchOptions), requestClone doesn't contain that property, So that per-request-setting wouldn't be passed on.
There was a problem hiding this comment.
Made an attempt to fix this. Sorry for the complexity, but it should work more reliably for you.
#698
There was a problem hiding this comment.
#698 fixes a different issue that is there so I would open another issue to point to your PR to.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #696 +/- ##
=======================================
Coverage 83.56% 83.56%
=======================================
Files 36 36
Lines 1813 1813
=======================================
Hits 1515 1515
Misses 298 298 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Issue #650:
Description of changes:
Support setting the dispatcher for the global fetch implementation.
Instead of ignoring the created request, we now use it and pass the dispatcher option if it is available.
Added an integration test with a reverse proxy to check that the trace header is there.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.