refactor: unify header construction in sendLogs and sendOnClose
- Dominant language
- Jupyter Notebook
- Stars
- 30
- Forks
- 15
- PR merge metrics
- No merged PRs in 30d
Description
## Summary
`sendLogs()` and `sendOnClose()` in `sendLogs.ts` construct HTTP headers independently with diverging logic. This causes bugs and makes adding new headers error-prone.
### Current Divergence
| Behavior | `sendLogs()` | `sendOnClose()` |
|---|---|---|
| `Content-Type` | ✅ | ✅ |
| `Authorization` (string) | ✅ | ✅ |
| `Authorization` (function callback) | ✅ calls the function | ❌ calls `.toString()` → sends function source code |
| `x-api-key` (#124) | ✅ | ✅ |
| `config.headers` (custom) | ✅ | ❌ **missing** |
| `updateAuthHeader()` | ✅ | ❌ **not called** |
| `updateCustomHeaders()` | ✅ | ❌ **not called** |
### Three bugs from this divergence
1. **Function-type `authHeader` broken in `sendOnClose`** — if `authHeader` is a callback (e.g., for token refresh), `sendOnClose` calls `.toString()` which sends the function source code as the `Authorization` header value
2. **Custom headers missing from pagehide flush** — headers registered via `registerHeadersCallback()` are never applied to the final `sendOnClose` flush
3. **Future headers must be added in two places** — every new header type (like `apiKey` in #124) requires changes to both functions
### Proposed Fix
Extract a shared `buildHeaders(config): Headers` helper:
```typescript
function buildHeaders(config: Configuration): Headers {
const headers = new Headers({ "Content-Type": "application/json;charset=UTF-8" });
updateAuthHeader(config);
if (config.authHeader) {
const value = typeof config.authHeader === "function" ? config.authHeader() : config.authHeader;
headers.set("Authorization", value);
}
if (config.apiKey) {
headers.set("x-api-key", config.apiKey);
}
updateCustomHeaders(config);
if (config.headers) {
for (const [header, value] of Object.entries(config.headers)) {
headers.set(header, value);
}
}
return headers;
}
```
Both `sendLogs()` and `sendOnClose()` call `buildHeaders(config)`.
### Testing
Add tests for `sendOnClose` that verify:
- [x] `x-api-key` header is present (test exists after #124)
- [ ] `Authorization` header works with function-type `authHeader`
- [ ] Custom headers from `registerHeadersCallback()` are included
- [ ] Headers from `config.headers` are included
### Context
Pre-existing issue, exposed during review of #124. The `sendOnClose` path is the `pagehide` event handler — it fires when users close tabs or navigate away. Logs sent via this path are currently missing custom headers and have broken auth callback support.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in sendLogs.ts by comparing sendLogs() and sendOnClose() with their existing header-update paths. Add focused sendOnClose tests for callback authentication, registered custom headers, and config.headers, then run the relevant test suite; done means both paths produce the expected headers consistently.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- observability, web-dev
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100