Skip to content

Harden the loopback HTTP concurrency smoke against transient connection resets #835

Description

@dgenio

Problem

During the 2026-08-10 dependency-policy PR validation, the exact unchanged ContextWeaver head had one transient failure in tests/test_sidecar_http.py::test_concurrent_requests_stay_correct: 3,556 tests passed, but one of the 20 loopback requests failed with ConnectionResetError. Re-running the same commit passed completely.

The test starts a real stdlib ThreadingHTTPServer on an ephemeral loopback port, launches 20 client threads simultaneously, and records only successful HTTP statuses. A client-level transport exception escapes the worker, so the final assertion sees fewer than 20 results and the entire otherwise-green CI matrix requires a rerun.

The test is valuable: it protects the sidecar's real concurrent HTTP transport and must not be weakened into “ignore failures until green”. The objective is to distinguish a real server correctness regression from a transient loopback/socket scheduling artifact and make any failure maximally attributable.

Goal

Keep a real concurrent transport test while eliminating unnecessary whole-PR reruns caused by unclassified transient client/server startup or connection-reset behavior.

Investigation first

  • Reproduce the test repeatedly and under CI-like CPU contention.
  • Capture whether failures happen during connect, request write, response read, or server shutdown.
  • Check the server/handler for connection-close, request-body drain, backlog, daemon-thread and startup-readiness semantics.
  • Do not add retries until the failure mode is understood; a retry that masks a deterministic server bug would weaken the test.

Candidate fixes

Prefer the smallest evidence-backed mechanism, for example:

  • an explicit server-ready barrier before starting client workers;
  • collecting client exceptions in the result set and asserting on their type/count with useful diagnostics instead of losing the worker result;
  • tuning server shutdown/thread lifecycle if the reset is caused by fixture teardown;
  • a very narrow retry only for a demonstrated OS/socket transient, while still failing if the second attempt or any semantic response is wrong;
  • reducing artificial simultaneous-connect burst only if the intended contract is ordinary concurrent requests rather than a connection-storm stress test.

If 20 simultaneous fresh TCP connections is intentionally the load contract, keep that strength and fix the server/test synchronization instead.

Acceptance criteria

  • A failure reports every worker outcome/exception rather than only len(results) < 20.
  • The test remains a real loopback HTTP concurrency test using the production sidecar server.
  • No broad try/except: pass, blanket flaky marker, or unconditional retry hides real server defects.
  • The test passes repeatedly under a stress/repetition run and in normal CI.
  • The chosen concurrency level and what it proves are documented in the test.
  • Any transport retry is limited to a demonstrated transient class and separately tested.

Related friction

ContextWeaver's primary CI already does the right thing by cancelling superseded PR runs, using quiet pytest output and measuring coverage on one canonical cell. This issue is therefore intentionally about the one observed flaky test rather than a broad CI rewrite.

Non-goals

  • Making the sidecar smoke non-gating merely because it once flaked.
  • Removing concurrent HTTP coverage.
  • Replacing the stdlib HTTP implementation as part of a test-only fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions