From 079fe0ca5068a227b16bdc1f2bdc84ca83c30f6b Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <maximilian@mbosch.me>
Date: Sun, 2 Mar 2025 11:36:24 +0100
Subject: [PATCH] libexpr: improve error reporting if NIX_PATH item cannot be
 downloaded
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Last week I was hunting a production issue at work related to CppNix and
the only hint I had was this warning. While the cause was clearly a
CppNix issue since it was related to the libgit2-backed tarball cache,
the fact that this exception was hidden, made it way harder to discover
this. It would've saved me a lot of time[1] if the error was just
printed out (and thus would've ended up in the logs).

This is what this patch results in:

  $ nix-instantiate -I https://notexisting.com --find-file . --option connect-timeout 1
  warning: error: unable to download 'https://notexisting.com': Resolving timed out after 1001 milliseconds (curl error code=28); retrying in 265 ms
  warning: error: unable to download 'https://notexisting.com/': Connection timed out after 1002 milliseconds (curl error code=28); retrying in 523 ms
  warning: error: unable to download 'https://notexisting.com/': Connection timed out after 1001 milliseconds (curl error code=28); retrying in 1324 ms
  warning: error: unable to download 'https://notexisting.com/': Connection timed out after 1000 milliseconds (curl error code=28); retrying in 2670 ms
  warning:
         … while downloading https://notexisting.com to satisfy NIX_PATH lookup (ignoring)

         warning: unable to download 'https://notexisting.com/': Connection timed out after 1001 milliseconds (curl error code=28)
  /path/to/nix/path/root

The `unable to download` part was hidden before. Granted, this is not the most
interesting showcase given the retries before, but I'd still argue that
this is useful for cases where you don't have a connection timeout (and
this was just the most trivial way to test for me).

[1] Unlucky as I was, I had to mount an older backup and nixos-enter
    into the mount to reproduce this.

Change-Id: If2894369fc0b159a307b448e1ce7d9b54a327df6
---
 doc/manual/rl-next/improve-nix_path-error.md | 21 ++++++++++++++++++++
 lix/libexpr/eval.cc                          |  6 +++---
 2 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 doc/manual/rl-next/improve-nix_path-error.md

diff --git a/doc/manual/rl-next/improve-nix_path-error.md b/doc/manual/rl-next/improve-nix_path-error.md
new file mode 100644
index 000000000..ab5679719
--- /dev/null
+++ b/doc/manual/rl-next/improve-nix_path-error.md
@@ -0,0 +1,21 @@
+---
+synopsis: Show error when item from NIX_PATH cannot be downloaded
+issues: []
+cls: []
+category: Fixes
+credits: [ma27]
+---
+
+For e.g. `nix-instantiate -I https://example.com/404`, you'd only get a warning if the download failed, such as
+
+    warning: Nix search path entry 'https://example.com/404' cannot be downloaded, ignoring
+
+Now, the full error that caused the download failure is displayed with a note that the search
+path entry is ignored, e.g.
+
+    warning:
+         … while downloading https://example.com/404 to satisfy NIX_PATH lookup, ignoring search path entry
+
+         warning: unable to download 'https://example.com/404': HTTP error 404 ()
+
+         response body: […]
diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc
index ea493294b..541583806 100644
--- a/lix/libexpr/eval.cc
+++ b/lix/libexpr/eval.cc
@@ -1,6 +1,7 @@
 #include "lix/libexpr/eval.hh"
 #include "lix/libexpr/eval-settings.hh"
 #include "lix/libutil/async.hh"
+#include "lix/libutil/error.hh"
 #include "lix/libutil/hash.hh"
 #include "lix/libexpr/primops.hh"
 #include "lix/libexpr/print-options.hh"
@@ -2824,9 +2825,8 @@ try {
                 store, EvalSettings::resolvePseudoUrl(value), "source", false)).tree.storePath;
             res = { store->toRealPath(storePath) };
         } catch (FileTransferError & e) {
-            logWarning({
-                .msg = HintFmt("Nix search path entry '%1%' cannot be downloaded, ignoring", value)
-            });
+            e.addTrace(nullptr, "while downloading %s to satisfy NIX_PATH lookup, ignoring search path entry", value);
+            logWarning(e.info());
             res = std::nullopt;
         }
     }