feat(core): Export a reusable function to add tracing headers#20076
feat(core): Export a reusable function to add tracing headers#20076
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧Core
Other
🤖 This preview updates automatically when you update the PR. |
size-limit report 📦
|
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| type PolymorphicRequestHeaders = | ||
| | Record<string, string | undefined> | ||
| | Array<[string, string]> | ||
| | Iterable<Iterable<string>> |
There was a problem hiding this comment.
Accepted Iterable type lacks runtime handling in function
Low Severity
The new Iterable<Iterable<string>> variant was added to PolymorphicRequestHeaders, which is used as the type for fetchOptionsObj.headers. However, the function body only handles Headers instances (via isHeaders), arrays (via Array.isArray), and plain objects (the else branch). A non-array, non-Headers iterable (e.g. a Map) would silently fall into the else branch, where in operator checks and object spread would not correctly process the iterable's entries, producing malformed headers. Since this function is now exported for cross-package use, the type promises broader acceptance than the implementation supports.
Additional Locations (1)
| }); | ||
|
|
||
| describe('_addTracingHeadersToFetchRequest', () => { | ||
| describe('getTracingHeadersForFetchRequest', () => { |
There was a problem hiding this comment.
Feat PR missing integration or E2E test
Low Severity
This feat PR exports getTracingHeadersForFetchRequest as a new cross-package API but includes no integration or E2E test. The existing unit tests were only renamed, and no new test cases cover the newly accepted URL parameter type or the Iterable<Iterable<string>> header variant. Adding at least one integration test that exercises the new export in a realistic scenario (e.g., Cloudflare Workers fetcher binding) would help validate cross-package usage.
Triggered by project rule: PR Review Guidelines for Cursor Bot
1 participant
This PR is an extraction of #19991
It basically exports
getTracingHeadersForFetchRequest, which was previously only exported for testing, but offers a great functionality if you want to add tracing headers to a request. I renamed it asaddTracingHeadersToFetchRequestsounded a little misleading, as it didn't really add headers to the request, as it returned the extracted headers from the request (or init, if there are any).Open question
I added
@hiddenand@internalto it, not sure if this is an approach we follow. I'm ok to remove it from the jsdoc