Skip to content

test: capture invariant hardening - #1029

Draft
eli-r-ph wants to merge 1 commit into
v1-capture-async-ai-lanefrom
v1-capture-invariant-tests
Draft

eli-r-ph wants to merge 1 commit into
v1-capture-async-ai-lanefrom
v1-capture-invariant-tests

Conversation

@eli-r-ph

@eli-r-ph eli-r-ph commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

💡 Motivation and Context

posthog-go and posthog-rs found a set of capture invariants the hard way: a data race on enqueue during close, a pass-through hook that changed the wire, a size guard that refused events the endpoint accepts. This PR checks the Python stack against each one, and adds tests only where no existing test fails on the regression.

Changes

Tests only. No library code changes. Each invariant, and where it is covered:

Invariant Coverage
A pass-through before_send leaves the wire unchanged (null option falls back to its legacy property, empty containers stay empty, nested maps survive) New test_capture_invariants.py, sync and async, both lanes
Enqueue racing shutdown, including the first AI event starting the lazy lane test_shutdown_waits_for_racing_enqueue_before_draining now runs on both lanes and asserts delivery and no live consumers. The async cross-thread admission test adds a capture_ai case that must start no AI workers
An AI event whose properties sit at the endpoint ceiling is sent Was covered for the queued sync lane. Now also covered for sync_mode, capture_ai_immediate and the async queued lane, plus an over-guard case
No silent loss: one aggregate line per failed batch, naming the lane's endpoint, with no response text The sync sync_mode and async tests now cover all four capture methods and assert the endpoint
Fork Already covered: TestLaneForkRebuild (both lanes, sync_mode)
Config rejects batch < event Not applicable: Python's batch target is a fixed 5 MiB soft limit that one larger event may exceed alone. The AI event cap can only be lowered from 8 MiB plus headroom, well under the request limit. capture_ai_max_event_bytes validation is already tested

💚 How did you test it?

Break-on-purpose, one mutation per invariant, each restored afterwards:

  • The hook path turns empty or falsy options into null, on the sync client and on the async client: both lane cases fail on each.
  • Lane admission checks _closed under the lock and puts outside it: both racing cases fail.
  • The AI guard loses its envelope headroom: four ceiling cases fail.
  • Async AI workers start before the shutdown recheck: the capture_ai admission case fails.
  • The loss line names the analytics endpoint for AI failures, separately on the sync path, the async queued path and the async immediate path: the matching AI case fails each time.

One mutation was not caught: _Lane.close() setting _closed without the lock. Shutdown still waits on that lock through wait_for_sync_sends, so the race does not occur, and I did not add a test for it.

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

If releasing new changes

  • Ran sampo add to generate a changeset file

Tests only; no changeset.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Written with Cursor (Claude Opus) under the direction of the assignee. Skills used: writing-tests, writing-pr-descriptions.

Agent calls worth review:

  • Most coverage extends existing tests with a lane parameter. The only new file holds the hook wire test, which has no existing neighbor and spans both clients.
  • The batch-versus-event config check is marked not applicable, not ported.

@eli-r-ph eli-r-ph self-assigned this Oct 7, 2026
@eli-r-ph
eli-r-ph force-pushed the v1-capture-async-ai-lane branch from d5eb0bc to ae647de Compare October 7, 2026 03:56
@eli-r-ph
eli-r-ph force-pushed the v1-capture-invariant-tests branch from 6650520 to 7f7757a Compare October 7, 2026 03:56
@eli-r-ph
eli-r-ph force-pushed the v1-capture-invariant-tests branch 4 times, most recently from 7f7757a to 44df57f Compare October 7, 2026 04:54

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant