From 7322f4dd8a6ca631af1a67c8f0555aaea703e927 Mon Sep 17 00:00:00 2001
From: Matthew Jackson <1085847+MattJackson@users.noreply.github.com>
Date: Wed, 29 Jul 2026 19:24:58 -0700
Subject: [PATCH] Record that the failed-vs-absent key source arm is
 unreachable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The `Ok(_) | Err(_)` arm in resolve_and_apply_traced conflates "this source
had no entry for the disc" with "this source failed", and reports both as
KeyNode::NoEntry. An operator whose key server is returning 502s is therefore
told their disc is not in the database.

The conflation is real but LATENT, and fixing it here would change nothing an
operator can see, because no shipped KeySource ever returns Err:
KeydbSource::get_unit_keys maps a load/parse failure to Ok(Vec::new()),
OnlineSource::get_unit_keys is Ok(self.query(ctx)) where query returns empty on
transport error, HTTP status, oversize body and bad JSON alike, and MultiSource
discards inner Errs. Only test doubles return Err. FetchOutcome::errored in
drive_unit_keys / drive_fmts_indexes is dead for the same reason — the right
contract, honoured by no source.

autorip already works around the missing signal by re-probing the service over
HTTP (probe_online_reachability / key_service_transient_status), and its own
comment names the incident: "the online keysource swallows every failure
(transport error, 502, timeout)".

So the fix belongs at the source boundary in freemkv-keysources, with
Disc::aacs_error as the channel the operator actually reads — not in this
trace. Documented here so the next reader does not assume the arm works, and
does not "fix" a dead path as I nearly did twice.
---
 src/keysource.rs | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/src/keysource.rs b/src/keysource.rs
index a7c3c16..885faf5 100644
--- a/src/keysource.rs
+++ b/src/keysource.rs
@@ -352,6 +352,29 @@ pub fn resolve_and_apply_traced(
             }
             // Empty (no key here) or a source failure — both are "no key from
             // this source"; move on to the next.
+            //
+            // NOTE: the `Err` half is currently UNREACHABLE in production, and the
+            // conflation below is therefore latent rather than live. Every shipped
+            // `KeySource` swallows its own failures into `Ok(Vec::new())`:
+            // `KeydbSource::get_unit_keys` maps a load/parse error to an empty vec,
+            // `OnlineSource::get_unit_keys` is `Ok(self.query(ctx))` where `query`
+            // returns empty on transport error, HTTP status, oversize body and bad
+            // JSON alike, and `MultiSource` discards inner `Err`s. Only test doubles
+            // return `Err`.
+            //
+            // Consequence: an unreachable key server arrives here as "no entry",
+            // and an operator is told their disc is not in the database. autorip
+            // works around it by re-probing the service over HTTP
+            // (`probe_online_reachability` / `key_service_transient_status`), whose
+            // own comment names the incident — "the online keysource swallows every
+            // failure (transport error, 502, timeout)".
+            //
+            // Fixing it HERE would change nothing: the fix belongs at the source
+            // boundary in `freemkv-keysources`, so a failure is reported as a
+            // failure, with `Disc::aacs_error` as the channel the operator actually
+            // reads. `FetchOutcome::errored` in `drive_unit_keys` /
+            // `drive_fmts_indexes` is dead for the same reason — it is the right
+            // contract, honoured by no source yet.
             Ok(_) | Err(_) => {
                 trace.keys.push(KeyStep {
                     who,