Skip to content

fix: HAR captures 0 entries — dangling HashMap keys cause new CDP client per request - #122

Closed
lekt9 wants to merge 1 commit into
justrach:mainfrom
lekt9:fix/cdp-client-key-ownership
Closed

fix: HAR captures 0 entries — dangling HashMap keys cause new CDP client per request#122
lekt9 wants to merge 1 commit into
justrach:mainfrom
lekt9:fix/cdp-client-key-ownership

Conversation

@lekt9

@lekt9 lekt9 commented Mar 29, 2026

Copy link
Copy Markdown

Summary

HAR recorder always returned 0 entries because every HTTP request created a new CDP client instead of reusing the existing one. Network.enable was sent on connection #1, but navigate and harStop used different connections that never received network events.

Root cause

getCdpClient() and getHarRecorder() in bridge.zig stored tab_id slices directly as StringHashMap keys. These slices point into per-request arena memory that is freed after each HTTP handler returns. On the next request, the new tab_id slice has the same content but a different memory address, and since the old key's memory was freed, HashMap.get() reads garbage and never matches — so it creates a new client every time.

Verified with debug logging:

# Before fix:
getCdpClient: creating NEW client for tab ABC...
getCdpClient: creating NEW client for tab ABC...  # different WS connection!
getCdpClient: creating NEW client for tab ABC...  # yet another!

# After fix:
getCdpClient: creating NEW client for tab ABC...
getCdpClient: reusing existing client for tab ABC...  ✓
getCdpClient: reusing existing client for tab ABC...  ✓

Changes

  • bridge.zig: Dupe tab_id keys with allocator.dupe() before HashMap.put() in both getCdpClient() and getHarRecorder()
  • client.zig: Replace fixed 32-slot EventBuffer with dynamic ArrayListUnmanaged so events aren't silently dropped. Add drainWsEvents() for mutex-safe WS reading.
  • router.zig: Use drainWsEvents() instead of direct WS access. Fix flushEventsToHar() to clear buffer without double-freeing arena-allocated events.
  • har.zig: Send Network.disable before setting recording=false so late events are still processed.

Results

  • httpbin.org: 0 → 2 HAR entries
  • nusmods.com: 0 → 19 HAR entries
  • No crashes (previously crashed with Invalid free in EventBuffer)

Closes #118, closes #120

Root cause: getCdpClient() and getHarRecorder() stored dangling pointer
keys in HashMaps, creating a new CDP client per request. Network.enable
was sent on connection #1 but navigate/harStop used connection #3.

Also adds Network.getResponseBody support — after HAR recording stops,
kuri fetches response bodies for JSON/text API responses via CDP and
includes them in the HAR output as content.text.

Fixes:
- Dupe tab_id keys before HashMap.put in getCdpClient/getHarRecorder
- Replace fixed 32-slot EventBuffer with dynamic ArrayListUnmanaged
- Add drainWsEvents() for mutex-safe WS event reading
- Fetch response bodies via Network.getResponseBody before Network.disable
- Fix toJson brace count and JSON escaping for response bodies

Result: httpbin.org → 2 entries with full response bodies (was 0).

Closes justrach#118, closes justrach#120

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@justrach justrach left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The runtime fix looks directionally right, but this patch introduces ownership regressions that should be fixed before merge.

  1. flushEventsToHar() now clears client.event_buf without freeing the buffered event payloads. Those payloads can be allocated from the bridge allocator during Network.enable / Network.disable, so this leaks event strings across HAR sessions.
  2. The new duplicated tab_id keys inserted into cdp_clients and har_recorders are not freed in Bridge.deinit() or removeTab(), so each tab now leaks one key per map.

I opened #123 with the same cross-request HAR fix, plus the ownership cleanup and validation against the original httpbin.org/get repro.

@justrach

Copy link
Copy Markdown
Owner

Closing — the core fixes here are already on main:

  • Bridge allocator.dupe(tab_id) for HashMap keys ✅
  • Dynamic ArrayListUnmanaged EventBuffer ✅
  • drainWsEvents() helper ✅
  • HAR entry ownership/cleanup ✅

The remaining response-body capture is handled more cleanly in PR #127, which will be merged separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants