Fix UDP retry condition: use $received_data flag instead of count($servers)

[mcfnord]

Summary

• Replaces count($servers) <= 1 retry guard with an explicit $received_data boolean flag
• Retry now triggers only when zero bytes were received before the timeout — not based on how many servers are in the list

Problem with the count($servers) approach

The count($servers) <= 1 heuristic conflates two different things:

• For ?query and ?server requests, $servers is pre-populated with 1 entry (the target server), so <= 1 correctly means "no response data received yet."
• For ?directory requests, $servers starts at 0 and grows as CLM_SERVER_LIST packets arrive. A directory server that legitimately has only one registered server will receive its full response, hit the trailing timeout, and still satisfy count($servers) <= 1 — triggering an unnecessary retry and adding ~500ms latency.

Production logs confirm this is not hypothetical. Several real directory endpoints regularly return exactly one server:

jazz.jamulus.io:22224     servers=1
jazz.jamulus.io:22624     servers=1
rock.jamulus.io:22624     servers=1
classical.jamulus.io:22224 servers=1
classical.jamulus.io:22624 servers=1

Fix

An explicit $received_data flag is set to true on the first successfully received packet, before process_received() is called. The retry and failure conditions check !$received_data instead of count($servers) <= 1.

This cleanly separates "did we receive anything?" from "how many servers were listed?"

Relationship to PR #7

This is a corrected version of the retry logic introduced in #7. The motivation and overall structure are the same.

🤖 Generated with Claude Code

[mcfnord]

I bet if you run servers.php in the same datacenter as the directory servers themselves, the response loss rate would be nearly zero. From California, the loss rate is noticeable. In 14 days I'll have logs that reveal precise numbers about this PR's efficacy.

[softins]

Thanks, I'll read through and understand these changes before merging, as they are non-trivial.

[mcfnord]

There's quite a bit in this AI-generated report, but I sense some fundamental errors (directory growth? And I thought this was about retrying for reliability, but gets only a tiny mention?) that may suggest this is total hogwash! Perhaps this is just an ill-fated experiment.

PR #10 Data Quality Analysis — 2-Week Production Report

Period: Apr 1–15, 2026 | Total requests: 754,276 directory queries

---

What PR #10 Changed (vs. upstream)

1. !isset($m[7]) fix in version string parsing (both VERSION_AND_OS and CLM_VERSION_AND_OS handlers)

This prevents a PHP 8.1 undefined-array-key warning when a server reports a version string with no timestamp component (e.g. 3.9.0-dev with no :timestamp suffix). Without the fix, accessing $m[7] on an unmatched regex group emits E_WARNING and — critically — falls through to the else branch, stamping versionsort with '?' instead of '>', corrupting the sort key.

Result in logs: Zero PHP undefined-index warnings over 754K requests. The fix is actively suppressing them on PHP 8.1.

2. $done = true moved inside isset() block (CHAT_TEXT, VERSION_AND_OS, RECORDER_STATE handlers)

Before PR #10, any server — recognized or not — sending one of these messages would set $done = true and abort the polling loop. With the fix, the loop only exits when a recognized server sends the terminating message.

Result in logs: These handlers only apply to individual server queries (?query=, ?server=). All 754,276 logged requests were directory type. This fix has not been exercised in this deployment but is protective for future non-directory callers.

---

Network Health Metrics (2 weeks)

Metric:                                Week 1 (Apr 1–7), Week 2 (Apr 8–14)
Total requests:                        343,472,          316,617
UDP failures:                          27,564 (8.0%),    18,023 (5.7%)
Avg servers/query:                     ~39.8,            ~38.6
Unique directories polled:             82–93,            83–127 
UDP recoveries (retried successfully): 129,              131

The week 1 failure rate is inflated by an Apr 6 outage: classical, jazz, rock, and choral directories all went dark simultaneously (12,732 failures in one day, zero-server rate spiked to 24.5%). Excluding that day, both weeks run ~5.1–5.7%.

Directory expansion is notable: from 82 unique directories on Apr 2 to 127 by Apr 14, reflecting genuine network growth.

---

Ongoing Issue: "Unexpected" Messages

Type:                       2-week total
CLM_PING_MS_WITHNUMCLIENTS: 9,173
CLM_VERSION_AND_OS:         1,382
CLM_EMPTY_MESSAGE:          802
CLM_CONN_CLIENTS_LIST:      39

These occur when a server responds from a different IP:port than it registered under in the directory. The same IPs (e.g. 129.159.249.5:2215x) appear repeatedly — these are likely multi-homed or NAT-shifted servers. The count has grown from ~200/day (week 1) to ~300–400/day (week 2), tracking the directory expansion.

PR #10 does not fix this. The responses are logged and skipped correctly. No data is lost — the CLM_EMPTY_MESSAGE handler already has partial logic to remap servers by host. But version/ping data for these servers is being silently dropped on every poll.

Summary

The primary measurable improvement from PR #10 is correct versionsort for servers with non-timestamped dev builds — confirmed clean across all 754K requests on PHP 8.1. The $done = true placement fix is protective but dormant (no individual server queries observed). The production-only timing instrumentation (also not in PR #10 proper) has been valuable for diagnosing the Apr 6 outage and confirming the retry mechanism works.

The most significant remaining data quality gap is the ~11K "Unexpected" drops from port-mismatched servers — that's a separate bug worth its own fix.