Merge pull request #28581 from vespa-engine/hmusum/better-error-message-when-file-missing-or-dir-empty

Give better error message for file config with errors

2 files changed, 45 insertions, 4 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
index 0e4ccf162ce..0196ffc7ce9 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFiles.java
@@ -11,8 +11,11 @@ import com.yahoo.path.Path;
 import com.yahoo.vespa.config.ConfigDefinition;
 import com.yahoo.vespa.config.ConfigDefinitionKey;
 import com.yahoo.vespa.config.ConfigPayloadBuilder;
+import com.yahoo.yolean.Exceptions;
 
+import java.io.File;
 import java.io.Serializable;
+import java.nio.file.Files;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Map;
@@ -45,7 +48,8 @@ public class UserConfiguredFiles implements Serializable {
             try {
                 register(builder, registeredFiles, key);
             } catch (IllegalArgumentException e) {
-                throw new IllegalArgumentException("Unable to send file specified in " + key, e);
+                throw new IllegalArgumentException("Unable to register file specified in services.xml for config '" + key + "': " +
+                                                   Exceptions.toMessageString(e));
             }
         }
     }
@@ -109,9 +113,9 @@ public class UserConfiguredFiles implements Serializable {
             String name = entry.getKey();
             ConfigPayloadBuilder fileEntry = builder.getObject(name);
             if (isEmptyOptionalPath(entry, fileEntry)) continue;
-            if (fileEntry.getValue() == null)
-                throw new IllegalArgumentException("Unable to send file for field '" + name +
-                                                   "': Invalid config value " + fileEntry.getValue());
+            if (fileEntry.getValue() == null || fileEntry.getValue().equals("."))
+                throw new IllegalArgumentException("Unable to register file for field '" + name +
+                                                   "': Invalid config value '" + fileEntry.getValue() + "'");
             registerFileEntry(fileEntry, registeredFiles, isModelType);
         }
     }
@@ -137,11 +141,19 @@ public class UserConfiguredFiles implements Serializable {
             path = Path.fromString(builder.getValue());
         }
 
+        File file = path.toFile();
+        if (file.isDirectory()) {
+            if (file.listFiles() == null || file.listFiles().length == 0)
+                throw new IllegalArgumentException("Directory '" + path + "' is empty");
+        }
+
         FileReference reference = registeredFiles.get(path);
         if (reference == null) {
             reference = fileRegistry.addFile(path.getRelative());
             registeredFiles.put(path, reference);
         }
+        if (reference == null)
+            throw new IllegalArgumentException("No such file or directory '" + path + "'");
 
         if (isModelType) {
             var model = ModelReference.valueOf(builder.getValue());
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
index fbc6e43cd78..cd5f76b422a 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/filedistribution/UserConfiguredFilesTest.java
@@ -24,6 +24,7 @@ import java.util.Optional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.fail;
 
 /**
  * @author Ulf Lilleengen
@@ -245,4 +246,32 @@ public class UserConfiguredFilesTest {
         userConfiguredFiles().register(producer);
     }
 
+    @Test
+    void require_that_missing_file_gives_sane_error_message() {
+        try {
+            def.addFileDef("fileVal");
+            builder.setField("fileVal", "foo.txt");
+            fileRegistry.pathToRef.put("bar.txt", new FileNode("barshash").value());
+            userConfiguredFiles().register(producer);
+            fail("Should have thrown exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Unable to register file specified in services.xml for config 'mynamespace.myname': No such file or directory 'foo.txt'", e.getMessage());
+        }
+    }
+
+    @Test
+    void require_that_using_dot_gives_sane_error_message() {
+        try {
+            def.addFileDef("fileVal");
+            builder.setField("fileVal", ".");
+            fileRegistry.pathToRef.put("bar.txt", new FileNode("barshash").value());
+            userConfiguredFiles().register(producer);
+            fail("Should have thrown exception");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Unable to register file specified in services.xml for config 'mynamespace.myname': Unable to register file for field 'fileVal': Invalid config value '.'",
+                         e.getMessage());
+        }
+    }
+
+
 }