ENG-821 - Exe transport errors stringify empty, and create_vm's read timeout has no headroom - #10
ENG-821 - Exe transport errors stringify empty, and create_vm's read timeout has no headroom#10druks-operator[bot] wants to merge 1 commit into
Conversation
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>
There was a problem hiding this comment.
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 withf"exe.dev API transport failed: {exc!r}", which always names the exception type even whenstr(exc)is empty, and preserves chaining viafrom exc. Covered bytest_request_names_exception_type_for_blank_transport_error(test_api.py:165-176), which fires a barehttpx.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 touchingself._clientorself.timeout.test_ordinary_command_after_create_vm_uses_general_timeoutproves awhoamicall issued aftercreate_vmon the same instance still uses the general 30s timeout. - AC4 (docs distinguish general timeout from creation floor, no new setting) — pass.
docs/deploy.md'sEXE_API_TIMEOUTrow 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 nametest_create_vm_raises_read_timeout_floor_when_general_timeout_belowreads as if it expects an exception ("raises"), but the test only asserts on outgoing timeout values (nopytest.raises). Consider a name liketest_create_vm_elevates_read_timeout_to_floor_when_general_timeout_belowto avoid the ambiguity.
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
src/providers/exe/api.pyat_request'shttpx.RequestErrortranslation soExeResponseErroralways names the caught exception type and retains exception chaining. Follow the established empty-timeout precedent insrc/diagnostics/checks.py; the ticket permits{exc!r}or the type name plus message._exec_dictand_request, used only byExeAPI.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.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._get_client()caches one sharedhttpx.AsyncClient, so creation must not mutate client state or leak its timeout into later list, delete, diagnostics, restart, or HTTP-proxy commands.src/providers/exe/tests/test_api.pywith mocked client/transport regressions for a bare empty-stringhttpx.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.EXE_API_TIMEOUTrow indocs/deploy.mdto 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
ExeCommandErrorsubclasses whose response-derived text is populated, so they need no equivalent change. The identical blank-string transport wraps insrc/providers/hetzner/api.py:149andsrc/providers/exoscale/api.py:209are 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 torestart_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.RequestErrorhas an empty string representation raisesExeResponseErrorwhose message names the underlying exception type; the original exception remains chained.Verification: Inspect
_requestand the focused bare-httpx.ReadTimeoutregression test insrc/providers/exe/tests/test_api.py.AC2
Description:
ExeAPI.create_vmapplies 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/_requesttimeout path and focused tests covering both a configured timeout below the creation floor and one already above it.AC3
Description: The cached
httpx.AsyncClientis not mutated by VM creation: aftercreate_vmon anExeAPIinstance, 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_TIMEOUTas 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
EXE_API_TIMEOUTalready exceeds that value would silently receive a shorter VM-creation budget.httpx.AsyncClienttimeout for creation: the enlarged budget would leak into subsequent ordinary commands on the sameExeAPIinstance.create_vmafter 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.restart_vmor provisioning-failure classification: the ticket provides no measured restart slow path, while classification is independently tracked by ENG-822.Acceptance Criteria
httpx.RequestErrorhas an empty string representation raisesExeResponseErrorwhose message names the underlying exception type; the original exception remains chained._requestand the focused bare-httpx.ReadTimeoutregression test insrc/providers/exe/tests/test_api.py.ExeAPI.create_vmapplies 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._exec_dict/_requesttimeout path and focused tests covering both a configured timeout below the creation floor and one already above it.httpx.AsyncClientis not mutated by VM creation: aftercreate_vmon anExeAPIinstance, an ordinary exe.dev command on that same instance still uses the configured general timeout.EXE_API_TIMEOUTas the general API timeout and explains that VM creation has a longer read-timeout floor without introducing another deployment variable.docs/deploy.md.