Add Tcp.sendBytes — a byte-clean one-shot TCP call - #764
Conversation
`Tcp.send` decodes its response with `String::from_utf8_lossy`, so every non-UTF-8 sequence becomes U+FFFD. The substitution is many-to-one — 128 of the 256 single-byte values collapse to the same replacement character — so it cannot be undone, and it is silent: the call still returns `Result.Ok`, and `String.len` reports what an uncorrupted read would. That puts every binary protocol out of reach. Add `Tcp.sendBytes : (String, Int, List<Int>) -> Result<List<Int>, String>` alongside it. Same socket behaviour as `Tcp.send` — open, write, shutdown(Write), read to EOF, same cap, timeouts and port validation — but payload and response stay bytes end to end. Additive rather than a change in place: reshaping `Tcp.send` would break every existing caller, and erroring on invalid UTF-8 would regress the ones already living with lossy text. Byte values outside 0..=255 return a catchable `Result.Err` naming the value and its index, matching how the port range is handled rather than trapping. A non-`List<Int>` payload stays a static type error. Wired through the VM path, the type checker, and the effect classification table, so Oracle `verify <fn> trace` can stub it. The codegen backends are not wired yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed, and ran it locally — both new tests pass (including the integration one with --include-ignored), fmt is clean. This is a well-made first PR: the wiring is complete across the VM path, the additive sibling is the right call, and the docs/llms.txt steering toward sendBytes for binary work is appreciated.
Answers to your open questions:
List<Int>is right for now. Consistency with theBytenamespace wins at this stage; if parsing pressure showsVector<Int>should be the primary shape, that's a pre-1.0 revisit, not something to solve here.- VM-only is fine for this PR. I'd take the Rust codegen arm as a follow-up rather than grow this one. (Edited: I originally echoed the "wasip2 is host territory" line from the PR description here — that's stale, and the mistake is on me since it's my own code:
Tcp.*graduated on--target wasip2in 0.20 with an in-module wasi-sockets implementation, and plain--target wasm-gcroutesTcp.sendthrough anaver.tcp_sendhost import. So both wasm targets can and will carrysendBytes— I'm taking that as follow-up PRs alongside the Rust codegen arm.)
One fixup before I merge: in bytes_arg, an Int outside i64 falls into the to_i64() failure and reports "payload must be a List". Aver's Int is arbitrary-precision, so this is reachable from ordinary source, and it lands as a runtime trap with a wrong message:
[65, 256] → Result.Err("… byte 256 at index 1 is out of range (0–255)") — catchable, as designed
[65, 1208925819614629174706176] → Runtime error: Tcp.sendBytes: payload must be a List<Int> — trap, and the message is wrong
By your own (correct) design both belong in the catchable out-of-range Result.Err, index included — treating to_i64()'s None the same as the u8::try_from failure should do it.
The two things you hit while testing deserve their own issues — the Rust codegen reporting Compiled [Rust] and then failing at cargo build, and the wasm-gc List<Int> literal rejection. Would you file them with your repros? You found them, you should get the credit.
CI and Proof are already green here; once the fixup is in and everything stays green, I'll merge.
|
Update on the two side-findings: the wasm-gc |
|
Didn't want your first PR hanging on a 3-liner, so I pushed the fixup myself (346f3f7) — an |
Add
Tcp.sendBytes— a byte-clean one-shot TCP callFixes #763. Broader context, including the other three binary-protocol
blockers this does not address: #753.
The problem
aver-rt/src/tcp.rsendssendwith:Every non-UTF-8 sequence in the response becomes U+FFFD. The substitution is
many-to-one — 128 of the 256 possible single bytes collapse to the same
replacement character — so it cannot be undone. It is also silent: the call
returns
Result.Okwith mangled data, andString.lenreports the same lengthan uncorrupted read would, so neither the return shape nor a length check
reveals it.
That makes every binary protocol unreachable from Aver. Demonstrated against a
server that always replies with the four bytes
F9 BE B4 D9(the Bitcoinmainnet magic — chosen because each byte is invalid UTF-8 for a different
reason):
The change
Adds
Tcp.sendBytes : (String, Int, List<Int>) -> Result<List<Int>, String>.Identical socket behaviour to
Tcp.send— open, write,shutdown(Write), readto EOF, same 10 MB cap, same timeouts, same port validation — but the payload
and the response stay bytes end to end, with no encoding or decoding in either
direction.
Design decisions
Additive, not a fix in place. Changing
Tcp.send's return type would breakevery existing caller, and making it error on invalid UTF-8 would regress
callers who currently tolerate lossy text. A sibling method leaves existing code
untouched and lets the docs steer new code to the right one.
List<Int>as the byte carrier. Follows the existingBytenamespaceconvention, which already operates on
Intrather than introducing a byteprimitive. Worth a maintainer opinion:
Vector<Int>would give O(1) indexedaccess, which matters more when parsing a length-prefixed header than when
sending one. I went with
List<Int>for consistency withArgs.getandDisk.listDir, and becauseVector.fromListmakes the conversion cheap at theone place it matters.
Out-of-range bytes are catchable, not fatal. A value outside
0..=255returns
Result.Err("Tcp.sendBytes: byte 256 at index 1 is out of range (0–255)")rather than trapping — the same treatment the port range gets afterPhase 4.7+ fix #13. The index is included so a long payload is debuggable. A
non-
List<Int>argument is still a type error, caught statically.What's wired
aver-rt::tcp::send_bytes— the runtime callregister/register_nv,DECLARED_EFFECTS,effects, and theVmBuiltinenumGenerativeOutput, with two newRuntimeTypevariants (
ListInt,ResultListIntStr). Oracleverify <fn> traceworkswith a
given tcp: Tcp.sendBytes = [stub]binding — confirmed against.result,.trace.length()and.trace.contains()What's not wired — deliberately, and I'd like direction
Codegen backends.
Tcp.sendBytesis VM-only. I stopped there rather thanguess at three backends in a first PR:
Compiled ... [Rust], then the generatedproject fails at
cargo buildwith "MIR walker could not render themainfnbody". Not a silent miscompile, but the error arrives one stage too late. The
binding itself looks like a short arm next to the existing
Tcp.sendone; theVec<u8>↔AverListconversion is the only real work.List<Int>literal,with or without this change (
List literal: cannot resolve list instantiation (got List<Int>)). I verified that against the released 0.27.1 binary too, soit's pre-existing and orthogonal.
Tcp.*as host territory, so nothing to do.Happy to extend to any of these if you'd point me at the shape you'd want.
CHANGELOG. Left alone on purpose —
AGENTS.mdis explicit thattools/release.pyowns version headers, and adding an entry means guessing thenext version number.
cargo fmt --all -- --checkpasses clean (rustfmt 1.9.0) with no diff.Tests
Two added to
tests/eval_spec.rs, alongside the existingtcp_tests:tcp_send_bytes_round_trips_non_utf8— echo server, asserts[249, 190, 180, 217]survives intact. Marked#[ignore]like itstcp_send_echo_serverneighbour since it binds a socket.tcp_send_bytes_rejects_out_of_range_byte— asserts the error names both theoffending value and its index.
Manually verified: empty payload →
Result.Ok([]); connection refused,out-of-range port, and a
Stringpassed whereList<Int>is expected allproduce the right diagnostics.