update error messages

jdconrad · Feb 2, 2024 · fc8a987 · fc8a987
1 parent c9090d8
commit fc8a987
Show file tree

Hide file tree

Showing 8 changed files with 52 additions and 14 deletions.
diff --git a/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java b/server/src/main/java/org/elasticsearch/search/builder/SearchSourceBuilder.java
@@ -1348,6 +1348,9 @@ private SearchSourceBuilder parseXContent(
                 }
             } else if (token == XContentParser.Token.START_OBJECT) {
                 if (RETRIEVER.match(currentFieldName, parser.getDeprecationHandler())) {
+                    if (clusterSupportsFeature.test(RetrieverBuilder.NODE_FEATURE) == false) {
+                        throw new ParsingException(parser.getTokenLocation(), "Unknown key for a START_OBJECT in [retriever].");
+                    }
                     retrieverBuilder = RetrieverBuilder.parseTopLevelRetrieverBuilder(
                         parser,
                         new RetrieverParserContext(

diff --git a/server/src/main/java/org/elasticsearch/search/retriever/KnnRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/KnnRetrieverBuilder.java
@@ -8,6 +8,7 @@
 
 package org.elasticsearch.search.retriever;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
 import org.elasticsearch.search.vectors.KnnSearchBuilder;
@@ -76,7 +77,7 @@ public final class KnnRetrieverBuilder extends RetrieverBuilder<KnnRetrieverBuil
 
     public static KnnRetrieverBuilder fromXContent(XContentParser parser, RetrieverParserContext context) throws IOException {
         if (context.clusterSupportsFeature(NODE_FEATURE) == false) {
-            throw new IllegalArgumentException("[" + NAME + "] retriever is not a supported feature");
+            throw new ParsingException(parser.getTokenLocation(), "unknown retriever [" + NAME + "]");
         }
         return PARSER.apply(parser, context);
     }

diff --git a/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/RetrieverBuilder.java
@@ -10,6 +10,7 @@
 
 import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.xcontent.SuggestingErrorOnUnknown;
+import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.index.query.AbstractQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
@@ -28,6 +29,8 @@
 
 public abstract class RetrieverBuilder<RB extends RetrieverBuilder<RB>> {
 
+    public static final NodeFeature NODE_FEATURE = new NodeFeature("retrievers");
+
     public static final ParseField PRE_FILTER_FIELD = new ParseField("filter");
 
     protected static void declareBaseParserFields(

diff --git a/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java b/server/src/main/java/org/elasticsearch/search/retriever/RetrieversFeatures.java
@@ -17,6 +17,6 @@ public class RetrieversFeatures implements FeatureSpecification {
 
     @Override
     public Set<NodeFeature> getFeatures() {
-        return Set.of(StandardRetrieverBuilder.NODE_FEATURE, KnnRetrieverBuilder.NODE_FEATURE);
+        return Set.of(RetrieverBuilder.NODE_FEATURE, StandardRetrieverBuilder.NODE_FEATURE, KnnRetrieverBuilder.NODE_FEATURE);
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/search/retriever/StandardRetrieverBuilder.java b/server/src/main/java/org/elasticsearch/search/retriever/StandardRetrieverBuilder.java
@@ -8,6 +8,7 @@
 
 package org.elasticsearch.search.retriever;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.index.query.AbstractQueryBuilder;
 import org.elasticsearch.index.query.BoolQueryBuilder;
@@ -86,7 +87,7 @@ public final class StandardRetrieverBuilder extends RetrieverBuilder<StandardRet
 
     public static StandardRetrieverBuilder fromXContent(XContentParser parser, RetrieverParserContext context) throws IOException {
         if (context.clusterSupportsFeature(NODE_FEATURE) == false) {
-            throw new IllegalArgumentException("[" + NAME + "] retriever is not a supported feature");
+            throw new ParsingException(parser.getTokenLocation(), "unknown retriever [" + NAME + "]");
         }
         return PARSER.apply(parser, context);
     }

diff --git a/server/src/test/java/org/elasticsearch/search/retriever/RetrieverBuilderVersionTests.java b/server/src/test/java/org/elasticsearch/search/retriever/RetrieverBuilderVersionTests.java
@@ -8,6 +8,7 @@
 
 package org.elasticsearch.search.retriever;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.search.SearchModule;
@@ -24,18 +25,43 @@
 public class RetrieverBuilderVersionTests extends ESTestCase {
 
     public void testRetrieverVersions() throws IOException {
-        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"blah\":{}}}")) {
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"standard\":{}}}")) {
             SearchSourceBuilder ssb = new SearchSourceBuilder();
-            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> ssb.parseXContent(parser, true, nf -> false));
-            assertEquals("[standard] retriever is not a supported feature", iae.getMessage());
-            ssb.parseXContent(parser, false, nf -> nf == StandardRetrieverBuilder.NODE_FEATURE);
+            ParsingException iae = expectThrows(ParsingException.class, () -> ssb.parseXContent(parser, true, nf -> false));
+            assertEquals("Unknown key for a START_OBJECT in [retriever].", iae.getMessage());
+        }
+
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"standard\":{}}}")) {
+            SearchSourceBuilder ssb = new SearchSourceBuilder();
+            ParsingException iae = expectThrows(
+                ParsingException.class,
+                () -> ssb.parseXContent(parser, true, nf -> nf == RetrieverBuilder.NODE_FEATURE)
+            );
+            assertEquals("unknown retriever [standard]", iae.getMessage());
+        }
+
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"standard\":{}}}")) {
+            SearchSourceBuilder ssb = new SearchSourceBuilder();
+            ssb.parseXContent(parser, true, nf -> nf == RetrieverBuilder.NODE_FEATURE || nf == StandardRetrieverBuilder.NODE_FEATURE);
         }
 
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"knn\":{}}}")) {
             SearchSourceBuilder ssb = new SearchSourceBuilder();
-            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> ssb.parseXContent(parser, true, nf -> false));
-            assertEquals("[knn] retriever is not a supported feature", iae.getMessage());
-            ssb.parseXContent(parser, false, nf -> nf == KnnRetrieverBuilder.NODE_FEATURE);
+            ParsingException iae = expectThrows(
+                ParsingException.class,
+                () -> ssb.parseXContent(parser, true, nf -> nf == RetrieverBuilder.NODE_FEATURE)
+            );
+            assertEquals("unknown retriever [knn]", iae.getMessage());
+        }
+
+        try (
+            XContentParser parser = createParser(
+                JsonXContent.jsonXContent,
+                "{\"retriever\":{\"knn\":{\"field\": \"test\", \"k\": 2, \"num_candidates\": 5, \"query_vector\": [1, 2, 3]}}}"
+            )
+        ) {
+            SearchSourceBuilder ssb = new SearchSourceBuilder();
+            ssb.parseXContent(parser, true, nf -> nf == RetrieverBuilder.NODE_FEATURE || nf == KnnRetrieverBuilder.NODE_FEATURE);
         }
     }
 

diff --git a/...k/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java b/...k/plugin/rank-rrf/src/main/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilder.java
@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.rank.rrf;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.license.LicenseUtils;
 import org.elasticsearch.search.builder.SearchSourceBuilder;
@@ -50,7 +51,7 @@ public final class RRFRetrieverBuilder extends RetrieverBuilder<RRFRetrieverBuil
 
     public static RRFRetrieverBuilder fromXContent(XContentParser parser, RetrieverParserContext context) throws IOException {
         if (context.clusterSupportsFeature(NODE_FEATURE) == false) {
-            throw new IllegalArgumentException("[" + RRFRankPlugin.NAME + "] retriever is not a supported feature");
+            throw new ParsingException(parser.getTokenLocation(), "unknown retriever [" + RRFRankPlugin.NAME + "]");
         }
         if (RRFRankPlugin.RANK_RRF_FEATURE.check(XPackPlugin.getSharedLicenseState()) == false) {
             throw LicenseUtils.newComplianceException("Reciprocal Rank Fusion (RRF)");

diff --git a/...gin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java b/...gin/rank-rrf/src/test/java/org/elasticsearch/xpack/rank/rrf/RRFRetrieverBuilderTests.java
@@ -7,6 +7,7 @@
 
 package org.elasticsearch.xpack.rank.rrf;
 
+import org.elasticsearch.common.ParsingException;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.features.NodeFeature;
 import org.elasticsearch.search.SearchModule;
@@ -28,9 +29,11 @@ public class RRFRetrieverBuilderTests extends ESTestCase {
     public void testRetrieverVersions() throws IOException {
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"retriever\":{\"rrf\":{}}}")) {
             SearchSourceBuilder ssb = new SearchSourceBuilder();
-            IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> ssb.parseXContent(parser, true, nf -> false));
-            assertEquals("[rrf] retriever is not a supported feature", iae.getMessage());
-            ssb.parseXContent(parser, false, nf -> nf == RRFRetrieverBuilder.NODE_FEATURE);
+            ParsingException iae = expectThrows(
+                ParsingException.class,
+                () -> ssb.parseXContent(parser, true, nf -> nf == RetrieverBuilder.NODE_FEATURE)
+            );
+            assertEquals("unknown retriever [rrf]", iae.getMessage());
         }
     }