libstore: treat more substituter failures as recoverable

if a substituter is entirely offline and cannot be queries at all we
should not be failing if other substituters are configured. likewise
if a substituter goes offline after querying but before we try using
it we should attempt to fetch that path from some other substituter.

ideally we'd treat all substituters as a single entity instead of as
one store each, then have that single entity take care of fallbacks,
retries, error reporting, etc. that requires larger rewrites though.

fixes #1061

Change-Id: I9d8fc0544ff380bf017256e8fcc82823dc634f10

lix/libstore/build/substitution-goal.cc

@@ -120,7 +120,8 @@ try {
             if (settings.tryFallback) {
                 logError(e.info());
             } else {
-                throw;
+                logErrorInfo(lvlWarn, e.info());
+                substituterFailed = true;
             }
         }
         co_return co_await tryNext();

lix/libstore/store-api.cc

@@ -563,6 +563,9 @@ try {
 kj::Promise<Result<void>> Store::querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos)
 try {
     if (!settings.useSubstitutes) co_return result::success();
+
+    std::unordered_map<StorePath, std::exception_ptr> errors;
+
     for (auto & sub : TRY_AWAIT(getDefaultSubstituters())) {
         for (auto & path : paths) {
             if (infos.count(path.first))
@@ -600,17 +603,24 @@ try {
                     .downloadSize = narInfo ? narInfo->fileSize : 0,
                     .narSize = info->narSize,
                 });
+                errors.erase(path.first);
             } catch (InvalidPath &) {
             } catch (SubstituterDisabled &) {
             } catch (Error & e) {
-                if (settings.tryFallback)
+                if (settings.tryFallback) {
                     logError(e.info());
-                else
-                    throw;
+                } else {
+                    logErrorInfo(lvlWarn, e.info());
+                    errors.emplace(path.first, std::current_exception());
+                }
             }
         }
     }
 
+    if (!errors.empty() && !settings.tryFallback) {
+        std::rethrow_exception(errors.begin()->second);
+    }
+
     co_return result::success();
 } catch (...) {
     co_return result::current_exception();

tests/functional2/build/test_substitution.py

@@ -0,0 +1,69 @@
+import shutil
+import pytest
+import re
+import dataclasses
+
+from testlib.fixtures.command import CommandResult
+from testlib.fixtures.nix import Nix
+from testlib.fixtures.env import ManagedEnv
+
+
+def build(nix: Nix, *args) -> CommandResult:
+    expr = """
+        derivation {
+            name = "test";
+            system = builtins.currentSystem;
+            builder = "/bin/sh";
+            args = [ "-c" "echo > $out" ];
+            outputHashMode = "flat";
+            outputHash = "sha256-AbpHGcgLb+kRsJGnwFEktk7uzpZOCcBY74+YBdrKVGs=";
+        }
+    """
+    return nix.nix_build(["-E", expr, "--no-link", "--no-require-sigs", *args], build=True).run()
+
+
+@dataclasses.dataclass
+class Caches:
+    good: str
+    bad: str
+
+
+@pytest.fixture
+def caches(nix: Nix, env: ManagedEnv) -> Caches:
+    good_path, bad_path = env.dirs.home / "good", env.dirs.home / "bad"
+    good_uri, bad_uri = f"file://{good_path}", f"file://{bad_path}"
+
+    # build the derivation
+    output = build(nix).ok().stdout_s.strip()
+    # copy it to the good cache
+    nix.nix(["copy", output, "--to", good_uri, "--no-require-sigs"], flake=True).run().ok()
+    # create the bad cache by simulating read failures
+    shutil.copytree(good_path, bad_path)
+    for info in bad_path.glob("*.narinfo"):
+        info.chmod(0o200)
+
+    nix.nix(["store", "delete", output], flake=True).run().ok()
+
+    return Caches(good=good_uri, bad=bad_uri)
+
+
+def test_substitution_fallback_good_first(nix: Nix, caches: Caches):
+    build(nix, "--substituters", f"{caches.good} {caches.bad}").ok()
+
+
+def test_substitution_fallback_bad_first(nix: Nix, caches: Caches):
+    # we expect three warnings for the single nar: two from querying, one from the substitution itself
+    result = build(nix, "--substituters", f"{caches.bad} {caches.good}").ok()
+    assert len(re.findall(r"warning.*narinfo", result.stderr_s)) == 3
+
+
+def test_substitution_fallback_may_build(nix: Nix, caches: Caches):
+    # we expect two errors for the single nar: one from querying, one from the substitution itself
+    result = build(nix, "--substituters", f"{caches.bad}", "--fallback").ok()
+    assert len(re.findall(r"error.*narinfo", result.stderr_s)) == 2
+
+
+def test_substitution_fallback_no_build(nix: Nix, caches: Caches):
+    # we expect one error, and it's fatal
+    result = build(nix, "--substituters", f"{caches.bad}").expect(1)
+    assert len(re.findall(r"error.*narinfo", result.stderr_s)) == 1