Skip to content
Merged
18 changes: 18 additions & 0 deletions docs/changelog/114739.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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 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
13 changes: 13 additions & 0 deletions server/src/main/java/org/elasticsearch/rest/RestResponse.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);

Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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());
}
Expand Down Expand Up @@ -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),
Expand All @@ -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)));
}
Expand All @@ -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)));
}
Expand All @@ -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)));
}
Expand All @@ -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)));
}
Expand All @@ -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),
Expand All @@ -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());
}
Expand Down Expand Up @@ -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"));
Expand Down Expand Up @@ -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));
});
}
Expand All @@ -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 -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
Loading