bugfix/3514: do not throw on substituter errors if other substituters are still enabled

src/libstore/build/substitution-goal.cc

@@ -55,9 +55,14 @@ Goal::Co PathSubstitutionGoal::init()
     auto subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>();
 
     bool substituterFailed = false;
+    std::optional<Error> lastStoresException = std::nullopt;
 
     for (const auto & sub : subs) {
         trace("trying next substituter");
+        if (lastStoresException.has_value()) {
+            logError(lastStoresException->info());
+            lastStoresException.reset();
+        }
 
         cleanup();
 
@@ -80,19 +85,13 @@ Goal::Co PathSubstitutionGoal::init()
         try {
             // FIXME: make async
             info = sub->queryPathInfo(subPath ? *subPath : storePath);
-        } catch (InvalidPath &) {
+        } catch (InvalidPath & e) {
             continue;
         } catch (SubstituterDisabled & e) {
-            if (settings.tryFallback)
             continue;
-            else
-                throw e;
         } catch (Error & e) {
-            if (settings.tryFallback) {
+            lastStoresException = std::make_optional(std::move(e));
-                logError(e.info());
             continue;
-            } else
-                throw e;
         }
 
         if (info->path != storePath) {
@@ -156,6 +155,12 @@ Goal::Co PathSubstitutionGoal::init()
         worker.failedSubstitutions++;
         worker.updateProgress();
     }
+    if (lastStoresException.has_value()) {
+        if (!settings.tryFallback) {
+            throw *lastStoresException;
+        } else
+            logError(lastStoresException->info());
+    }
 
     /* Hack: don't indicate failure if there were no substituters.
        In that case the calling derivation should just do a

src/libstore/store-api.cc

@@ -1,3 +1,4 @@
+#include "nix/util/logging.hh"
 #include "nix/util/signature/local-keys.hh"
 #include "nix/util/source-accessor.hh"
 #include "nix/store/globals.hh"
@@ -392,11 +393,14 @@ void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, Substituta
 {
     if (!settings.useSubstitutes)
         return;
-    for (auto & sub : getDefaultSubstituters()) {
+
     for (auto & path : paths) {
-            if (infos.count(path.first))
+        std::optional<Error> lastStoresException = std::nullopt;
-                // Choose first succeeding substituter.
+        for (auto & sub : getDefaultSubstituters()) {
-                continue;
+            if (lastStoresException.has_value()) {
+                logError(lastStoresException->info());
+                lastStoresException.reset();
+            }
 
             auto subPath(path.first);
 
@@ -437,12 +441,15 @@ void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, Substituta
             } catch (InvalidPath &) {
             } catch (SubstituterDisabled &) {
             } catch (Error & e) {
-                if (settings.tryFallback)
+                lastStoresException = std::make_optional(std::move(e));
-                    logError(e.info());
-                else
-                    throw;
             }
         }
+        if (lastStoresException.has_value()) {
+            if (!settings.tryFallback) {
+                throw *lastStoresException;
+            } else
+                logError(lastStoresException->info());
+        }
     }
 }