Repository navigation
sdk/python: send the first frames with the agent upgrade request over plain HTTP - #324
Conversation
… plain HTTP A relay dial waited for the 101 before writing its first frame, one extra round trip on every dial: the proto probe, every call after the idle window, every call with keep-alive off. sandboxd replays bytes buffered behind the request, so over plain HTTP the request now leaves with the first frame and the 101 is read on the first recv, under the dial timeout or the caller's tighter socket timeout. HTTPS keeps the old order, since a TLS edge may be a proxy that drops early bytes. A connection that closes before the upgrade reply is now a ProtocolError, so a run deadline that cuts a pending upgrade surfaces as SandboxTimeout. The relay fakes set TCP_NODELAY as sandboxd does; without it Nagle held a reply behind the 101 and the fake's reset dropped it.
|
Review note (pre-101 sends with no socket timeout): checked, no change.
|
What
This is the Python SDK counterpart of #322. Over plain HTTP,
dial_agentused to write the upgrade request and wait for the 101 before the first frame went out. It now returns aConnthat holds the request. The firstsendwrites the request and the first frame in onesendall, and the firstrecvparses the 101.HTTPS keeps the old order, because a TLS edge may be a proxy that drops bytes sent ahead of the 101 (#322 has the probe: Go's
httputil.ReverseProxydrops them, Caddy forwards them).Details:
_read_upgrade), moved out of the old inline loop.min(caller socket timeout, client.timeout). With no per-call deadline, that is the sameclient.timeoutbudget the old in-dial read had. Arundeadline still cuts a pending upgrade through its watchdog.ProtocolErrorinstead ofAPIError(0, ""), matching Go. This change lets arundeadline that cuts a pending upgrade surface asSandboxTimeout.TCP_NODELAY, as sandboxd's Go sockets do. Without it, Nagle held a reply behind the 101 and the fake's reset close dropped it. One existing test failed 9 of 30 runs on Linux without it, and passed 100 of 100 with it.Not ported: Go's pipelined proto probe. Python's first kept call still sends
info, waits for its reply, and then sends the request. With this PR the upgrade andinfoleave together, so that first call takes 2 round trips instead of 3 (Go's takes 1). Porting the probe means_connecthas to carry the request through the old-daemon redial, which touchesrun,_lease,_open_stream, and about 15 call sites. That is left for a follow-up.Known edge, accepted: while the 101 is being read, the socket timeout is temporarily
min(caller, client.timeout). Arun(stdin=…)pump thread whosesendallstarts inside that window inherits the bound. To fail, the guest must stop reading stdin for longer thanclient.timeoutat exactly that point. I judged that contrived and left it.Numbers
Bare-metal test host, real sandboxd, plain HTTP. Both SDK copies went through a byte-level TCP forwarder that adds a fixed 5 ms one-way delay (10 ms RTT). Runs were interleaved old/new/new/old/old/new, n=100 each,
stat("/")p50:keep_alive=0(every call dials)On loopback, without the forwarder, the two are within noise (p50 313–356 µs for both), because one loopback round trip is tens of µs.
Acceptance against real sandboxd (plain HTTP)
Old and new behave identically:
run(["sh","-c","wc -c"], stdin=8 MiB)→ exit 0,8388608.exec("echo","hi")→hi.APIError agent upgrade: {"error":"unknown sandbox"} (HTTP 404).Tests and gates
tests/test_upgrade.py: the fake server reads the request, waits for the frame, and only then sends the 101. There are two cases, keep-alive off (fs_mkdir) and the probe (info), plus a rejected-upgrade case and a test that a run deadline cuts an unanswered upgrade. On main, 3 of these 4 fail; with this PR, 4 of 4 pass.ruff format --check,ruff check,mypyandpytestall pass: 228 passed on macOS py3.12, on the py3.9 floor, and in a linux/arm64 container.TestCaddyTLSCluster/Python(HTTPS path) and the whole e2e package pass.After both this PR and #322 are merged, the
docs/sandboxd-api.mdsentence that #322 adds should say "the Go and Python SDKs".