Merge pull request #7546 from vespa-engine/bratseth/improve-error-message

Improve error message etc

6 files changed, 29 insertions, 16 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/ConfigProducerGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/ConfigProducerGroup.java
index e8142999433..66294d3fcef 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/ConfigProducerGroup.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/ConfigProducerGroup.java
@@ -20,10 +20,10 @@ public class ConfigProducerGroup<CHILD extends AbstractConfigProducer<?>> extend
     }
 
     public void addComponent(ComponentId id, CHILD producer) {
-        boolean wasAdded = producerById.put(id, producer) == null;
-        if (!wasAdded) {
-            throw new IllegalArgumentException("Two entities have the same component id '" +
-                                               id + "' in the same scope.");
+        CHILD existing = producerById.put(id, producer);
+        if ( existing != null) {
+            throw new IllegalArgumentException("Both " + producer + " and " + existing + " are configured" +
+                                               " with the id '" + id + "'. All components must have a unique id.");
         }
         addChild(producer);
     }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chain.java b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chain.java
index f795e481f62..6b4f8c8f8b5 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chain.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/component/chain/Chain.java
@@ -78,4 +78,9 @@ public class Chain<T extends ChainedComponent<?>> extends AbstractConfigProducer
         return TYPE;
     }
 
+    @Override
+    public String toString() {
+        return "chain '" + componentId + "'";
+    }
+
 }
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java
index 2605736e23b..ff211264a34 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/search/searchchain/SearchChain.java
@@ -28,4 +28,9 @@ public class SearchChain extends Chain<Searcher<?>> {
         return Collections.emptyList();
     }
 
+    @Override
+    public String toString() {
+        return "search chain '" + getId() + "'";
+    }
+
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SearchChainsTest2.java b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SearchChainsTest2.java
index 9122e855461..6ba75f1ff05 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SearchChainsTest2.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/search/searchchain/SearchChainsTest2.java
@@ -9,13 +9,11 @@ import org.junit.Before;
 import org.junit.Test;
 import org.w3c.dom.Element;
 
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
-import static org.hamcrest.Matchers.containsString;
 
 /**
  * @author gjoranv
- * @since 5.1.11
  */
 public class SearchChainsTest2 {
 
@@ -38,7 +36,8 @@ public class SearchChainsTest2 {
             chains.validate();
             fail("Expected exception when inheriting a nonexistent search chain.");
         } catch (Exception e) {
-            assertThat(e.getMessage(), containsString("Missing chain 'nonexistent'"));
+            assertEquals("Missing chain 'nonexistent'.",
+                         e.getMessage());
         }
     }
 
@@ -56,12 +55,13 @@ public class SearchChainsTest2 {
             ContainerModelBuilderTest.createModel(root, clusterElem);
             fail("Expected exception when declaring chains with duplicate id.");
         } catch (Exception e) {
-            assertThat(e.getMessage(), containsString("Two entities have the same component id 'same'"));
+            assertEquals("Both search chain 'same' and search chain 'same' are configured with the id 'same'. All components must have a unique id.",
+                         e.getMessage());
         }
     }
 
     @Test
-    public void fail_upon_user_declared_chain_with_same_id_as_builtin_chain() throws Exception {
+    public void fail_upon_user_declared_chain_with_same_id_as_builtin_chain() {
         final Element clusterElem = DomBuilderTest.parse(
                 "<jdisc id='cluster1' version='1.0'>",
                 ContainerModelBuilderTest.nodesXml,
@@ -73,7 +73,8 @@ public class SearchChainsTest2 {
             ContainerModelBuilderTest.createModel(root, clusterElem);
             fail("Expected exception when taking the id from a builtin chain.");
         } catch (Exception e) {
-            assertThat(e.getMessage(), containsString("Two entities have the same component id 'vespa'"));
+            assertEquals("Both search chain 'vespa' and search chain 'vespa' are configured with the id 'vespa'. All components must have a unique id.",
+                         e.getMessage());
         }
     }
 }
diff --git a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
index 1fad45a99e3..ca6fd44af50 100644
--- a/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
+++ b/container-search/src/main/java/com/yahoo/search/query/ranking/SoftTimeout.java
@@ -8,7 +8,7 @@ import com.yahoo.search.query.profile.types.QueryProfileType;
 import java.util.Objects;
 
 /**
- * Holds the settings for the soft-timeout feature.
+ * Settings for the soft-timeout feature.
  *
  * @author baldersheim
  */
@@ -41,22 +41,24 @@ public class SoftTimeout implements Cloneable {
 
     public void setFactor(double factor) {
         if ((factor < 0.0) || (factor > 1.0)) {
-            throw new IllegalArgumentException("factor must be in the range [0.0, 1.0]. It is " + factor);
+            throw new IllegalArgumentException("factor must be in the range [0.0, 1.0], got " + factor);
         }
         this.factor = factor;
     }
+
     public Double getFactor() { return factor; }
+
     public void setTailcost(double tailcost) {
         if ((tailcost < 0.0) || (tailcost > 1.0)) {
-            throw new IllegalArgumentException("tailcost must be in the range [0.0, 1.0]. It is " + tailcost);
+            throw new IllegalArgumentException("tailcost must be in the range [0.0, 1.0], got " + tailcost);
         }
         this.tailcost = tailcost;
     }
+
     public Double getTailcost() { return tailcost; }
 
     /** Internal operation - DO NOT USE */
     public void prepare(RankProperties rankProperties) {
-
         if (enable != null) {
             rankProperties.put("vespa.softtimeout.enable", String.valueOf(enable));
         }
diff --git a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
index 01501674ae8..83ef955a6d9 100644
--- a/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
+++ b/container-search/src/test/java/com/yahoo/search/query/SoftTimeoutTestCase.java
@@ -37,7 +37,7 @@ public class SoftTimeoutTestCase {
         } catch (QueryException e) {
             assertEquals("Invalid request parameter", e.getMessage());
             assertEquals("Could not set 'ranking.softtimeout." + key + "' to '" + value +"'", e.getCause().getMessage());
-            assertEquals(key + " must be in the range [0.0, 1.0]. It is " + value, e.getCause().getCause().getMessage());
+            assertEquals(key + " must be in the range [0.0, 1.0], got " + value, e.getCause().getCause().getMessage());
         }
     }
     @Test