feat(http): inject x-request-id header for log correlation

[runyaga]

Summary

• Inject a unique x-request-id header (UUID v4) on every outgoing HTTP request for client-server log correlation
• Replace timestamp-counter ID generator with UUID v4 for global uniqueness across devices/sessions
• Preserve caller-supplied x-request-id headers for distributed tracing scenarios

Changes

• packages/soliplex_client/pubspec.yaml: Add uuid: ^4.5.2 dependency
• ObservableHttpClient: Switch default ID generator to UUID v4, inject x-request-id header in both request() and requestStream(), respect caller-supplied headers
• Tests: Add 8 new tests covering header injection, UUID format, caller preservation, redaction passthrough, and streaming; update 3 existing tests for enriched headers

Test plan

• dart format --set-exit-if-changed packages/soliplex_client (0 changes)
• dart analyze --fatal-infos packages/soliplex_client (0 issues)
• dart test packages/soliplex_client (all pass)
• flutter test (all pass)
• Manual: inspect outgoing request headers in Network Inspector, confirm x-request-id is a valid UUID v4

Implements milestone 11 from docs/planning/logging/11-x-request-id-correlation.md.

[svarlet]

Code Review Report

Reviewer: Claude (AI assistant)

---

Overall Assessment: ✅ Approve with suggestions

This is a well-designed feature with solid implementation and comprehensive test coverage. The architectural approach is sound - client-generated request IDs are an industry-standard pattern used by observability tools (OpenTelemetry, Datadog) and infrastructure (AWS ALB, Kong, nginx).

---

Architecture Discussion: Is client-generated x-request-id appropriate?

Yes, this is the right approach. Here's why:

1. Industry standard: Major API gateways, proxies, and observability platforms expect and support client-supplied x-request-id headers
2. Network failure visibility: If a request fails before reaching the server (DNS, TLS, timeout), the client still has a correlation ID for its logs
3. End-to-end tracing: The "respect caller-supplied IDs" behavior enables distributed tracing where an upstream service passes a trace context
4. UUID v4 guarantees: Global uniqueness across devices/sessions eliminates collision concerns

The only alternative—server-generated IDs returned in responses—loses value precisely when you need correlation most (failed requests).

---

Strengths

Aspect	Notes
Minimal footprint	Only 22 lines added to production code
Consistent implementation	Both request() and requestStream() follow identical patterns
Test coverage	8 new tests covering edge cases (caller preservation, redaction passthrough, streaming)
Documentation	The planning doc clearly explains design decisions and tradeoffs
Trace propagation	Respecting caller-supplied IDs is crucial for distributed tracing

---

Issue: Case-insensitive header handling 🐛

Severity: Medium

HTTP headers are case-insensitive per RFC 7230 §3.2. The current implementation uses case-sensitive map lookup:

final requestId = headers?['x-request-id'] ?? _generateRequestId();

If a caller passes {'X-Request-Id': 'my-trace'}:

1. Lookup for 'x-request-id' returns null
2. A new UUID is generated
3. enrichedHeaders ends up with both keys: {'X-Request-Id': 'my-trace', 'x-request-id': '<uuid>'}

While most servers will treat these as duplicates (and pick one), this is undefined behavior and could cause confusion in logs or with strict intermediaries.

Suggested fix:

String? _findExistingRequestId(Map<String, String>? headers) {
  if (headers == null) return null;
  for (final entry in headers.entries) {
    if (entry.key.toLowerCase() == 'x-request-id') return entry.value;
  }
  return null;
}

final requestId = _findExistingRequestId(headers) ?? _generateRequestId();

---

Minor Observations

1. static const _uuid = Uuid() — This works because Uuid has a const constructor and is stateless. Worth adding a brief comment for future readers who might question it.

2. Retry semantics — The doc correctly notes that retries get new IDs. Consider adding a test that explicitly demonstrates this behavior (mock a 401 → refresh → retry flow) for documentation purposes.

3. Test for case-insensitivity — Once the fix above is applied, add a test like:

   test('preserves caller-supplied X-Request-Id (case-insensitive)', () async {
     await observableClient.request(
       'GET', uri,
       headers: {'X-Request-Id': 'Mixed-Case-ID'},  // uppercase X-R-I
     );
     // verify only one header sent, with value 'Mixed-Case-ID'
   });

---

Summary

Category	Status
Design	✅ Sound architectural decision
Implementation	⚠️ Case-sensitivity issue needs fix
Tests	✅ Comprehensive
Documentation	✅ Thorough

Recommend addressing the case-insensitive header lookup before merge. Everything else is solid.

[svarlet]

Few changes suggested by my code reviewer