Merge pull request #12091 from cole-h/fixup-s3-bad-error-formatting

libstore: fixup unformatted uri when S3 getObject fails
NixOS · Dec 19, 2024 · 6126007 · 6126007
2 parents 3f3feae + b978fa8
commit 6126007
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc
@@ -767,7 +767,7 @@ struct curlFileTransfer : public FileTransfer
                 auto s3Res = s3Helper.getObject(bucketName, key);
                 FileTransferResult res;
                 if (!s3Res.data)
-                    throw FileTransferError(NotFound, "S3 object '%s' does not exist", request.uri);
+                    throw FileTransferError(NotFound, {}, "S3 object '%s' does not exist", request.uri);
                 res.data = std::move(*s3Res.data);
                 res.urls.push_back(request.uri);
                 callback(std::move(res));

diff --git a/tests/nixos/s3-binary-cache-store.nix b/tests/nixos/s3-binary-cache-store.nix
@@ -10,6 +10,7 @@ let
   env = "AWS_ACCESS_KEY_ID=${accessKey} AWS_SECRET_ACCESS_KEY=${secretKey}";
 
   storeUrl = "s3://my-cache?endpoint=http://server:9000&region=eu-west-1";
+  objectThatDoesNotExist = "s3://my-cache/foo-that-does-not-exist?endpoint=http://server:9000&region=eu-west-1";
 
 in {
   name = "s3-binary-cache-store";
@@ -20,7 +21,10 @@ in {
         { virtualisation.writableStore = true;
           virtualisation.additionalPaths = [ pkgA ];
           environment.systemPackages = [ pkgs.minio-client ];
-          nix.extraOptions = "experimental-features = nix-command";
+          nix.extraOptions = ''
+            experimental-features = nix-command
+            substituters =
+          '';
           services.minio = {
             enable = true;
             region = "eu-west-1";
@@ -35,7 +39,10 @@ in {
       client =
         { config, pkgs, ... }:
         { virtualisation.writableStore = true;
-          nix.extraOptions = "experimental-features = nix-command";
+          nix.extraOptions = ''
+            experimental-features = nix-command
+            substituters =
+          '';
         };
     };
 
@@ -54,6 +61,12 @@ in {
     # Test fetchurl on s3:// URLs while we're at it.
     client.succeed("${env} nix eval --impure --expr 'builtins.fetchurl { name = \"foo\"; url = \"s3://my-cache/nix-cache-info?endpoint=http://server:9000&region=eu-west-1\"; }'")
 
+    # Test that the format string in the error message is properly setup and won't display `%s` instead of the failed URI
+    msg = client.fail("${env} nix eval --impure --expr 'builtins.fetchurl { name = \"foo\"; url = \"${objectThatDoesNotExist}\"; }' 2>&1")
+    if "S3 object '${objectThatDoesNotExist}' does not exist" not in msg:
+      print(msg) # So that you can see the message that was improperly formatted
+      raise Exception("Error message formatting didn't work")
+
     # Copy a package from the binary cache.
     client.fail("nix path-info ${pkgA}")