Skip to content

ENG-821 - Exe transport errors stringify empty, and create_vm's read timeout has no headroom - #10

Open
druks-operator[bot] wants to merge 1 commit into
mainfrom
agent/ENG-821
Open

ENG-821 - Exe transport errors stringify empty, and create_vm's read timeout has no headroom#10
druks-operator[bot] wants to merge 1 commit into
mainfrom
agent/ENG-821

Conversation

@druks-operator

Copy link
Copy Markdown
Contributor

Linear ticket: ENG-821

Plan

Summary

Harden the existing exe.dev API boundary in one focused PR: ensure empty-string httpx failures remain diagnosable and give legitimately slow VM creation meaningful read-timeout headroom without weakening operator-configured budgets or affecting ordinary commands.

Implementation

  1. Update src/providers/exe/api.py at _request's httpx.RequestError translation so ExeResponseError always names the caught exception type and retains exception chaining. Follow the established empty-timeout precedent in src/diagnostics/checks.py; the ticket permits {exc!r} or the type name plus message.
  2. Add an internal per-call timeout path through _exec_dict and _request, used only by ExeAPI.create_vm. Define a VM-creation read-timeout floor that provides real headroom beyond the measured healthy ~21-second latency and the default 30-second general budget; leave the exact floor to the implementation.
  3. Derive the creation timeout from the configured self.timeout: raise only its read budget when below the floor, never shorten a configured read timeout already above the floor, and preserve its connect, write, and pool budgets.
  4. Apply the creation budget to the individual request. _get_client() caches one shared httpx.AsyncClient, so creation must not mutate client state or leak its timeout into later list, delete, diagnostics, restart, or HTTP-proxy commands.
  5. Extend src/providers/exe/tests/test_api.py with mocked client/transport regressions for a bare empty-string httpx.ReadTimeout; creation with a general timeout below the floor; creation with a configured timeout already above the floor; and an ordinary command after creation on the same API instance.
  6. Update the EXE_API_TIMEOUT row in docs/deploy.md to distinguish the operator-configured general timeout from the internal VM-creation read floor. Do not add another setting.

Scope and risk

The code change remains within src/providers/exe/api.py, its focused unit tests, and exe.dev deployment documentation. Provider/service interfaces and exe.dev HTTP contracts do not change.

The audit of sibling exe-provider catches is complete: they translate ExeCommandError subclasses whose response-derived text is populated, so they need no equivalent change. The identical blank-string transport wraps in src/providers/hetzner/api.py:149 and src/providers/exoscale/api.py:209 are explicitly out of scope and should be considered separately.

Do not retry create_vm: exe.dev may have accepted the command before the read timeout, so replay can provision a duplicate VM. Do not expand the timeout policy to restart_vm. Provisioning-failure classification and retry policy remain out of scope under ENG-822.

Acceptance criteria

AC1

Description: An exe.dev transport failure whose underlying httpx.RequestError has an empty string representation raises ExeResponseError whose message names the underlying exception type; the original exception remains chained.

Verification: Inspect _request and the focused bare-httpx.ReadTimeout regression test in src/providers/exe/tests/test_api.py.

AC2

Description: ExeAPI.create_vm applies a per-request read-timeout floor with genuine headroom beyond the measured healthy ~21-second VM-creation baseline and the default 30-second general timeout. The effective creation read timeout is never lower than the configured general read timeout; connect, write, and pool budgets remain unchanged.

Verification: Inspect the _exec_dict/_request timeout path and focused tests covering both a configured timeout below the creation floor and one already above it.

AC3

Description: The cached httpx.AsyncClient is not mutated by VM creation: after create_vm on an ExeAPI instance, an ordinary exe.dev command on that same instance still uses the configured general timeout.

Verification: Inspect the mocked client/transport regression that executes creation and an ordinary command on one API instance and compares their request timeout behavior.

AC4

Description: The exe.dev deployment documentation identifies EXE_API_TIMEOUT as the general API timeout and explains that VM creation has a longer read-timeout floor without introducing another deployment variable.

Verification: Inspect the exe.dev provider configuration table in docs/deploy.md.

Ruled out

  • Raise the global or shared-client timeout: list, delete, diagnostics, restart, and HTTP-proxy failures would all take longer to surface even though only VM creation has the measured slow path.
  • Use a fixed creation timeout as an unconditional override: deployments whose configured EXE_API_TIMEOUT already exceeds that value would silently receive a shorter VM-creation budget.
  • Mutate the cached httpx.AsyncClient timeout for creation: the enlarged budget would leak into subsequent ordinary commands on the same ExeAPI instance.
  • Add a dedicated VM-creation deployment setting: it creates overlapping operator configuration and documentation when an internal read-timeout floor preserves existing configured behavior.
  • Retry create_vm after a read timeout: exe.dev may already have accepted and executed the command, so replay can create duplicate VMs; retry classification belongs to ENG-822.
  • Expand this PR to the identical Hetzner and Exoscale transport wraps: those independent provider boundaries are outside ENG-821's targeted exe.dev scope and should be handled as follow-up work.
  • Extend the larger timeout to restart_vm or provisioning-failure classification: the ticket provides no measured restart slow path, while classification is independently tracked by ENG-822.

Acceptance Criteria

  • AC1: An exe.dev transport failure whose underlying httpx.RequestError has an empty string representation raises ExeResponseError whose message names the underlying exception type; the original exception remains chained.
    • Verification: Inspect _request and the focused bare-httpx.ReadTimeout regression test in src/providers/exe/tests/test_api.py.
  • AC2: ExeAPI.create_vm applies a per-request read-timeout floor with genuine headroom beyond the measured healthy ~21-second VM-creation baseline and the default 30-second general timeout. The effective creation read timeout is never lower than the configured general read timeout; connect, write, and pool budgets remain unchanged.
    • Verification: Inspect the _exec_dict/_request timeout path and focused tests covering both a configured timeout below the creation floor and one already above it.
  • AC3: The cached httpx.AsyncClient is not mutated by VM creation: after create_vm on an ExeAPI instance, an ordinary exe.dev command on that same instance still uses the configured general timeout.
    • Verification: Inspect the mocked client/transport regression that executes creation and an ordinary command on one API instance and compares their request timeout behavior.
  • AC4: The exe.dev deployment documentation identifies EXE_API_TIMEOUT as the general API timeout and explains that VM creation has a longer read-timeout floor without introducing another deployment variable.
    • Verification: Inspect the exe.dev provider configuration table in docs/deploy.md.

Wrap httpx.RequestError with {exc!r} so bare timeouts name their type instead
of stringifying blank, and apply a per-request 90s read-timeout floor to
create_vm without mutating the cached client or shortening a higher configured
timeout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@druks-reviewer druks-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verification

Verdict: pass — all 4 acceptance criteria are satisfied.

  • AC1 (transport error names exception type, chains cause) — pass. _request (src/providers/exe/api.py:263) wraps transport errors with f"exe.dev API transport failed: {exc!r}", which always names the exception type even when str(exc) is empty, and preserves chaining via from exc. Covered by test_request_names_exception_type_for_blank_transport_error (test_api.py:165-176), which fires a bare httpx.ReadTimeout("") and asserts both the message and __cause__.
  • AC2 (create_vm read-timeout floor with real headroom, never lowers a higher configured value, other budgets untouched) — pass. _creation_timeout() (api.py:104-121) raises only the read budget to a 90s floor when the configured read timeout is below it, otherwise passes the configured timeout through unchanged; connect/write/pool are always preserved. Both branches (below-floor and above-floor) are covered by focused tests with explicit assertions on all four timeout components.
  • AC3 (cached AsyncClient not mutated by creation) — pass. The raised timeout is passed as a per-request override to client.post(..., timeout=timeout), never touching self._client or self.timeout. test_ordinary_command_after_create_vm_uses_general_timeout proves a whoami call issued after create_vm on the same instance still uses the general 30s timeout.
  • AC4 (docs distinguish general timeout from creation floor, no new setting) — pass. docs/deploy.md's EXE_API_TIMEOUT row now explains the general/creation-floor relationship and explicitly states no separate creation setting exists.

Scope discipline holds: only docs/deploy.md, src/providers/exe/api.py, and its tests are touched. restart_vm is untouched (correctly stays on the general timeout), no retry logic was added around create_vm, and no new deployment settings were introduced. Sibling Hetzner/Exoscale transport wraps were correctly left out of scope per the plan.

CI (head_sha 07186de): Build Docker Image, Run Lint, Run Static Checks, Run Tests (postgres), Run Tests (sqlite), and Run API Tests (docker provider) all pass. Local reproduction of lint, pyright, and the full test_api.py suite (29/29) corroborates.

No blocking findings.

Code review

Advisory pass on maintainability — findings do not affect the verdict above.

The diff is small and clean: it threads an optional per-request httpx.Timeout through _exec_dict/_request without mutating the cached client, and the new tests exercise real request behavior (outgoing timeout values, non-mutation across consecutive calls) rather than internal call mechanics.

One low-severity nit, no follow-up ticket filed (low-only):

  • src/providers/exe/tests/test_api.py:181 — the test name test_create_vm_raises_read_timeout_floor_when_general_timeout_below reads as if it expects an exception ("raises"), but the test only asserts on outgoing timeout values (no pytest.raises). Consider a name like test_create_vm_elevates_read_timeout_to_floor_when_general_timeout_below to avoid the ambiguity.

@druks-operator
druks-operator Bot marked this pull request as ready for review August 18, 2026 06:20
@druks-operator
druks-operator Bot requested a review from czpython August 18, 2026 06:20
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.

0 participants