Skip to content

fix(benchpack-host): close provider connections on request timeout to stop request accumulation - #29

Open
danieltvela wants to merge 1 commit into
stevibe:mainfrom
danieltvela:fix-timeout-close-connection
Open

fix(benchpack-host): close provider connections on request timeout to stop request accumulation#29
danieltvela wants to merge 1 commit into
stevibe:mainfrom
danieltvela:fix-timeout-close-connection

Conversation

@danieltvela

Copy link
Copy Markdown

Summary

Fixes a resource leak in @benchlocal/benchpack-host where a scenario request that hits the request timeout is not actually torn down. The host used to fire the abort signal and immediately move on to the next test, leaving the timed-out provider request running in the background. Because provider traffic flows through a shared undici Agent, the provider process never sees the connection close and keeps generating — so across scenarios it accumulates in-flight requests that appear to loop forever.

This PR closes that gap in two complementary layers:

  1. Graceful drain before advancing — after a timeout, the host now awaits the timed-out runScenario promise with a short, bounded grace period so the cooperative AbortSignal has a chance to unwind the request before the next test starts.
  2. Transport-level connection close — provider fetches are now routed through a dedicated undici Agent per provider origin, and on timeout the host force-closes that provider's connections (agent.destroy()) so the provider process cancels any still-running generation. The next request to the same provider transparently reopens a fresh connection ("close and reopen").

The force-close is gated by a per-provider in-flight counter so it only happens when the timed-out scenario is the sole active request to that provider — concurrent requests in the parallel execution modes are never disrupted.

Problem

Timeout handling in runScenarioSafely

The scenario timeout in packages/benchpack-host/src/index.ts was implemented with Promise.race between the real run and a timer:

const result = await Promise.race([
  runPromise, // prepared.runScenario(...)
  new Promise((_resolve, reject) => {
    timeout = setTimeout(() => {
      timedOut = true;
      scenarioController?.abort(error);
      reject(error);
    }, timeoutMs);
  })
]);

On timeout this:

  • sets timedOut = true,
  • aborts the cooperative scenarioController,
  • rejects the race, and
  • returns a failure ScenarioResult.

However, Promise.race does not cancel or await the losing promise. The actual prepared.runScenario(...) keeps running in the background. The finally block only cleared the timer and removed the parent-abort listener; nothing awaited runPromise.

The orchestration loops (executeSerialByModelMode, executeSerialTestCasesMode, and the parallel variants) advance to the next scenario/model as soon as runScenarioWithRepeats resolves — there was no drain or wait for the previous request to settle.

Why the provider connection stays open

  • The only cancellation channel was a cooperative AbortSignal passed into prepared.runScenario. Whether the underlying provider fetch actually aborts depends entirely on the Bench Pack honoring that signal; the host never forced the connection shut.
  • All provider traffic went through a single global undici Agent installed at module load (setGlobalDispatcher(new Agent({ bodyTimeout: 0, headersTimeout: 0 }))). The globalThis.fetch wrapper used for provider-error capture passed init through unchanged — it did not manage or scope connections.
  • For inference through the local relay, the upstream fetch received no signal, and the pipeline to the client did not propagate a client disconnect back to the upstream — so even a well-behaved client abort could leave the provider generation running.

Result: on timeout the provider kept generating/streaming to an open connection while the next test piled on top, matching the "accumulating infinite-loop requests" symptom.

Fix

Part A — drain the timed-out request

In runScenarioSafely, runPromise is hoisted to function scope. When the timeout fires, before returning the failure result the host now awaits it with a bounded grace period:

if (timedOut) {
  // Give the cooperative abort a bounded chance to unwind the underlying
  // request before returning, so the next test does not start while the
  // timed-out request is still running against the provider.
  await drainRunPromise(runPromise);
  ...
}

drainRunPromise races the promise against a 2 s grace timer (TIMED_OUT_RUN_DRAIN_GRACE_MS), so a well-behaved Bench Pack that honors the abort signal settles quickly, and a misbehaving one cannot block the run forever.

Part B — per-provider connection close and reopen

  1. Per-provider undici Agents. The single anonymous global Agent was replaced by:

    • a default global Agent (still { bodyTimeout: 0, headersTimeout: 0 }) for non-provider fetches, and
    • a Map<normalizedBaseUrl, Agent> (providerFetchAgents) that lazily creates a dedicated Agent per provider origin.
  2. Routing provider fetches through the provider's Agent. The globalThis.fetch wrapper (installProviderFetchCapture) now resolves the provider origin for each request URL and injects that provider's Agent via undici's dispatcher option before calling the native fetch. This covers both direct Bench Pack fetches and the inference relay's upstream fetches.

  3. Force-close on timeout. closeProviderFetchAgent(baseUrl) looks up the provider's Agent, removes it from the map, and calls agent.destroy(). This closes the TCP connections to that provider, so the provider process sees the connection close and cancels generation. Because the entry is removed from the map, the next request to the same provider lazily creates a fresh Agent — the "close and reopen" behavior.

Safety gate for parallel execution

A per-provider-origin in-flight counter (providerActiveScenarioCounts) tracks how many scenarios are currently using each provider (beginProviderScenario / endProviderScenario in runScenarioSafely). The force-close only runs when the timed-out scenario is the sole active request to that provider:

if (isSoleActiveProviderScenario(providerKey) && input.providerBaseUrl) {
  closeProviderFetchAgent(input.providerBaseUrl);
}

This means:

  • serial_by_model (one request at a time): force-close applies — this is where the accumulation was observed.
  • parallel_by_test_case, parallel_by_model, full_parallel, or multiple models sharing the same provider in serial: force-close is skipped, so healthy concurrent requests to the same provider are never reset.

Files changed

  • packages/benchpack-host/src/index.ts — all changes. No protocol, SDK, or public API changes.

Testing

  • npm run typecheck (all workspaces) — passes.
  • npm test (typecheck + UI contract check) — passes.

Notes:

  • A pre-existing baseline tsc failure in @benchlocal/benchpack-host exists at HEAD when the @benchlocal/core / @benchlocal/sdk dist/ builds are missing (the workspace typecheck depends on those packages being built). It is unrelated to this change; npm run build:compile builds the packages first.
  • Manual verification recommended: configure a provider that hangs (or set a small request_timeout_seconds), run a serial_by_model run, and confirm the provider cancels the in-flight generation on timeout and no requests accumulate across scenarios; then confirm a full_parallel run with concurrent requests is unaffected.

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