tst-udp: fire-and-forget unconnected sender + IPv4-first hostname resolution - #147
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens UDP sending semantics and hostname resolution to match real-world TS-over-UDP sender/receiver behavior, and adds a portable helper for classifying connection-refused errors in tst-core.
Changes:
- Switch
UdpTransportfrom a connected UDP socket (send) to an unconnected socket usingsend_to(peer)to avoid LinuxECONNREFUSEDsurfacing as a fatal transport break. - Update
udp://hostname resolution to prefer IPv4 among “probe-clean” candidates to avoid dual-stacklocalhostchoosing::1against IPv4-only listeners. - Add
TransportError::is_connection_refused()(std-gated) plus new/updated regression tests and changelog/public-api baselines.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/coverage/surface-manifest.toml | Adds surface coverage entry for the new TransportError::is_connection_refused API. |
| crates/tst-udp/tests/pipeline_round_trip.rs | Updates test commentary to reflect unconnected UDP send behavior. |
| crates/tst-udp/tests/loopback_unicast.rs | Adjusts hostname loopback test binding logic; adds regression test for “absent peer never errors”. |
| crates/tst-udp/src/url.rs | Implements IPv4-first selection among probe-clean resolved addresses; adds resolution regression test. |
| crates/tst-udp/src/transport.rs | Changes sender from connected UDP to unconnected send_to with stored peer address. |
| crates/tst-core/src/transport.rs | Adds std-gated TransportError::is_connection_refused() plus a unit test. |
| crates/tst-core/public-api.txt | Updates public API baseline for the new method. |
| CHANGELOG.md | Documents the new helper and the UDP behavioral changes. |
Suppressed comments (1)
crates/tst-core/src/transport.rs:215
- This test hard-codes
ECONNREFUSEDvalues for only a few target OSes. On other platforms (e.g. netbsd/dragonfly or any non-matchingtarget_os),refusedwill be undefined and the test won’t compile. Preferlibc::ECONNREFUSEDfor all Unix targets and keep the Windows constant for Windows.
#[cfg(target_os = "linux")]
let refused: i32 = 111; // ECONNREFUSED
#[cfg(any(target_os = "macos", target_os = "freebsd", target_os = "openbsd"))]
let refused: i32 = 61;
#[cfg(windows)]
let refused: i32 = 10061; // WSAECONNREFUSED
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+204
to
+206
| #[cfg(test)] | ||
| mod error_tests { | ||
| use super::*; |
Owner
Author
There was a problem hiding this comment.
Fixed in b31087b — error_tests is now #[cfg(all(test, feature = "std"))]. Note for posterity: the in-workspace repro passes either way (a dev-dep re-enables tst-core/std via test-kind feature unification), so the genuine RED was demonstrated on the extracted cargo package tree, which is what a real downstream no-default-features consumer sees.
Comment on lines
+365
to
+372
| fn hostname_resolution_prefers_ipv4() { | ||
| let u = UdpUrl::parse("udp://localhost:5004").unwrap(); | ||
| assert!( | ||
| u.addr.is_ipv4(), | ||
| "expected IPv4-first for localhost, got {}", | ||
| u.addr | ||
| ); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in b31087b — the test now resolves localhost first and skips (with an eprintln) when the resolver returns no IPv4 candidate; the IPv4-preference assertion is unchanged whenever an A record exists.
Comment on lines
+49
to
+54
| let ipv4 = ("localhost", 0u16) | ||
| .to_socket_addrs() | ||
| .expect("resolve localhost") | ||
| .next() | ||
| .expect("localhost resolved to no addresses"); | ||
| let recv = UdpSocket::bind(first).expect("bind recv"); | ||
| .find(|a| a.is_ipv4()) | ||
| .expect("localhost resolved no IPv4 address"); | ||
| let recv = UdpSocket::bind(ipv4).expect("bind recv"); |
Owner
Author
There was a problem hiding this comment.
Fixed in b31087b — the receiver bind now prefers an IPv4 candidate and falls back to the first resolved address of any family on IPv6-only hosts, mirroring resolve_host's prefer-not-require contract.
An integrator field report, reproduced against a live media server: a receiver restarting or idling mid-stream sends back ICMP port-unreachable, which a connected UDP socket surfaces on Linux as a fatal ECONNREFUSED on the sender's next send. UdpTransport now sends via send_to on an unconnected socket instead -- fire-and-forget datagram semantics matching every other TS-over-UDP sender (ffmpeg, VLC, mediamtx). No knob; this is the only mode. Residual from the same report: a UDP connect-probe used to pick a resolved hostname candidate can reject an unconfigured/unroutable address family but can't detect an absent listener, so `localhost` resolving [::1, 127.0.0.1] on a dual-stack host picked whichever family the resolver listed first and could die against an IPv4-only listener. resolve_host now prefers IPv4 among probe-clean candidates. Also adds TransportError::is_connection_refused() (std-gated) so callers can classify a refused connection without hard-coding the platform errno split.
…t tolerance TransportError's error_tests module was #[cfg(test)] only, so a truly standalone no_std build of tst-core (no dev-dependency unifying std back in) would fail to compile it against the std-gated is_connection_refused method it exercises -- gate the module on feature = "std" too. hostname_resolution_prefers_ipv4 and the loopback_unicast hostname test both assumed an IPv4 localhost record exists. Neither requirement holds on an IPv6-only host: the unit test now skips (with an eprintln) when there's no IPv4 candidate to prefer, and the loopback test falls back to the first resolved candidate, mirroring resolve_host's own prefer-not-require contract.
aklofas
force-pushed
the
fix/udp-fire-and-forget-send
branch
from
August 6, 2026 10:07
b31087b to
120add8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (live-reproduced in an integrator field report)
UdpTransportused a connected UDP socket. On Linux, a connected socket surfaces ICMP port-unreachable asECONNREFUSEDon a latersend, which the send path classified as fatalTransportError::Broken. Real receivers (e.g. a media server that unbinds its idle ingest socket periodically, or simply restarts) therefore killed long-running senders — a transient condition became a dead transport. Our ownpipeline_round_triptest had a workaround comment for exactly this behavior.Fix
send_to(peer)— fire-and-forget datagram semantics, matching what every TS-over-UDP sender (ffmpeg, VLC, mediamtx) does. The failure class is deleted structurally; the struct rustdoc documents why this is deliberate. No knob: connected mode had no consumer.localhost → [::1, 127.0.0.1]against an IPv4-only listener previously picked::1and died.resolve_hostnow prefers the first probe-clean IPv4, then first probe-clean any-family, then first resolved; the preference is documented.TransportError::is_connection_refused()(std-gated, tst-core): portable refused-classification via theio::ErrorKindmapping, so consumers stop hard-coding the 111/61/10061 errno split.Tests
send_to_absent_peer_never_errors— red pre-fix (real ECONNREFUSED on send P7(c-2) S2: cross-compile libsrt for bare-metal arm-none-eabi + boot smoke #2), green post-fix.hostname_resolution_prefers_ipv4— genuinely red pre-fix on this host (resolver returns::1first), green post-fix. An adjacent hostname-loopback test was un-flaked for IPv6-first resolvers as a direct consequence.is_connection_refused_classifies_portably— per-OS errno constants through the helper; false forNone/non-errno variants.tst-core public-api baseline re-rendered for the one new method (+ surface-manifest coverage row); tst-udp surface unchanged. Full local rail battery green (3 feature modes, clippy, fmt, nightly doc, doc-tests, ratchet sweep, pinned public-api ×10, non_exhaustive 309, fuzz check, loopback binary stress ×5). No C ABI change.