fixing headers issue in FetchTransport instantiation#9876
fixing headers issue in FetchTransport instantiation#9876ssakhamu wants to merge 1 commit intocrashlyticsfrom
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the initialization of headers in the OTLPLogExporter to ensure they are treated as an object. The review feedback points out that the current implementation incorrectly ignores function-based header factories and warns of a high-severity issue where headers could accumulate indefinitely due to shared mutable state within the transport layer.
| headers: new Headers( | ||
| typeof config.headers === 'object' | ||
| ? (config.headers as Record<string, string>) | ||
| : {} | ||
| ), |
There was a problem hiding this comment.
The current implementation has a few issues:
- Ignored
HeadersFactory: The checktypeof config.headers === 'object'silently ignoresconfig.headersif it is a function (HeadersFactory). This means dynamic headers provided via the standard OpenTelemetry configuration will not be sent. - Code Complexity: The type check and cast can be simplified while still satisfying the TypeScript compiler.
- Shared Mutable State (High Severity): Note that
FetchTransport(infetch-transport.ts:87) appends dynamic headers directly to theheadersinstance passed in the constructor. Since this single instance is shared across allsend()calls, headers will accumulate indefinitely with every export. While the fix for the mutation should ideally be inFetchTransport.send(), you should be aware that passing a singleHeadersinstance here triggers this behavior.
headers: new Headers(
typeof config.headers === 'function' ? undefined : config.headers
),1 participant
The branch currently has an unresolved error for headers value in OTLPLogExporter constructor's FetchTransport instantiation:
Argument of type 'Record<string, string> | HeadersFactory | undefined' is not assignable to parameter of type 'HeadersInit | undefined'.
Type 'HeadersFactory' is not assignable to type 'HeadersInit | undefined'
The type mismatch issue is fixed by making it consistent with what it is in the crashlytics-traces branch. config.headers is casted as a Record such that a HeadersFactory type cannot be passed into the Headers constructor.