[8.x] Add a basic deprecation warning that the JSON format is changing in v9 (#116330)

docs/changelog/114739.yaml

@@ -0,0 +1,20 @@
+pr: 114739
+summary: Add a basic deprecation warning that the JSON format for non-detailed error responses is changing in v9
+area: Infra/REST API
+type: deprecation
+issues: [89387]
+deprecation:
+  title: The format of non-detailed error responses is changing in v9
+  area: REST API
+  details: |-
+    When an error occurs when processing a request, Elasticsearch returns information on that error in the REST response.
+    If `http:detailed_errors.enabled: false` is specified in node settings with the v8 REST API and below,
+    the format of this response changes significantly.
+    Starting with the v9 REST API, the JSON structure of responses with errors when the `http.detailed_errors.enabled: false` option is set
+    will be the same as when detailed errors are enabled (which is the default).
+    To keep using the existing format for non-detailed error responses, use the v8 REST API.
+  impact: |-
+      If you have set `http.detailed_errors.enabled: false` (the default is `true`)
+      the structure of JSON when any exceptions occur will change with the v9 REST API.
+      To keep using the existing format, use the v8 REST API.
+  notable: false

server/src/main/java/org/elasticsearch/rest/RestResponse.java

@@ -16,6 +16,8 @@
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.logging.DeprecationCategory;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.util.Maps;
 import org.elasticsearch.core.Nullable;
 import org.elasticsearch.core.Releasable;
@@ -43,6 +45,7 @@ public final class RestResponse implements Releasable {
     static final String STATUS = "status";
 
     private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed");
+    private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(AbstractRestChannel.class);
 
     private final RestStatus status;
 
@@ -142,6 +145,16 @@ public RestResponse(RestChannel channel, RestStatus status, Exception e) throws
         if (params.paramAsBoolean("error_trace", false) && status != RestStatus.UNAUTHORIZED) {
             params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params);
         }
+
+        if (channel.detailedErrorsEnabled() == false) {
+            deprecationLogger.warn(
+                DeprecationCategory.API,
+                "http_detailed_errors",
+                "The JSON format of non-detailed errors will change in Elasticsearch 9.0 to match the JSON structure"
+                    + " used for detailed errors. To keep using the existing format, use the V8 REST API."
+            );
+        }
+
         try (XContentBuilder builder = channel.newErrorBuilder()) {
             build(builder, params, status, channel.detailedErrorsEnabled(), e);
             this.content = BytesReference.bytes(builder);

server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionTests.java

@@ -26,7 +26,7 @@ public void testEmptyRequestBody() throws Exception {
             .withParams(Map.of("synonymsSet", "testSet", "synonymRuleId", "testRule"))
             .build();
 
-        FakeRestChannel channel = new FakeRestChannel(request, false, 0);
+        FakeRestChannel channel = new FakeRestChannel(request, true, 0);
         try (var threadPool = createThreadPool()) {
             final var nodeClient = new NoOpNodeClient(threadPool);
             expectThrows(IllegalArgumentException.class, () -> action.handleRequest(request, channel, nodeClient));

server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionTests.java

@@ -26,7 +26,7 @@ public void testEmptyRequestBody() throws Exception {
             .withParams(Map.of("synonymsSet", "test"))
             .build();
 
-        FakeRestChannel channel = new FakeRestChannel(request, false, 0);
+        FakeRestChannel channel = new FakeRestChannel(request, true, 0);
         try (var threadPool = createThreadPool()) {
             final var nodeClient = new NoOpNodeClient(threadPool);
             expectThrows(IllegalArgumentException.class, () -> action.handleRequest(request, channel, nodeClient));

server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java

@@ -271,7 +271,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
         final RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build();
         final RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel(
             fakeRequest,
-            false,
+            true,
             RestStatus.BAD_REQUEST
         );
 
@@ -361,7 +361,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
         Map<String, List<String>> restHeaders = new HashMap<>();
         restHeaders.put(Task.TRACE_PARENT_HTTP_HEADER, Collections.singletonList(traceParentValue));
         RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build();
-        RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
 
         try (
             AbstractHttpServerTransport transport = new AbstractHttpServerTransport(

server/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java

@@ -73,7 +73,7 @@ public List<Route> routes() {
         params.put("consumed", randomAlphaOfLength(8));
         params.put("unconsumed", randomAlphaOfLength(8));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         final IllegalArgumentException e = expectThrows(
             IllegalArgumentException.class,
             () -> handler.handleRequest(request, channel, mockClient)
@@ -108,7 +108,7 @@ public List<Route> routes() {
         params.put("unconsumed-first", randomAlphaOfLength(8));
         params.put("unconsumed-second", randomAlphaOfLength(8));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         final IllegalArgumentException e = expectThrows(
             IllegalArgumentException.class,
             () -> handler.handleRequest(request, channel, mockClient)
@@ -155,7 +155,7 @@ public List<Route> routes() {
         params.put("very_close_to_parametre", randomAlphaOfLength(8));
         params.put("very_far_from_every_consumed_parameter", randomAlphaOfLength(8));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         final IllegalArgumentException e = expectThrows(
             IllegalArgumentException.class,
             () -> handler.handleRequest(request, channel, mockClient)
@@ -206,7 +206,7 @@ public List<Route> routes() {
         params.put("consumed", randomAlphaOfLength(8));
         params.put("response_param", randomAlphaOfLength(8));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         handler.handleRequest(request, channel, mockClient);
         assertTrue(restChannelConsumer.executed);
         assertTrue(restChannelConsumer.closed);
@@ -238,7 +238,7 @@ public List<Route> routes() {
         params.put("human", null);
         params.put("error_trace", randomFrom("true", "false", null));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         handler.handleRequest(request, channel, mockClient);
         assertTrue(restChannelConsumer.executed);
         assertTrue(restChannelConsumer.closed);
@@ -283,7 +283,7 @@ public List<Route> routes() {
         params.put("size", randomAlphaOfLength(8));
         params.put("time", randomAlphaOfLength(8));
         RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
-        RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        RestChannel channel = new FakeRestChannel(request, true, 1);
         handler.handleRequest(request, channel, mockClient);
         assertTrue(restChannelConsumer.executed);
         assertTrue(restChannelConsumer.closed);
@@ -314,7 +314,7 @@ public List<Route> routes() {
                 new BytesArray(builder.toString()),
                 XContentType.JSON
             ).build();
-            final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+            final RestChannel channel = new FakeRestChannel(request, true, 1);
             handler.handleRequest(request, channel, mockClient);
             assertTrue(restChannelConsumer.executed);
             assertTrue(restChannelConsumer.closed);
@@ -341,7 +341,7 @@ public List<Route> routes() {
         };
 
         final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).build();
-        final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+        final RestChannel channel = new FakeRestChannel(request, true, 1);
         handler.handleRequest(request, channel, mockClient);
         assertTrue(restChannelConsumer.executed);
         assertTrue(restChannelConsumer.closed);
@@ -371,7 +371,7 @@ public List<Route> routes() {
                 new BytesArray(builder.toString()),
                 XContentType.JSON
             ).build();
-            final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
+            final RestChannel channel = new FakeRestChannel(request, true, 1);
             final IllegalArgumentException e = expectThrows(
                 IllegalArgumentException.class,
                 () -> handler.handleRequest(request, channel, mockClient)

server/src/test/java/org/elasticsearch/rest/ChunkedRestResponseBodyPartTests.java

@@ -56,7 +56,7 @@ public void testEncodesChunkedXContentCorrectly() throws IOException {
             ToXContent.EMPTY_PARAMS,
             new FakeRestChannel(
                 new FakeRestRequest.Builder(xContentRegistry()).withContent(BytesArray.EMPTY, randomXContent.type()).build(),
-                randomBoolean(),
+                true,
                 1
             )
         );

server/src/test/java/org/elasticsearch/rest/RestControllerTests.java

@@ -158,7 +158,7 @@ public void testApplyProductSpecificResponseHeaders() {
         final ThreadContext threadContext = client.threadPool().getThreadContext();
         final RestController restController = new RestController(null, null, circuitBreakerService, usageService, telemetryProvider);
         RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).build();
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         restController.dispatchRequest(fakeRequest, channel, threadContext);
         // the rest controller relies on the caller to stash the context, so we should expect these values here as we didn't stash the
         // context in this test
@@ -177,7 +177,7 @@ public void testRequestWithDisallowedMultiValuedHeader() {
         restHeaders.put("header.1", Collections.singletonList("boo"));
         restHeaders.put("header.2", List.of("foo", "bar"));
         RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build();
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         restController.dispatchRequest(fakeRequest, channel, threadContext);
         assertTrue(channel.getSendResponseCalled());
     }
@@ -208,7 +208,7 @@ public String getName() {
                 });
             }
         });
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK);
         spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
         verify(requestsCounter).incrementBy(
             eq(1L),
@@ -232,7 +232,7 @@ public MethodHandlers next() {
                 return null;
             }
         });
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
         verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
     }
@@ -254,7 +254,7 @@ public MethodHandlers next() {
             }
         });
 
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
         verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
     }
@@ -277,7 +277,7 @@ public String getName() {
         }));
         when(spyRestController.getAllHandlers(any(), eq(fakeRequest.rawPath()))).thenAnswer(x -> handlers.iterator());
 
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.METHOD_NOT_ALLOWED);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.METHOD_NOT_ALLOWED);
         spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
         verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 405)));
     }
@@ -287,7 +287,7 @@ public void testDispatchBadRequestEmitsMetric() {
         final RestController restController = new RestController(null, null, circuitBreakerService, usageService, telemetryProvider);
         RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).build();
 
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         restController.dispatchBadRequest(channel, threadContext, new Exception());
         verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
     }
@@ -311,7 +311,7 @@ public MethodHandlers next() {
                 return new MethodHandlers("/").addMethod(GET, RestApiVersion.current(), (request, channel, client) -> {});
             }
         });
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
         restController.dispatchRequest(fakeRequest, channel, threadContext);
         verify(tracer).startTrace(
             eq(threadContext),
@@ -337,7 +337,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() {
                 new RestResponse(RestStatus.OK, RestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)
             )
         );
-        AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK);
+        AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK);
         restController.dispatchRequest(fakeRequest, channel, threadContext);
         assertTrue(channel.getSendResponseCalled());
     }
@@ -801,7 +801,7 @@ public void testFavicon() {
         final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(GET)
             .withPath("/favicon.ico")
             .build();
-        final AssertingChannel channel = new AssertingChannel(fakeRestRequest, false, RestStatus.OK);
+        final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK);
         restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext());
         assertTrue(channel.getSendResponseCalled());
         assertThat(channel.getRestResponse().contentType(), containsString("image/x-icon"));
@@ -1085,7 +1085,7 @@ public void testApiProtectionWithServerlessDisabled() {
         List<String> accessiblePaths = List.of("/public", "/internal", "/hidden");
         accessiblePaths.forEach(path -> {
             RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath(path).build();
-            AssertingChannel channel = new AssertingChannel(request, false, RestStatus.OK);
+            AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK);
             restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY));
         });
     }
@@ -1107,7 +1107,7 @@ public void testApiProtectionWithServerlessEnabledAsEndUser() {
 
         final Consumer<List<String>> checkUnprotected = paths -> paths.forEach(path -> {
             RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath(path).build();
-            AssertingChannel channel = new AssertingChannel(request, false, RestStatus.OK);
+            AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK);
             restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY));
         });
         final Consumer<List<String>> checkProtected = paths -> paths.forEach(path -> {

server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java

@@ -97,7 +97,7 @@ public void testUnsupportedMethodResponseHttpHeader() throws Exception {
         RestRequest restRequest = fakeRestRequestBuilder.build();
 
         // Send the request and verify the response status code
-        FakeRestChannel restChannel = new FakeRestChannel(restRequest, false, 1);
+        FakeRestChannel restChannel = new FakeRestChannel(restRequest, true, 1);
         restController.dispatchRequest(restRequest, restChannel, new ThreadContext(Settings.EMPTY));
         assertThat(restChannel.capturedResponse().status().getStatus(), is(405));