Skip to content

Commit bd091d3

Browse files
thecoopthecoop
authored
Add a deprecation warning that the JSON format of non-detailed errors is changing in v9 (elastic#116330)
1 parent 6325e46 commit bd091d3

27 files changed

+87
-56
lines changed

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.elasticsearch.ExceptionsHelper;
1717
import org.elasticsearch.common.bytes.BytesArray;
1818
import org.elasticsearch.common.bytes.BytesReference;
19+
import org.elasticsearch.common.logging.DeprecationCategory;
20+
import org.elasticsearch.common.logging.DeprecationLogger;
1921
import org.elasticsearch.common.util.Maps;
2022
import org.elasticsearch.core.Nullable;
2123
import org.elasticsearch.core.Releasable;
@@ -43,6 +45,7 @@ public final class RestResponse implements Releasable {
4345
static final String STATUS = "status";
4446

4547
private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed");
48+
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(AbstractRestChannel.class);
4649

4750
private final RestStatus status;
4851

@@ -142,6 +145,16 @@ public RestResponse(RestChannel channel, RestStatus status, Exception e) throws
142145
if (params.paramAsBoolean("error_trace", false) && status != RestStatus.UNAUTHORIZED) {
143146
params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params);
144147
}
148+
149+
if (channel.detailedErrorsEnabled() == false) {
150+
deprecationLogger.warn(
151+
DeprecationCategory.API,
152+
"http_detailed_errors",
153+
"The JSON format of non-detailed errors will change in Elasticsearch 9.0 to match the JSON structure"
154+
+ " used for detailed errors. To keep using the existing format, use the V8 REST API."
155+
);
156+
}
157+
145158
try (XContentBuilder builder = channel.newErrorBuilder()) {
146159
build(builder, params, status, channel.detailedErrorsEnabled(), e);
147160
this.content = BytesReference.bytes(builder);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void testEmptyRequestBody() throws Exception {
2626
.withParams(Map.of("synonymsSet", "testSet", "synonymRuleId", "testRule"))
2727
.build();
2828

29-
FakeRestChannel channel = new FakeRestChannel(request, false, 0);
29+
FakeRestChannel channel = new FakeRestChannel(request, true, 0);
3030
try (var threadPool = createThreadPool()) {
3131
final var nodeClient = new NoOpNodeClient(threadPool);
3232
expectThrows(IllegalArgumentException.class, () -> action.handleRequest(request, channel, nodeClient));

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ public void testEmptyRequestBody() throws Exception {
2626
.withParams(Map.of("synonymsSet", "test"))
2727
.build();
2828

29-
FakeRestChannel channel = new FakeRestChannel(request, false, 0);
29+
FakeRestChannel channel = new FakeRestChannel(request, true, 0);
3030
try (var threadPool = createThreadPool()) {
3131
final var nodeClient = new NoOpNodeClient(threadPool);
3232
expectThrows(IllegalArgumentException.class, () -> action.handleRequest(request, channel, nodeClient));

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
271271
final RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build();
272272
final RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel(
273273
fakeRequest,
274-
false,
274+
true,
275275
RestStatus.BAD_REQUEST
276276
);
277277

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

366366
try (
367367
AbstractHttpServerTransport transport = new AbstractHttpServerTransport(

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public List<Route> routes() {
7373
params.put("consumed", randomAlphaOfLength(8));
7474
params.put("unconsumed", randomAlphaOfLength(8));
7575
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
76-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
76+
RestChannel channel = new FakeRestChannel(request, true, 1);
7777
final IllegalArgumentException e = expectThrows(
7878
IllegalArgumentException.class,
7979
() -> handler.handleRequest(request, channel, mockClient)
@@ -108,7 +108,7 @@ public List<Route> routes() {
108108
params.put("unconsumed-first", randomAlphaOfLength(8));
109109
params.put("unconsumed-second", randomAlphaOfLength(8));
110110
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
111-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
111+
RestChannel channel = new FakeRestChannel(request, true, 1);
112112
final IllegalArgumentException e = expectThrows(
113113
IllegalArgumentException.class,
114114
() -> handler.handleRequest(request, channel, mockClient)
@@ -155,7 +155,7 @@ public List<Route> routes() {
155155
params.put("very_close_to_parametre", randomAlphaOfLength(8));
156156
params.put("very_far_from_every_consumed_parameter", randomAlphaOfLength(8));
157157
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
158-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
158+
RestChannel channel = new FakeRestChannel(request, true, 1);
159159
final IllegalArgumentException e = expectThrows(
160160
IllegalArgumentException.class,
161161
() -> handler.handleRequest(request, channel, mockClient)
@@ -206,7 +206,7 @@ public List<Route> routes() {
206206
params.put("consumed", randomAlphaOfLength(8));
207207
params.put("response_param", randomAlphaOfLength(8));
208208
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
209-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
209+
RestChannel channel = new FakeRestChannel(request, true, 1);
210210
handler.handleRequest(request, channel, mockClient);
211211
assertTrue(restChannelConsumer.executed);
212212
assertTrue(restChannelConsumer.closed);
@@ -238,7 +238,7 @@ public List<Route> routes() {
238238
params.put("human", null);
239239
params.put("error_trace", randomFrom("true", "false", null));
240240
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
241-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
241+
RestChannel channel = new FakeRestChannel(request, true, 1);
242242
handler.handleRequest(request, channel, mockClient);
243243
assertTrue(restChannelConsumer.executed);
244244
assertTrue(restChannelConsumer.closed);
@@ -283,7 +283,7 @@ public List<Route> routes() {
283283
params.put("size", randomAlphaOfLength(8));
284284
params.put("time", randomAlphaOfLength(8));
285285
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withParams(params).build();
286-
RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
286+
RestChannel channel = new FakeRestChannel(request, true, 1);
287287
handler.handleRequest(request, channel, mockClient);
288288
assertTrue(restChannelConsumer.executed);
289289
assertTrue(restChannelConsumer.closed);
@@ -314,7 +314,7 @@ public List<Route> routes() {
314314
new BytesArray(builder.toString()),
315315
XContentType.JSON
316316
).build();
317-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
317+
final RestChannel channel = new FakeRestChannel(request, true, 1);
318318
handler.handleRequest(request, channel, mockClient);
319319
assertTrue(restChannelConsumer.executed);
320320
assertTrue(restChannelConsumer.closed);
@@ -341,7 +341,7 @@ public List<Route> routes() {
341341
};
342342

343343
final RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).build();
344-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
344+
final RestChannel channel = new FakeRestChannel(request, true, 1);
345345
handler.handleRequest(request, channel, mockClient);
346346
assertTrue(restChannelConsumer.executed);
347347
assertTrue(restChannelConsumer.closed);
@@ -371,7 +371,7 @@ public List<Route> routes() {
371371
new BytesArray(builder.toString()),
372372
XContentType.JSON
373373
).build();
374-
final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 1);
374+
final RestChannel channel = new FakeRestChannel(request, true, 1);
375375
final IllegalArgumentException e = expectThrows(
376376
IllegalArgumentException.class,
377377
() -> handler.handleRequest(request, channel, mockClient)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ public void testEncodesChunkedXContentCorrectly() throws IOException {
5656
ToXContent.EMPTY_PARAMS,
5757
new FakeRestChannel(
5858
new FakeRestRequest.Builder(xContentRegistry()).withContent(BytesArray.EMPTY, randomXContent.type()).build(),
59-
randomBoolean(),
59+
true,
6060
1
6161
)
6262
);

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ public void testApplyProductSpecificResponseHeaders() {
161161
final ThreadContext threadContext = client.threadPool().getThreadContext();
162162
final RestController restController = new RestController(null, null, circuitBreakerService, usageService, telemetryProvider);
163163
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).build();
164-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
164+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
165165
restController.dispatchRequest(fakeRequest, channel, threadContext);
166166
// the rest controller relies on the caller to stash the context, so we should expect these values here as we didn't stash the
167167
// context in this test
@@ -180,7 +180,7 @@ public void testRequestWithDisallowedMultiValuedHeader() {
180180
restHeaders.put("header.1", Collections.singletonList("boo"));
181181
restHeaders.put("header.2", List.of("foo", "bar"));
182182
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build();
183-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
183+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
184184
restController.dispatchRequest(fakeRequest, channel, threadContext);
185185
assertTrue(channel.getSendResponseCalled());
186186
}
@@ -211,7 +211,7 @@ public String getName() {
211211
});
212212
}
213213
});
214-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK);
214+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK);
215215
spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
216216
verify(requestsCounter).incrementBy(
217217
eq(1L),
@@ -235,7 +235,7 @@ public MethodHandlers next() {
235235
return null;
236236
}
237237
});
238-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
238+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
239239
spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
240240
verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
241241
}
@@ -257,7 +257,7 @@ public MethodHandlers next() {
257257
}
258258
});
259259

260-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
260+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
261261
spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
262262
verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
263263
}
@@ -280,7 +280,7 @@ public String getName() {
280280
}));
281281
when(spyRestController.getAllHandlers(any(), eq(fakeRequest.rawPath()))).thenAnswer(x -> handlers.iterator());
282282

283-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.METHOD_NOT_ALLOWED);
283+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.METHOD_NOT_ALLOWED);
284284
spyRestController.dispatchRequest(fakeRequest, channel, threadContext);
285285
verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 405)));
286286
}
@@ -290,7 +290,7 @@ public void testDispatchBadRequestEmitsMetric() {
290290
final RestController restController = new RestController(null, null, circuitBreakerService, usageService, telemetryProvider);
291291
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).build();
292292

293-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
293+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
294294
restController.dispatchBadRequest(channel, threadContext, new Exception());
295295
verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400)));
296296
}
@@ -314,7 +314,7 @@ public MethodHandlers next() {
314314
return new MethodHandlers("/").addMethod(GET, RestApiVersion.current(), (request, channel, client) -> {});
315315
}
316316
});
317-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST);
317+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST);
318318
restController.dispatchRequest(fakeRequest, channel, threadContext);
319319
verify(tracer).startTrace(
320320
eq(threadContext),
@@ -340,7 +340,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() {
340340
new RestResponse(RestStatus.OK, RestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)
341341
)
342342
);
343-
AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK);
343+
AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK);
344344
restController.dispatchRequest(fakeRequest, channel, threadContext);
345345
assertTrue(channel.getSendResponseCalled());
346346
}
@@ -831,7 +831,7 @@ public void testFavicon() {
831831
final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(GET)
832832
.withPath("/favicon.ico")
833833
.build();
834-
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, false, RestStatus.OK);
834+
final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK);
835835
restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext());
836836
assertTrue(channel.getSendResponseCalled());
837837
assertThat(channel.getRestResponse().contentType(), containsString("image/x-icon"));
@@ -1115,7 +1115,7 @@ public void testApiProtectionWithServerlessDisabled() {
11151115
List<String> accessiblePaths = List.of("/public", "/internal", "/hidden");
11161116
accessiblePaths.forEach(path -> {
11171117
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath(path).build();
1118-
AssertingChannel channel = new AssertingChannel(request, false, RestStatus.OK);
1118+
AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK);
11191119
restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY));
11201120
});
11211121
}
@@ -1137,7 +1137,7 @@ public void testApiProtectionWithServerlessEnabledAsEndUser() {
11371137

11381138
final Consumer<List<String>> checkUnprotected = paths -> paths.forEach(path -> {
11391139
RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath(path).build();
1140-
AssertingChannel channel = new AssertingChannel(request, false, RestStatus.OK);
1140+
AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK);
11411141
restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY));
11421142
});
11431143
final Consumer<List<String>> checkProtected = paths -> paths.forEach(path -> {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ public void testUnsupportedMethodResponseHttpHeader() throws Exception {
9797
RestRequest restRequest = fakeRestRequestBuilder.build();
9898

9999
// Send the request and verify the response status code
100-
FakeRestChannel restChannel = new FakeRestChannel(restRequest, false, 1);
100+
FakeRestChannel restChannel = new FakeRestChannel(restRequest, true, 1);
101101
restController.dispatchRequest(restRequest, restChannel, new ThreadContext(Settings.EMPTY));
102102
assertThat(restChannel.capturedResponse().status().getStatus(), is(405));
103103

0 commit comments

Comments
 (0)