Fix/meta fields bad request (#117229)

 400 rather a 5xx error is returned when _source / _seq_no / _feature / _nested_path / _field_names is requested, via fields

Author: drempapis

docs/changelog/117229.yaml

@@ -0,0 +1,6 @@
+pr: 117229
+summary: "In this pr, a 400 error is returned when _source / _seq_no / _feature /\
+  \ _nested_path / _field_names is requested, rather a 5xx"
+area: Search
+type: bug
+issues: []

modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/RankFeatureMetaFieldMapper.java

@@ -48,7 +48,7 @@ public String typeName() {
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-            throw new UnsupportedOperationException("Cannot fetch values for internal field [" + typeName() + "].");
+            throw new IllegalArgumentException("Cannot fetch values for internal field [" + typeName() + "].");
         }
 
         @Override

rest-api-spec/build.gradle

@@ -66,4 +66,5 @@ tasks.named("yamlRestCompatTestTransform").configure ({ task ->
   task.skipTest("logsdb/20_source_mapping/stored _source mode is supported", "no longer serialize source_mode")
   task.skipTest("logsdb/20_source_mapping/include/exclude is supported with stored _source", "no longer serialize source_mode")
   task.skipTest("logsdb/20_source_mapping/synthetic _source is default", "no longer serialize source_mode")
+  task.skipTest("search/520_fetch_fields/fetch _seq_no via fields", "error code is changed from 5xx to 400 in 9.0")
 })

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/520_fetch_fields.yml

@@ -128,18 +128,88 @@ fetch _seq_no via stored_fields:
 
 ---
 fetch _seq_no via fields:
+  - requires:
+      cluster_features: ["meta_fetch_fields_error_code_changed"]
+      reason: The fields_api returns a 400 instead a 5xx when _seq_no is requested via fields
 
   - do:
-      catch: "request"
+      catch: bad_request
       search:
         index: test
         body:
           fields: [ _seq_no ]
 
-  # This should be `unauthorized` (401) or `forbidden` (403) or at least `bad request` (400)
-  # while instead it is mapped to an `internal_server_error (500)`
-  - match: { status: 500 }
-  - match: { error.root_cause.0.type: unsupported_operation_exception }
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: illegal_argument_exception }
+  - match: { error.root_cause.0.reason: "error fetching [_seq_no]: Cannot fetch values for internal field [_seq_no]." }
+
+---
+fetch _source via fields:
+  - requires:
+      cluster_features: ["meta_fetch_fields_error_code_changed"]
+      reason: The fields_api returns a 400 instead a 5xx when _seq_no is requested via fields
+
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          fields: [ _source ]
+
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: illegal_argument_exception }
+  - match: { error.root_cause.0.reason: "error fetching [_source]: Cannot fetch values for internal field [_source]." }
+
+---
+fetch _feature via fields:
+  - requires:
+      cluster_features: ["meta_fetch_fields_error_code_changed"]
+      reason: The fields_api returns a 400 instead a 5xx when _seq_no is requested via fields
+
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          fields: [ _feature ]
+
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: illegal_argument_exception }
+  - match: { error.root_cause.0.reason: "error fetching [_feature]: Cannot fetch values for internal field [_feature]." }
+
+---
+fetch _nested_path via fields:
+  - requires:
+      cluster_features: ["meta_fetch_fields_error_code_changed"]
+      reason: The fields_api returns a 400 instead a 5xx when _seq_no is requested via fields
+
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          fields: [ _nested_path ]
+
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: illegal_argument_exception }
+  - match: { error.root_cause.0.reason: "error fetching [_nested_path]: Cannot fetch values for internal field [_nested_path]." }
+
+---
+fetch _field_names via fields:
+  - requires:
+      cluster_features: ["meta_fetch_fields_error_code_changed"]
+      reason: The fields_api returns a 400 instead a 5xx when _seq_no is requested via fields
+
+  - do:
+      catch: bad_request
+      search:
+        index: test
+        body:
+          fields: [ _field_names ]
+
+  - match: { status: 400 }
+  - match: { error.root_cause.0.type: illegal_argument_exception }
+  - match: { error.root_cause.0.reason: "error fetching [_field_names]: Cannot fetch values for internal field [_field_names]." }
 
 ---
 fetch fields with none stored_fields:

server/src/main/java/org/elasticsearch/index/mapper/FieldNamesFieldMapper.java

@@ -135,7 +135,7 @@ public boolean isEnabled() {
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-            throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
+            throw new IllegalArgumentException("Cannot fetch values for internal field [" + name() + "].");
         }
 
         @Override

server/src/main/java/org/elasticsearch/index/mapper/MapperFeatures.java

@@ -61,6 +61,8 @@ public Set<NodeFeature> getFeatures() {
         "mapper.constant_keyword.synthetic_source_write_fix"
     );
 
+    public static final NodeFeature META_FETCH_FIELDS_ERROR_CODE_CHANGED = new NodeFeature("meta_fetch_fields_error_code_changed");
+
     @Override
     public Set<NodeFeature> getTestFeatures() {
         return Set.of(
@@ -71,7 +73,8 @@ public Set<NodeFeature> getTestFeatures() {
             IgnoredSourceFieldMapper.IGNORED_SOURCE_AS_TOP_LEVEL_METADATA_ARRAY_FIELD,
             IgnoredSourceFieldMapper.ALWAYS_STORE_OBJECT_ARRAYS_IN_NESTED_OBJECTS,
             MapperService.LOGSDB_DEFAULT_IGNORE_DYNAMIC_BEYOND_LIMIT,
-            CONSTANT_KEYWORD_SYNTHETIC_SOURCE_WRITE_FIX
+            CONSTANT_KEYWORD_SYNTHETIC_SOURCE_WRITE_FIX,
+            META_FETCH_FIELDS_ERROR_CODE_CHANGED
         );
     }
 }

server/src/main/java/org/elasticsearch/index/mapper/NestedPathFieldMapper.java

@@ -67,7 +67,7 @@ public Query existsQuery(SearchExecutionContext context) {
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-            throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
+            throw new IllegalArgumentException("Cannot fetch values for internal field [" + name() + "].");
         }
 
         @Override

server/src/main/java/org/elasticsearch/index/mapper/SeqNoFieldMapper.java

@@ -168,7 +168,7 @@ public boolean mayExistInIndex(SearchExecutionContext context) {
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-            throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
+            throw new IllegalArgumentException("Cannot fetch values for internal field [" + name() + "].");
         }
 
         @Override

server/src/main/java/org/elasticsearch/index/mapper/SourceFieldMapper.java

@@ -325,7 +325,7 @@ public String typeName() {
 
         @Override
         public ValueFetcher valueFetcher(SearchExecutionContext context, String format) {
-            throw new UnsupportedOperationException("Cannot fetch values for internal field [" + name() + "].");
+            throw new IllegalArgumentException("Cannot fetch values for internal field [" + name() + "].");
         }
 
         @Override

server/src/test/java/org/elasticsearch/search/fetch/subphase/FieldFetcherTests.java

@@ -271,7 +271,7 @@ public void testMetadataFields() throws IOException {
             FieldNamesFieldMapper.NAME,
             NestedPathFieldMapper.name(IndexVersion.current())
         )) {
-            expectThrows(UnsupportedOperationException.class, () -> fetchFields(mapperService, source, fieldname));
+            expectThrows(IllegalArgumentException.class, () -> fetchFields(mapperService, source, fieldname));
         }
     }