From 42537decef3fdcfe9bec85664283f6714dace171 Mon Sep 17 00:00:00 2001 From: Aron Bakes Date: Sun, 30 Aug 2026 13:33:50 +1000 Subject: [PATCH] Fall back to a second host, then a TCP handshake, where the internet probe fails omarchy-network-status measures the internet hop with a single ICMP echo to one hardcoded host. Anything that filters either the host or the protocol -- an ISP that blackholes 1.1.1.1, a university or corporate WLAN with a default-deny egress ACL, a captive-portal appliance, CGNAT -- makes every sample come back empty, so the panel reads Timeout and climbs to 100% packet loss on a link that is working fine. Probe 1.1.1.1 and 8.8.8.8 together and take the first reply in list order, so a blocked host costs no time and nothing changes for a network where 1.1.1.1 answers. Only when no echo returns from either, time a TCP handshake to each in turn and report it as internet_tcp_ms. It crosses the network the same single round trip, so the number is comparable, and the panel labels the row Ping (TCP) rather than passing a handshake off as an echo reply. internet_ping_ms keeps meaning "an echo returned in this long", because that is what the packet loss row counts: a sample the handshake rescued is still a lost echo, and folding the two together would report a link shedding half its packets as healthy. Where no echo returns all window but the handshake keeps landing, loss is not measurable from here, so the row holds at -- instead of claiming 0%. Uses bash's /dev/tcp under a timeout rather than curl, which no script in bin/ currently needs. Fixes #9068 Fixes #9261 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01AFuwaBrZB6VbYVHy7oUU64 --- bin/omarchy-network-status | 49 ++++++++-- shell/plugins/panels/network/Model.js | 23 ++++- shell/plugins/panels/network/Panel.qml | 11 ++- test/shell.d/network-test.sh | 126 ++++++++++++++++++++++++- 4 files changed, 190 insertions(+), 19 deletions(-) diff --git a/bin/omarchy-network-status b/bin/omarchy-network-status index e97c6a29bd5..aa9d32dda1a 100755 --- a/bin/omarchy-network-status +++ b/bin/omarchy-network-status @@ -6,6 +6,8 @@ verbose=false internet_probe=1.1.1.1 +internet_probes="$internet_probe 8.8.8.8" +internet_probe_port=443 case "${1:-}" in "") @@ -54,31 +56,58 @@ ping_latency_ms() { LC_ALL=C ping -n -c 1 -W 1 "$host" 2>/dev/null | awk -F'time[=<]' '/time[=<]/ { split($2, parts, " "); print parts[1]; exit }' } +# Where ICMP is filtered, time a TCP handshake instead: one round trip, like an echo. +tcp_latency_ms() { + local host=$1 port=$2 + + LC_ALL=C timeout 2 bash -c ' + start=${EPOCHREALTIME/./} + exec 3<>"/dev/tcp/$1/$2" || exit 1 + finish=${EPOCHREALTIME/./} + exec 3>&- 3<&- + elapsed=$((finish - start)) + printf "%d.%03d\n" $((elapsed / 1000)) $((elapsed % 1000)) + ' _ "$host" "$port" 2>/dev/null +} + print_ping_samples() { local gateway=$1 - local tmpdir router_file internet_file - local router_pid="" internet_pid="" + local tmpdir probe echo_ms="" handshake_ms="" + local router_pid="" internet_pids=() omarchy-cmd-present ping || return tmpdir=$(mktemp -d) || return - router_file="$tmpdir/router" - internet_file="$tmpdir/internet" if [[ -n $gateway ]]; then - ping_latency_ms "$gateway" >"$router_file" & + ping_latency_ms "$gateway" >"$tmpdir/router" & router_pid=$! fi - ping_latency_ms "$internet_probe" >"$internet_file" & - internet_pid=$! + # Probed together, so a host the network blocks costs no time; first reply in list order wins. + for probe in $internet_probes; do + ping_latency_ms "$probe" >"$tmpdir/$probe" & + internet_pids+=($!) + done if [[ -n $router_pid ]]; then wait "$router_pid" - printf 'router_ping_ms\t%s\n' "$(cat "$router_file")" + printf 'router_ping_ms\t%s\n' "$(cat "$tmpdir/router")" + fi + + wait "${internet_pids[@]}" + for probe in $internet_probes; do + [[ -s $tmpdir/$probe ]] && { echo_ms=$(cat "$tmpdir/$probe"); break; } + done + printf 'internet_ping_ms\t%s\n' "$echo_ms" + + # Separate field: internet_ping_ms stays echo-only because the loss row counts it. + if [[ -z $echo_ms ]]; then + for probe in $internet_probes; do + handshake_ms=$(tcp_latency_ms "$probe" "$internet_probe_port") && break + done + printf 'internet_tcp_ms\t%s\n' "$handshake_ms" fi - wait "$internet_pid" - printf 'internet_ping_ms\t%s\n' "$(cat "$internet_file")" rm -rf "$tmpdir" } diff --git a/shell/plugins/panels/network/Model.js b/shell/plugins/panels/network/Model.js index b4c84c68962..a99e245e40a 100644 --- a/shell/plugins/panels/network/Model.js +++ b/shell/plugins/panels/network/Model.js @@ -210,7 +210,9 @@ function formatPacketLoss(percent, hasSamples) { if (hasSamples === false) return "--" var value = parseInt(percent, 10) - if (!value || value < 0) return "0%" + // Negative means loss could not be measured: see pingLatencyState. + if (value < 0) return "--" + if (!value) return "0%" return value + "%" } @@ -223,17 +225,27 @@ function pingLatencyState(previous, next, limit, averageLimit) { var reset = iface === "" || iface !== (prev.pingIface || "") var routerSamples = reset ? [] : prev.routerPingSamples var internetSamples = reset ? [] : prev.internetPingSamples + var tcpSamples = reset ? [] : prev.internetTcpSamples routerSamples = sample.router_ping_ms === undefined ? [] : appendPingSample(routerSamples, sample.router_ping_ms, window) internetSamples = sample.internet_ping_ms === undefined ? [] : appendPingSample(internetSamples, sample.internet_ping_ms, window) + tcpSamples = sample.internet_ping_ms === undefined ? [] : appendPingSample(tcpSamples, sample.internet_tcp_ms, window) + + var icmpLatency = averagePingLatency(internetSamples, averageWindow) + var tcpLatency = averagePingLatency(tcpSamples, averageWindow) + + // No echo all window but the handshake lands: ICMP is filtered, not the link down. + var filtered = icmpLatency < 0 && tcpLatency >= 0 return { pingIface: iface, routerPingSamples: routerSamples, internetPingSamples: internetSamples, + internetTcpSamples: tcpSamples, + internetProbeMethod: filtered ? "tcp" : "icmp", routerPingLatency: averagePingLatency(routerSamples, averageWindow), - internetPingLatency: averagePingLatency(internetSamples, averageWindow), - internetPingPacketLoss: pingPacketLossPercent(internetSamples) + internetPingLatency: filtered ? tcpLatency : icmpLatency, + internetPingPacketLoss: filtered ? -1 : pingPacketLossPercent(internetSamples) } } @@ -250,6 +262,10 @@ function formatRate(bytesPerSec) { return formatBytes(bytesPerSec) + "/s" } +function formatPingLabel(method) { + return method === "tcp" ? "Ping (TCP)" : "Ping" +} + // `hasSamples` false means no probe has come back yet, which is different from // a probe that timed out. The rows stay mounted through that gap and read "--" // so the grid doesn't reflow a second after the panel opens. @@ -368,6 +384,7 @@ if (typeof module !== "undefined") { formatBytes: formatBytes, formatRate: formatRate, formatPingLatency: formatPingLatency, + formatPingLabel: formatPingLabel, wifiRow: wifiRow, sortWifiRows: sortWifiRows, wifiSectionTitle: wifiSectionTitle, diff --git a/shell/plugins/panels/network/Panel.qml b/shell/plugins/panels/network/Panel.qml index d1e41149e8d..04f8aff0ccb 100644 --- a/shell/plugins/panels/network/Panel.qml +++ b/shell/plugins/panels/network/Panel.qml @@ -42,8 +42,10 @@ Panel { property real downloadRate: 0 // bytes/sec property real uploadRate: 0 // bytes/sec property string pingIface: "" + property string internetProbeMethod: "" property var routerPingSamples: [] property var internetPingSamples: [] + property var internetTcpSamples: [] property real routerPingLatency: -1 property real internetPingLatency: -1 property int internetPingPacketLoss: 0 @@ -338,8 +340,10 @@ Panel { downloadRate = 0 uploadRate = 0 pingIface = "" + internetProbeMethod = "" routerPingSamples = [] internetPingSamples = [] + internetTcpSamples = [] routerPingLatency = -1 internetPingLatency = -1 internetPingPacketLoss = 0 @@ -542,12 +546,15 @@ Panel { var state = Model.pingLatencyState({ pingIface: pingIface, routerPingSamples: routerPingSamples, - internetPingSamples: internetPingSamples + internetPingSamples: internetPingSamples, + internetTcpSamples: internetTcpSamples }, next, pingHistoryWindow, pingAverageWindow) pingIface = state.pingIface + internetProbeMethod = state.internetProbeMethod routerPingSamples = state.routerPingSamples internetPingSamples = state.internetPingSamples + internetTcpSamples = state.internetTcpSamples routerPingLatency = state.routerPingLatency internetPingLatency = state.internetPingLatency internetPingPacketLoss = state.internetPingPacketLoss @@ -1231,7 +1238,7 @@ Panel { // opened, once the first probe returned, shoving everything below // them down. They now hold their place and read "--" until there is // a sample. - InfoLabel { text: "Ping" } + InfoLabel { text: Model.formatPingLabel(root.internetProbeMethod) } DetailValue { text: root.formatPingLatency(root.internetPingLatency) color: root.internetPingPacketLoss > 0 ? root.bar.urgent : root.bar.foreground diff --git a/test/shell.d/network-test.sh b/test/shell.d/network-test.sh index 11fdf9dda71..0d54775523f 100644 --- a/test/shell.d/network-test.sh +++ b/test/shell.d/network-test.sh @@ -132,29 +132,102 @@ let ping = network.pingLatencyState( ) assertDeepEqual( ping, - { pingIface: 'wlan0', routerPingSamples: [2], internetPingSamples: [20], routerPingLatency: 2, internetPingLatency: 20, internetPingPacketLoss: 0 }, + { pingIface: 'wlan0', routerPingSamples: [2], internetPingSamples: [20], internetTcpSamples: [null], internetProbeMethod: 'icmp', routerPingLatency: 2, internetPingLatency: 20, internetPingPacketLoss: 0 }, 'network seeds ping latency samples' ) ping = network.pingLatencyState(ping, { iface: 'wlan0', router_ping_ms: '4.0', internet_ping_ms: '' }, 4) assertDeepEqual( ping, - { pingIface: 'wlan0', routerPingSamples: [2, 4], internetPingSamples: [20, null], routerPingLatency: 3, internetPingLatency: 20, internetPingPacketLoss: 50 }, + { pingIface: 'wlan0', routerPingSamples: [2, 4], internetPingSamples: [20, null], internetTcpSamples: [null, null], internetProbeMethod: 'icmp', routerPingLatency: 3, internetPingLatency: 20, internetPingPacketLoss: 50 }, 'network averages recent successful ping samples' ) assertDeepEqual( network.pingLatencyState(ping, { iface: 'eth0', router_ping_ms: '1.5', internet_ping_ms: '10.0' }, 4), - { pingIface: 'eth0', routerPingSamples: [1.5], internetPingSamples: [10], routerPingLatency: 1.5, internetPingLatency: 10, internetPingPacketLoss: 0 }, + { pingIface: 'eth0', routerPingSamples: [1.5], internetPingSamples: [10], internetTcpSamples: [null], internetProbeMethod: 'icmp', routerPingLatency: 1.5, internetPingLatency: 10, internetPingPacketLoss: 0 }, 'network resets ping samples when interface changes' ) assertDeepEqual( network.pingLatencyState(ping, { iface: 'wlan0', internet_ping_ms: '22.0' }, 4), - { pingIface: 'wlan0', routerPingSamples: [], internetPingSamples: [20, null, 22], routerPingLatency: -1, internetPingLatency: 21, internetPingPacketLoss: 33 }, + { pingIface: 'wlan0', routerPingSamples: [], internetPingSamples: [20, null, 22], internetTcpSamples: [null, null, null], internetProbeMethod: 'icmp', routerPingLatency: -1, internetPingLatency: 21, internetPingPacketLoss: 33 }, 'network clears ping samples when a target is unavailable' ) +// Ping and Packet Loss answer different questions, and the TCP fallback only +// answers the first. One row per case the two rows have to tell apart. +function pingWindow(samples) { + let state = { pingIface: '', routerPingSamples: [], internetPingSamples: [], internetTcpSamples: [] } + for (const s of samples) state = network.pingLatencyState(state, Object.assign({ iface: 'wlan0' }, s), 24, 5) + return { + loss: network.formatPacketLoss(state.internetPingPacketLoss, true), + latency: network.formatPingLatency(state.internetPingLatency, true), + label: network.formatPingLabel(state.internetProbeMethod) + } +} + +const echo = { internet_ping_ms: '20.0' } +const rescued = { internet_ping_ms: '', internet_tcp_ms: '10.0' } +const silent = { internet_ping_ms: '', internet_tcp_ms: '' } +const every = sample => Array.from({ length: 8 }, () => sample) +const alternating = (a, b) => Array.from({ length: 8 }, (_, i) => i % 2 ? a : b) + +;[ + [every(silent), { loss: '100%', latency: 'Timeout', label: 'Ping' }, + 'reports a dead link as total loss'], + [alternating(echo, rescued), { loss: '50%', latency: '20 ms', label: 'Ping' }, + 'still counts an echo the handshake rescued as lost'], + [every(rescued), { loss: '--', latency: '10 ms', label: 'Ping (TCP)' }, + 'holds the loss row where ICMP is filtered rather than inventing a figure'], + [every(echo), { loss: '0%', latency: '20 ms', label: 'Ping' }, + 'leaves a healthy link unchanged'], + [alternating(echo, { internet_ping_ms: '' }), { loss: '50%', latency: '20 ms', label: 'Ping' }, + 'counts loss unchanged when the status script sends no handshake time'] +].forEach(([samples, expected, description]) => assertDeepEqual(pingWindow(samples), expected, 'network ' + description)) + +// Varying handshake times prove the window is kept: the last 5 of +// [30,10,10,10,10,10,10,50] average 18 ms; last-sample-only would say 50. +;[ + [ + [ + { internet_ping_ms: '', internet_tcp_ms: '30.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '50.0' } + ], + { loss: '--', latency: '18 ms', label: 'Ping (TCP)' }, + 'averages TCP handshake times over a 5-sample window' + ], + [ + [ + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '10.0' }, + { internet_ping_ms: '', internet_tcp_ms: '' } + ], + { loss: '--', latency: '10 ms', label: 'Ping (TCP)' }, + 'does not flash Timeout after one dropped handshake on a filtered network' + ] +].forEach(([samples, expected, description]) => assertDeepEqual(pingWindow(samples), expected, 'network ' + description)) + +assert(/InfoLabel \{ text: Model\.formatPingLabel\(root\.internetProbeMethod\) \}/.test(panelSource), 'network panel labels the ping row with the probe that produced it') + +// The panel must pass internetTcpSamples back into pingLatencyState so the TCP +// window is retained across ticks, matching how ICMP samples are passed. +assert( + /pingLatencyState\(\{[\s\S]*?internetTcpSamples: internetTcpSamples[\s\S]*?\},/.test(panelSource), + 'network panel passes internetTcpSamples into pingLatencyState to retain the TCP window' +) + assertEqual(network.formatBytes(1536), '1.5 KB', 'network formats bytes') assertEqual(network.formatRate(1536), '1.5 KB/s', 'network formats rates') assertEqual(network.formatPingLatency('2.54'), '2.5 ms', 'network formats low ping with precision') @@ -294,3 +367,48 @@ assertDeepEqual( assertEqual(network.headerDetail({ type: 'wifi', freq: '5745' }), '', 'network keeps wifi band state out of the hero') assertEqual(network.headerDetail({ type: 'ethernet', speed: '100' }), '100mbit', 'network keeps ethernet speed in the hero') JS + +status="$ROOT/bin/omarchy-network-status" + +# Run print_ping_samples for real, with ping and the handshake stubbed. ECHO lists +# the hosts whose echo returns; TCP lists the hosts whose handshake completes. +stub_bin=$(mktemp -d) +cat >"$stub_bin/ping" <<'PING' +#!/bin/bash +host=${@: -1} +[[ " $ECHO " == *" $host "* ]] || exit 1 +echo "64 bytes from $host: icmp_seq=1 ttl=57 time=20.0 ms" +PING +printf '#!/bin/bash\nexit 0\n' >"$stub_bin/omarchy-cmd-present" +printf '#!/bin/bash\n' >"$stub_bin/ip" +chmod +x "$stub_bin"/* + +ping_samples() { + ECHO="$1" TCP="$2" PATH="$stub_bin:$PATH" bash -c ' + status=$1; set -- + source "$status" >/dev/null + tcp_latency_ms() { [[ " $TCP " == *" $1 "* ]] && echo 12.000; } + print_ping_samples 192.0.2.1 + ' _ "$status" | grep -v '^router_ping_ms' | tr '\n' '|' +} + +expect_samples() { + local actual + actual=$(ping_samples "$1" "$2") + [[ $actual == "$3" ]] || fail "$4: expected '$3', got '$actual'" + pass "omarchy-network-status $4" +} + +expect_samples "1.1.1.1 8.8.8.8" "1.1.1.1 8.8.8.8" $'internet_ping_ms\t20.0|' "reports the echo and no handshake on a healthy link" +expect_samples "8.8.8.8" "1.1.1.1 8.8.8.8" $'internet_ping_ms\t20.0|' "falls back to a second echo host when the first is blocked" +expect_samples "" "1.1.1.1 8.8.8.8" $'internet_ping_ms\t|internet_tcp_ms\t12.000|' "reports a handshake separately where ICMP is filtered" +expect_samples "" "8.8.8.8" $'internet_ping_ms\t|internet_tcp_ms\t12.000|' "tries the second host for the handshake too" +expect_samples "" "" $'internet_ping_ms\t|internet_tcp_ms\t|' "reports nothing rescued on a dead link" + +if ! grep -F 'timeout 2 bash -c' "$status" >/dev/null; then + fail "omarchy-network-status must bound the TCP probe so a blackholed port cannot stall the panel" +fi +if grep -F 'tcp_latency_ms "$gateway"' "$status" >/dev/null; then + fail "omarchy-network-status must keep the router row on ICMP" +fi +pass "omarchy-network-status bounds the handshake and keeps the router on ICMP"