From a00cc1dd4665c8a3889cb0a01ec60c4f94806929 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Mon, 14 Oct 2024 16:46:35 +0100 Subject: [PATCH 1/7] Add a basic deprecation warning that the JSON format is changing in v9 --- .../rest/AbstractRestChannel.java | 12 ++++++++++ .../synonyms/PutSynonymRuleActionTests.java | 2 +- .../synonyms/PutSynonymsActionTests.java | 2 +- .../AbstractHttpServerTransportTests.java | 4 ++-- .../rest/BaseRestHandlerTests.java | 18 +++++++------- .../ChunkedRestResponseBodyPartTests.java | 2 +- .../rest/RestControllerTests.java | 24 +++++++++---------- .../rest/RestHttpResponseHeadersTests.java | 2 +- .../elasticsearch/rest/RestResponseTests.java | 18 ++++++++++++++ .../rest/action/RestBuilderListenerTests.java | 2 +- .../indices/RestValidateQueryActionTests.java | 2 +- .../rest/action/cat/RestTasksActionTests.java | 2 +- .../action/document/RestBulkActionTests.java | 2 +- .../action/search/RestSearchActionTests.java | 2 +- .../scroll/RestClearScrollActionTests.java | 2 +- .../scroll/RestSearchScrollActionTests.java | 2 +- .../test/rest/RestActionTestCase.java | 2 +- .../EnterpriseSearchBaseRestHandlerTests.java | 2 +- .../action/SecurityBaseRestHandlerTests.java | 2 +- .../apikey/ApiKeyBaseRestHandlerTests.java | 2 +- .../apikey/RestCreateApiKeyActionTests.java | 2 +- ...stCreateCrossClusterApiKeyActionTests.java | 2 +- .../apikey/RestGetApiKeyActionTests.java | 6 ++--- .../RestInvalidateApiKeyActionTests.java | 4 ++-- .../apikey/RestQueryApiKeyActionTests.java | 8 +++---- ...stUpdateCrossClusterApiKeyActionTests.java | 2 +- .../oauth2/RestGetTokenActionTests.java | 6 ++--- .../action/user/RestQueryUserActionTests.java | 4 ++-- 28 files changed, 85 insertions(+), 55 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index 1b506b80c5496..5d2ae81caa437 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -13,6 +13,8 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.BytesStream; import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.logging.DeprecationCategory; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.xcontent.ParsedMediaType; import org.elasticsearch.xcontent.XContentBuilder; @@ -32,6 +34,7 @@ public abstract class AbstractRestChannel implements RestChannel { private static final Logger logger = LogManager.getLogger(AbstractRestChannel.class); + private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(AbstractRestChannel.class); private static final Predicate INCLUDE_FILTER = f -> f.charAt(0) != '-'; private static final Predicate EXCLUDE_FILTER = INCLUDE_FILTER.negate(); @@ -61,6 +64,15 @@ protected AbstractRestChannel(RestRequest request, boolean detailedErrorsEnabled this.filterPath = request.param("filter_path", null); this.pretty = request.paramAsBoolean("pretty", false); this.human = request.paramAsBoolean("human", false); + + if (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." + ); + } } @Override diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionTests.java index 0cb4a56794c22..a1b9c59571496 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionTests.java +++ b/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)); diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionTests.java index 54dff48788f52..4dce73fcf0e89 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionTests.java +++ b/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)); diff --git a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java index 9b88ea5536b11..b8b0183ae4fee 100644 --- a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java +++ b/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> 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( diff --git a/server/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java b/server/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java index 9f82911ed121f..8a8bed9ca73db 100644 --- a/server/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/BaseRestHandlerTests.java @@ -73,7 +73,7 @@ public List 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 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 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 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 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 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 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 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 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) diff --git a/server/src/test/java/org/elasticsearch/rest/ChunkedRestResponseBodyPartTests.java b/server/src/test/java/org/elasticsearch/rest/ChunkedRestResponseBodyPartTests.java index eece90ed94cf9..907c16aad5fdc 100644 --- a/server/src/test/java/org/elasticsearch/rest/ChunkedRestResponseBodyPartTests.java +++ b/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 ) ); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 1d946681661e7..cb7e78ef4ae92 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/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 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> 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> checkProtected = paths -> paths.forEach(path -> { diff --git a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java index 3b839896bc34f..4345f3c5e3fb4 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java +++ b/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)); diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java index c65fd85307ece..cfed83f352951 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -93,6 +93,7 @@ public void testWithHeaders() throws Exception { assertThat(response.getHeaders().get("n1"), contains("v11", "v12")); assertThat(response.getHeaders().get("n2"), notNullValue()); assertThat(response.getHeaders().get("n2"), contains("v21", "v22")); + assertChannelWarnings(channel); } public void testEmptyChunkedBody() { @@ -117,6 +118,7 @@ public void testSimpleExceptionMessage() throws Exception { assertThat(text, not(containsString("FileNotFoundException"))); assertThat(text, not(containsString("/foo/bar"))); assertThat(text, not(containsString("error_trace"))); + assertChannelWarnings(channel); } public void testDetailedExceptionMessage() throws Exception { @@ -143,6 +145,7 @@ public void testNonElasticsearchExceptionIsNotShownAsSimpleMessage() throws Exce assertThat(text, not(containsString("FileNotFoundException[/foo/bar]"))); assertThat(text, not(containsString("error_trace"))); assertThat(text, containsString("\"error\":\"No ElasticsearchException found\"")); + assertChannelWarnings(channel); } public void testErrorTrace() throws Exception { @@ -174,6 +177,7 @@ public void testAuthenticationFailedNoStackTrace() throws IOException { RestResponse response = new RestResponse(channel, authnException); assertThat(response.status(), is(RestStatus.UNAUTHORIZED)); assertThat(response.content().utf8ToString(), not(containsString(ElasticsearchException.STACK_TRACE))); + assertChannelWarnings(channel); } } } @@ -198,6 +202,7 @@ public void testStackTrace() throws IOException { } else { assertThat(response.content().utf8ToString(), not(containsString(ElasticsearchException.STACK_TRACE))); } + assertChannelWarnings(channel); } } } @@ -229,6 +234,7 @@ public void testNullThrowable() throws Exception { String text = response.content().utf8ToString(); assertThat(text, containsString("\"error\":\"unknown\"")); assertThat(text, not(containsString("error_trace"))); + assertChannelWarnings(channel); } public void testConvert() throws IOException { @@ -429,6 +435,7 @@ public void testErrorToAndFromXContent() throws IOException { assertEquals(expected.status(), parsedError.status()); assertDeepEquals(expected, parsedError); + assertChannelWarnings(channel); } public void testNoErrorFromXContent() throws IOException { @@ -495,6 +502,7 @@ public void testResponseContentTypeUponException() throws Exception { Exception t = new ElasticsearchException("an error occurred reading data", new FileNotFoundException("/foo/bar")); RestResponse response = new RestResponse(channel, t); assertThat(response.contentType(), equalTo(mediaType)); + assertChannelWarnings(channel); } public void testSupressedLogging() throws IOException { @@ -526,6 +534,7 @@ public void testSupressedLogging() throws IOException { "401", "unauthorized" ); + assertChannelWarnings(channel); } private void assertLogging( @@ -551,6 +560,15 @@ private void assertLogging( } } + private void assertChannelWarnings(RestChannel channel) { + if (channel.detailedErrorsEnabled() == false) { + assertWarnings( + "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." + ); + } + } + public static class WithHeadersException extends ElasticsearchException { WithHeadersException() { diff --git a/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java b/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java index 827a07b89b2b8..4e6c46a7191db 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java @@ -26,7 +26,7 @@ public class RestBuilderListenerTests extends ESTestCase { public void testXContentBuilderClosedInBuildResponse() throws Exception { AtomicReference builderAtomicReference = new AtomicReference<>(); RestBuilderListener builderListener = new RestBuilderListener( - new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 1) + new FakeRestChannel(new FakeRestRequest(), true, 1) ) { @Override public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws Exception { diff --git a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java index 2d719c1ed537d..d4f5e46904fda 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/admin/indices/RestValidateQueryActionTests.java @@ -188,7 +188,7 @@ public void testTypeParameter() { } private void performRequest(RestRequest request) { - RestChannel channel = new FakeRestChannel(request, false, 1); + RestChannel channel = new FakeRestChannel(request, true, 1); ThreadContext threadContext = new ThreadContext(Settings.EMPTY); controller.dispatchRequest(request, channel, threadContext); } diff --git a/server/src/test/java/org/elasticsearch/rest/action/cat/RestTasksActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestTasksActionTests.java index 880a0bc9eabd7..8104ecfc31c3d 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/cat/RestTasksActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/cat/RestTasksActionTests.java @@ -34,7 +34,7 @@ public void testConsumesParameters() throws Exception { FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withParams( Map.of("parent_task_id", "the node:3", "nodes", "node1,node2", "actions", "*") ).build(); - FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, false, 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, true, 1); try (var threadPool = createThreadPool()) { final var nodeClient = buildNodeClient(threadPool); action.handleRequest(fakeRestRequest, fakeRestChannel, nodeClient); diff --git a/server/src/test/java/org/elasticsearch/rest/action/document/RestBulkActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/document/RestBulkActionTests.java index 3b6b280565da5..0d35e4311032d 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/document/RestBulkActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/document/RestBulkActionTests.java @@ -222,7 +222,7 @@ public void next() { }) .withHeaders(Map.of("Content-Type", Collections.singletonList("application/json"))) .build(); - FakeRestChannel channel = new FakeRestChannel(request, false, 1); + FakeRestChannel channel = new FakeRestChannel(request, true, 1); RestBulkAction.ChunkHandler chunkHandler = new RestBulkAction.ChunkHandler( true, diff --git a/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java index e207d150ac6cd..8e424986f04ee 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/search/RestSearchActionTests.java @@ -75,7 +75,7 @@ public void testEnableFieldsEmulationNoErrors() throws Exception { Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader) ).withMethod(RestRequest.Method.GET).withPath("/some_index/_search").withParams(params).build(); - action.handleRequest(request, new FakeRestChannel(request, false, 1), verifyingClient); + action.handleRequest(request, new FakeRestChannel(request, true, 1), verifyingClient); } public void testValidateSearchRequest() { diff --git a/server/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java b/server/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java index 0c95340fdb6f7..33978b4cd6b9f 100644 --- a/server/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java +++ b/server/src/test/java/org/elasticsearch/search/scroll/RestClearScrollActionTests.java @@ -54,7 +54,7 @@ public void clearScroll(ClearScrollRequest request, ActionListener routes() { }; FakeRestRequest fakeRestRequest = new FakeRestRequest(); - FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), isLicensed ? 0 : 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, true, isLicensed ? 0 : 1); try (var threadPool = createThreadPool()) { final var client = new NoOpNodeClient(threadPool); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandlerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandlerTests.java index 5d4ea0f30cb15..8509a6475aa71 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandlerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/SecurityBaseRestHandlerTests.java @@ -58,7 +58,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien } }; FakeRestRequest fakeRestRequest = new FakeRestRequest(); - FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), securityEnabled ? 0 : 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, true, securityEnabled ? 0 : 1); try (var threadPool = createThreadPool()) { final var client = new NoOpNodeClient(threadPool); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/ApiKeyBaseRestHandlerTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/ApiKeyBaseRestHandlerTests.java index 6ff05faf22d11..b734e602ec291 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/ApiKeyBaseRestHandlerTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/ApiKeyBaseRestHandlerTests.java @@ -56,7 +56,7 @@ protected RestChannelConsumer innerPrepareRequest(RestRequest request, NodeClien } }; final var fakeRestRequest = new FakeRestRequest(); - final var fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), requiredSettingsEnabled ? 0 : 1); + final var fakeRestChannel = new FakeRestChannel(fakeRestRequest, true, requiredSettingsEnabled ? 0 : 1); try (var threadPool = createThreadPool()) { final var client = new NoOpNodeClient(threadPool); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateApiKeyActionTests.java index 9a05230d82ae6..79dba637d53d0 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateApiKeyActionTests.java @@ -75,7 +75,7 @@ public void testCreateApiKeyApi() throws Exception { ).withParams(Collections.singletonMap("refresh", randomFrom("false", "true", "wait_for"))).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyActionTests.java index 812354986d5bc..a47855731b37a 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestCreateCrossClusterApiKeyActionTests.java @@ -115,7 +115,7 @@ public void testLicenseEnforcement() throws Exception { } }"""), XContentType.JSON).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java index d88a217cd0949..c65634a76b532 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestGetApiKeyActionTests.java @@ -91,7 +91,7 @@ public void testGetApiKey() throws Exception { final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withParams(params).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -159,7 +159,7 @@ public void testGetApiKeyWithProfileUid() throws Exception { } final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withParams(param).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -224,7 +224,7 @@ public void testGetApiKeyOwnedByCurrentAuthenticatedUser() throws Exception { final FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withParams(param).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java index ac472378d4874..2cb1b6a66b02b 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestInvalidateApiKeyActionTests.java @@ -77,7 +77,7 @@ public void testInvalidateApiKey() throws Exception { ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -144,7 +144,7 @@ public void testInvalidateApiKeyOwnedByCurrentAuthenticatedUser() throws Excepti ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyActionTests.java index d5aa249b1d0f5..7005b5158e626 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestQueryApiKeyActionTests.java @@ -110,7 +110,7 @@ public void testQueryParsing() throws Exception { ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -184,7 +184,7 @@ public void testAggsAndAggregationsTogether() { XContentType.JSON ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -230,7 +230,7 @@ public void testParsingSearchParameters() throws Exception { ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -290,7 +290,7 @@ public void testQueryApiKeyWithProfileUid() throws Exception { } FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withParams(param).build(); SetOnce responseSetOnce = new SetOnce<>(); - RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyActionTests.java index ddeffc0675498..879e1ac8ad157 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/apikey/RestUpdateCrossClusterApiKeyActionTests.java @@ -97,7 +97,7 @@ public void testLicenseEnforcement() throws Exception { XContentType.JSON ).withParams(Map.of("id", randomAlphaOfLength(10))).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/oauth2/RestGetTokenActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/oauth2/RestGetTokenActionTests.java index 2ac33a780313e..bd665560f425f 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/oauth2/RestGetTokenActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/oauth2/RestGetTokenActionTests.java @@ -43,7 +43,7 @@ public class RestGetTokenActionTests extends ESTestCase { public void testListenerHandlesExceptionProperly() { FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final SetOnce responseSetOnce = new SetOnce<>(); - RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -67,7 +67,7 @@ public void sendResponse(RestResponse restResponse) { public void testSendResponse() { FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final SetOnce responseSetOnce = new SetOnce<>(); - RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -114,7 +114,7 @@ public void sendResponse(RestResponse restResponse) { public void testSendResponseKerberosError() { FakeRestRequest restRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); final SetOnce responseSetOnce = new SetOnce<>(); - RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestQueryUserActionTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestQueryUserActionTests.java index 4a593eeb24ac6..38405a2167808 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestQueryUserActionTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/action/user/RestQueryUserActionTests.java @@ -73,7 +73,7 @@ public void testQueryParsing() throws Exception { ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); @@ -132,7 +132,7 @@ public void testParsingSearchParameters() throws Exception { ).build(); final SetOnce responseSetOnce = new SetOnce<>(); - final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, true) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); From d255ce61e4bb19505a2b71b46cea168d2f429d3d Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 6 Nov 2024 14:31:20 +0000 Subject: [PATCH 2/7] Some more test fixes --- .../elasticsearch/rest/action/RestBuilderListenerTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java b/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java index 4e6c46a7191db..03ae366050646 100644 --- a/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/action/RestBuilderListenerTests.java @@ -44,7 +44,7 @@ public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws E public void testXContentBuilderNotClosedInBuildResponseAssertionsDisabled() throws Exception { AtomicReference builderAtomicReference = new AtomicReference<>(); RestBuilderListener builderListener = new RestBuilderListener( - new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 1) + new FakeRestChannel(new FakeRestRequest(), true, 1) ) { @Override public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws Exception { @@ -68,7 +68,7 @@ public void testXContentBuilderNotClosedInBuildResponseAssertionsEnabled() throw assumeTrue("tests are not being run with assertions", RestBuilderListener.class.desiredAssertionStatus()); RestBuilderListener builderListener = new RestBuilderListener( - new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 1) + new FakeRestChannel(new FakeRestRequest(), true, 1) ) { @Override public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws Exception { From 608d9dffbb0e006c1671ad31c3f6f5d0a31e8ad0 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Tue, 12 Nov 2024 17:01:56 +0000 Subject: [PATCH 3/7] Add changelog entry for the deprecation --- docs/changelog/114739.yaml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 docs/changelog/114739.yaml diff --git a/docs/changelog/114739.yaml b/docs/changelog/114739.yaml new file mode 100644 index 0000000000000..4d269f31a3444 --- /dev/null +++ b/docs/changelog/114739.yaml @@ -0,0 +1,17 @@ +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: Infra/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 for v8 and below, the format of this response + changes significantly. + In Elasticsearch v9, the JSON format 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). + impact: "If you have set `http.detailed_errors.enabled: false` (the default is `true`)\ + \ the structure of JSON when any exceptions occur will change on Elasticsearch 9.0 + notable: false From 82f23e1d3b016e9ada387968cfcce28b63b1a610 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 09:43:15 +0000 Subject: [PATCH 4/7] Only log deprecation warning when error actually happens --- .../org/elasticsearch/rest/AbstractRestChannel.java | 12 ------------ .../java/org/elasticsearch/rest/RestResponse.java | 13 +++++++++++++ 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index 5d2ae81caa437..1b506b80c5496 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -13,8 +13,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.BytesStream; import org.elasticsearch.common.io.stream.BytesStreamOutput; -import org.elasticsearch.common.logging.DeprecationCategory; -import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.xcontent.ParsedMediaType; import org.elasticsearch.xcontent.XContentBuilder; @@ -34,7 +32,6 @@ public abstract class AbstractRestChannel implements RestChannel { private static final Logger logger = LogManager.getLogger(AbstractRestChannel.class); - private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(AbstractRestChannel.class); private static final Predicate INCLUDE_FILTER = f -> f.charAt(0) != '-'; private static final Predicate EXCLUDE_FILTER = INCLUDE_FILTER.negate(); @@ -64,15 +61,6 @@ protected AbstractRestChannel(RestRequest request, boolean detailedErrorsEnabled this.filterPath = request.param("filter_path", null); this.pretty = request.paramAsBoolean("pretty", false); this.human = request.paramAsBoolean("human", false); - - if (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." - ); - } } @Override diff --git a/server/src/main/java/org/elasticsearch/rest/RestResponse.java b/server/src/main/java/org/elasticsearch/rest/RestResponse.java index fd8b90a99e7f6..a5a0d9ed26b03 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestResponse.java +++ b/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()) { + 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); From 4580d7f12ada5691090e3e7e01ea790b9b0e5279 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 10:24:12 +0000 Subject: [PATCH 5/7] Update changelog --- docs/changelog/114739.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/changelog/114739.yaml b/docs/changelog/114739.yaml index 4d269f31a3444..44ebd79f92a5e 100644 --- a/docs/changelog/114739.yaml +++ b/docs/changelog/114739.yaml @@ -5,13 +5,14 @@ type: deprecation issues: [89387] deprecation: title: The format of non-detailed error responses is changing in v9 - area: Infra/REST API + 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 for v8 and below, the format of this response changes significantly. In Elasticsearch v9, the JSON format 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). - impact: "If you have set `http.detailed_errors.enabled: false` (the default is `true`)\ - \ the structure of JSON when any exceptions occur will change on Elasticsearch 9.0 + impact: |- + If you have set `http.detailed_errors.enabled: false` (the default is `true`) + the structure of JSON when any exceptions occur will change on Elasticsearch 9.0 notable: false From 879e98454747eddab65187d44795cdcd21873729 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 10:37:35 +0000 Subject: [PATCH 6/7] Get the check the right way round --- server/src/main/java/org/elasticsearch/rest/RestResponse.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestResponse.java b/server/src/main/java/org/elasticsearch/rest/RestResponse.java index a5a0d9ed26b03..29cae343fb09e 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/RestResponse.java @@ -146,7 +146,7 @@ public RestResponse(RestChannel channel, RestStatus status, Exception e) throws params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params); } - if (channel.detailedErrorsEnabled()) { + if (channel.detailedErrorsEnabled() == false) { deprecationLogger.warn( DeprecationCategory.API, "http_detailed_errors", From c77a7e13778de34679ed648c172c8a2743eada0f Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:08:07 +0000 Subject: [PATCH 7/7] Update changelog --- docs/changelog/114739.yaml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/changelog/114739.yaml b/docs/changelog/114739.yaml index 44ebd79f92a5e..16660e0a07e71 100644 --- a/docs/changelog/114739.yaml +++ b/docs/changelog/114739.yaml @@ -8,11 +8,13 @@ deprecation: 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 for v8 and below, the format of this response - changes significantly. - In Elasticsearch v9, the JSON format of responses with errors when the `http.detailed_errors.enabled: false` option is set + 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 on Elasticsearch 9.0 + 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