From b0c068262efb0577390972126f208a2930e028f6 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 29 Sep 2022 16:12:50 +0100 Subject: [PATCH 01/28] Output a consistent format when generating error json Now, error fields will always have 'type' and 'reason' fields, and the information in those fields is the same regardless of whether the output is detailed or not --- .../elasticsearch/ElasticsearchException.java | 30 ++--- .../ElasticsearchExceptionTests.java | 124 +++++++++++++++--- 2 files changed, 117 insertions(+), 37 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 51718a226173d..e121ea54b068c 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -422,7 +422,7 @@ public static ElasticsearchException innerFromXContent(XContentParser parser, bo if (TYPE.equals(currentFieldName)) { type = parser.text(); } else if (REASON.equals(currentFieldName)) { - reason = parser.text(); + reason = parser.textOrNull(); } else if (STACK_TRACE.equals(currentFieldName)) { stack = parser.text(); } else if (token == XContentParser.Token.VALUE_STRING) { @@ -543,24 +543,22 @@ public static void generateThrowableXContent(XContentBuilder builder, Params par */ public static void generateFailureXContent(XContentBuilder builder, Params params, @Nullable Exception e, boolean detailed) throws IOException { - // No exception to render as an error if (e == null) { - builder.field(ERROR, "unknown"); + // No exception to render as an error + builder.startObject(ERROR); + builder.field(TYPE, "unknown"); + builder.field(REASON, "unknown"); + builder.endObject(); return; } - // Render the exception with a simple message if (detailed == false) { - String message = "No ElasticsearchException found"; - Throwable t = e; - for (int counter = 0; counter < 10 && t != null; counter++) { - if (t instanceof ElasticsearchException) { - message = t.getClass().getSimpleName() + "[" + t.getMessage() + "]"; - break; - } - t = t.getCause(); - } - builder.field(ERROR, message); + // just render the type & message + Throwable t = ExceptionsHelper.unwrapCause(e); + builder.startObject(ERROR); + builder.field(TYPE, getExceptionName(t)); + builder.field(REASON, t.getMessage()); + builder.endObject(); return; } @@ -663,8 +661,8 @@ public static String getExceptionName(Throwable ex) { static String buildMessage(String type, String reason, String stack) { StringBuilder message = new StringBuilder("Elasticsearch exception ["); - message.append(TYPE).append('=').append(type).append(", "); - message.append(REASON).append('=').append(reason); + message.append(TYPE).append('=').append(type); + message.append(", ").append(REASON).append('=').append(reason); if (stack != null) { message.append(", ").append(STACK_TRACE).append('=').append(stack); } diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index d9f9bad00522d..2cf1236ede62a 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -505,12 +505,12 @@ public void testGetDetailedMessage() { public void testToXContent() throws IOException { { ElasticsearchException e = new ElasticsearchException("test"); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ {"type":"exception","reason":"test"}"""); } { ElasticsearchException e = new IndexShardRecoveringException(new ShardId("_test", "_0", 5)); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ { "type": "index_shard_recovering_exception", "reason": "CurrentState[RECOVERING] Already recovering", @@ -525,7 +525,7 @@ public void testToXContent() throws IOException { "foo", new IllegalStateException("bar") ); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ { "type": "illegal_state_exception", "reason": "bar" @@ -533,7 +533,7 @@ public void testToXContent() throws IOException { } { ElasticsearchException e = new ElasticsearchException(new IllegalArgumentException("foo")); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ { "type": "exception", "reason": "java.lang.IllegalArgumentException: foo", @@ -546,7 +546,7 @@ public void testToXContent() throws IOException { { ElasticsearchException e = new SearchParseException(SHARD_TARGET, "foo", new XContentLocation(1, 0)); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ {"type":"search_parse_exception","reason":"foo","line":1,"col":0}"""); } { @@ -554,7 +554,7 @@ public void testToXContent() throws IOException { "foo", new ElasticsearchException("bar", new IllegalArgumentException("index is closed", new RuntimeException("foobar"))) ); - assertExceptionAsJson(ex, """ + assertThrowableAsJson(ex, """ { "type": "exception", "reason": "foo", @@ -575,7 +575,7 @@ public void testToXContent() throws IOException { { ElasticsearchException e = new ElasticsearchException("foo", new IllegalStateException("bar")); - assertExceptionAsJson(e, """ + assertThrowableAsJson(e, """ { "type": "exception", "reason": "foo", @@ -610,6 +610,85 @@ public void testToXContent() throws IOException { } } + public void testGenerateFailureToXContentWithNoDetails() throws IOException { + { + Exception ex; + if (randomBoolean()) { + // just a wrapper which is omitted + ex = new RemoteTransportException("foobar", new FileNotFoundException("foo not found")); + } else { + ex = new FileNotFoundException("foo not found"); + } + assertFailureAsJson(ex, """ + {"error":{"type":"file_not_found_exception","reason":"foo not found"}}""", false); + } + { + ParsingException ex = new ParsingException(1, 2, "foobar", null); + assertFailureAsJson(ex, """ + {"error":{"type":"parsing_exception","reason":"foobar"}}""", false); + } + + { // render header and metadata + ParsingException ex = new ParsingException(1, 2, "foobar", null); + ex.addMetadata("es.test1", "value1"); + ex.addMetadata("es.test2", "value2"); + ex.addHeader("test", "some value"); + ex.addHeader("test_multi", "some value", "another value"); + + String expected = """ + {"error":{"type": "parsing_exception","reason": "foobar"}}"""; + assertFailureAsJson(ex, expected, false); + } + } + + public void testGenerateFailureToXContentWithDetails() throws IOException { + { + Exception ex; + if (randomBoolean()) { + // just a wrapper which is omitted + ex = new RemoteTransportException("foobar", new FileNotFoundException("foo not found")); + } else { + ex = new FileNotFoundException("foo not found"); + } + assertFailureAsJson(ex, """ + {"error":{"type":"file_not_found_exception","reason":"foo not found", + "root_cause":[{"type":"file_not_found_exception","reason":"foo not found"}]}}""", true); + } + { + ParsingException ex = new ParsingException(1, 2, "foobar", null); + assertFailureAsJson(ex, """ + {"error":{"type":"parsing_exception","reason":"foobar","line":1,"col":2, + "root_cause":[{"type":"parsing_exception","reason":"foobar","line":1,"col":2}]}}""", true); + } + + { // render header and metadata + ParsingException ex = new ParsingException(1, 2, "foobar", null); + ex.addMetadata("es.test1", "value1"); + ex.addMetadata("es.test2", "value2"); + ex.addHeader("test", "some value"); + ex.addHeader("test_multi", "some value", "another value"); + + String expectedFragment = """ + { + "type": "parsing_exception", + "reason": "foobar", + "line": 1, + "col": 2, + "test1": "value1", + "test2": "value2", + "header": { + "test_multi": [ + "some value", + "another value" + ], + "test": "some value" + } + """; + String expected = "{\"error\":" + expectedFragment + ",\"root_cause\":[" + expectedFragment + "}]}}"; + assertFailureAsJson(ex, expected, true); + } + } + public void testGenerateThrowableToXContent() throws IOException { { Exception ex; @@ -619,12 +698,12 @@ public void testGenerateThrowableToXContent() throws IOException { } else { ex = new FileNotFoundException("foo not found"); } - assertExceptionAsJson(ex, """ + assertThrowableAsJson(ex, """ {"type":"file_not_found_exception","reason":"foo not found"}"""); } { ParsingException ex = new ParsingException(1, 2, "foobar", null); - assertExceptionAsJson(ex, """ + assertThrowableAsJson(ex, """ {"type":"parsing_exception","reason":"foobar","line":1,"col":2}"""); } @@ -664,7 +743,7 @@ public void testGenerateThrowableToXContent() throws IOException { "test": "some value" } }"""; - assertExceptionAsJson(ex, expected); + assertThrowableAsJson(ex, expected); } } @@ -705,7 +784,7 @@ public void testToXContentWithHeadersAndMetadata() throws IOException { } }"""; - assertExceptionAsJson(e, expectedJson); + assertThrowableAsJson(e, expectedJson); ElasticsearchException parsed; try (XContentParser parser = createParser(XContentType.JSON.xContent(), expectedJson)) { @@ -825,7 +904,7 @@ public void testFromXContentWithHeadersAndMetadata() throws IOException { } assertNotNull(parsed); - assertEquals(parsed.getMessage(), "Elasticsearch exception [type=exception, reason=foo]"); + assertEquals("Elasticsearch exception [type=exception, reason=foo]", parsed.getMessage()); assertThat(parsed.getHeaderKeys(), hasSize(1)); assertThat(parsed.getHeader("foo_1"), hasItem("foo1")); assertThat(parsed.getMetadataKeys(), hasSize(1)); @@ -978,7 +1057,7 @@ public void testUnknownFailureToAndFromXContent() throws IOException { } // Failure was null, expecting a "unknown" reason - assertEquals("Elasticsearch exception [type=exception, reason=unknown]", parsedFailure.getMessage()); + assertEquals("Elasticsearch exception [type=unknown, reason=unknown]", parsedFailure.getMessage()); assertEquals(0, parsedFailure.getHeaders().size()); assertEquals(0, parsedFailure.getMetadata().size()); } @@ -1006,13 +1085,9 @@ public void testFailureToAndFromXContentWithNoDetails() throws IOException { } assertNotNull(parsedFailure); - String reason; - if (failure instanceof ElasticsearchException) { - reason = failure.getClass().getSimpleName() + "[" + failure.getMessage() + "]"; - } else { - reason = "No ElasticsearchException found"; - } - assertEquals(ElasticsearchException.buildMessage("exception", reason, null), parsedFailure.getMessage()); + String type = ElasticsearchException.getExceptionName(failure); + String reason = failure.getMessage(); + assertEquals(ElasticsearchException.buildMessage(type, reason, null), parsedFailure.getMessage()); assertEquals(0, parsedFailure.getHeaders().size()); assertEquals(0, parsedFailure.getMetadata().size()); assertNull(parsedFailure.getCause()); @@ -1163,13 +1238,20 @@ private static void assertToXContentAsJson(ToXContent e, String expectedJson) th assertToXContentEquivalent(new BytesArray(expectedJson), actual, XContentType.JSON); } - private static void assertExceptionAsJson(Exception e, String expectedJson) throws IOException { + private static void assertThrowableAsJson(Exception e, String expectedJson) throws IOException { assertToXContentAsJson((builder, params) -> { ElasticsearchException.generateThrowableXContent(builder, params, e); return builder; }, expectedJson); } + private static void assertFailureAsJson(Exception e, String expectedJson, boolean detailed) throws IOException { + assertToXContentAsJson((builder, params) -> { + ElasticsearchException.generateFailureXContent(builder, params, e, detailed); + return builder; + }, expectedJson); + } + public static void assertDeepEquals(ElasticsearchException expected, ElasticsearchException actual) { do { if (expected == null) { From 4cd9a4edb97390e44b1f966e5266c5071e03e5ed Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 29 Sep 2022 16:23:57 +0100 Subject: [PATCH 02/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/90529.yaml diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml new file mode 100644 index 0000000000000..25a663c541858 --- /dev/null +++ b/docs/changelog/90529.yaml @@ -0,0 +1,5 @@ +pr: 90529 +summary: Output a consistent format when generating error json +area: Infra/REST API +type: bug +issues: [] From a731fa6e41d8a4b5b8af33e80b2554236b3ba417 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 29 Sep 2022 16:31:09 +0100 Subject: [PATCH 03/28] Update changelog with issue number --- docs/changelog/90529.yaml | 2 +- .../src/main/java/org/elasticsearch/ElasticsearchException.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 25a663c541858..7b3fe06d7ba02 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -2,4 +2,4 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API type: bug -issues: [] +issues: [89387] diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index e121ea54b068c..58a6a84a5af17 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -422,7 +422,7 @@ public static ElasticsearchException innerFromXContent(XContentParser parser, bo if (TYPE.equals(currentFieldName)) { type = parser.text(); } else if (REASON.equals(currentFieldName)) { - reason = parser.textOrNull(); + reason = parser.text(); } else if (STACK_TRACE.equals(currentFieldName)) { stack = parser.text(); } else if (token == XContentParser.Token.VALUE_STRING) { From cec444ab1f5ccaefa6fda89084c79595e9de6c5f Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 29 Sep 2022 16:56:12 +0100 Subject: [PATCH 04/28] Update RestResponseTests --- .../elasticsearch/rest/RestResponseTests.java | 56 +++++-------------- 1 file changed, 14 insertions(+), 42 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java index 57b456642650c..1d842cbfdb867 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -66,7 +66,7 @@ public void testSimpleExceptionMessage() throws Exception { Exception t = new ElasticsearchException("an error occurred reading data", new FileNotFoundException("/foo/bar")); RestResponse response = new RestResponse(channel, t); String text = response.content().utf8ToString(); - assertThat(text, containsString("ElasticsearchException[an error occurred reading data]")); + assertThat(text, containsString("an error occurred reading data")); assertThat(text, not(containsString("FileNotFoundException"))); assertThat(text, not(containsString("/foo/bar"))); assertThat(text, not(containsString("error_trace"))); @@ -85,19 +85,6 @@ public void testDetailedExceptionMessage() throws Exception { {"type":"file_not_found_exception","reason":"/foo/bar"}""")); } - public void testNonElasticsearchExceptionIsNotShownAsSimpleMessage() throws Exception { - RestRequest request = new FakeRestRequest(); - RestChannel channel = new SimpleExceptionRestChannel(request); - - Exception t = new UnknownException("an error occurred reading data", new FileNotFoundException("/foo/bar")); - RestResponse response = new RestResponse(channel, t); - String text = response.content().utf8ToString(); - assertThat(text, not(containsString("UnknownException[an error occurred reading data]"))); - assertThat(text, not(containsString("FileNotFoundException[/foo/bar]"))); - assertThat(text, not(containsString("error_trace"))); - assertThat(text, containsString("\"error\":\"No ElasticsearchException found\"")); - } - public void testErrorTrace() throws Exception { RestRequest request = new FakeRestRequest(); request.params().put("error_trace", "true"); @@ -138,7 +125,8 @@ public void testNullThrowable() throws Exception { RestResponse response = new RestResponse(channel, null); String text = response.content().utf8ToString(); - assertThat(text, containsString("\"error\":\"unknown\"")); + assertThat(text, containsString("\"type\":\"unknown\"")); + assertThat(text, containsString("\"reason\":\"unknown\"")); assertThat(text, not(containsString("error_trace"))); } @@ -229,32 +217,26 @@ public void testErrorToAndFromXContent() throws IOException { original = new ElasticsearchException("ElasticsearchException without cause"); if (detailed) { addHeadersOrMetadata = randomBoolean(); - reason = "ElasticsearchException without cause"; - } else { - reason = "ElasticsearchException[ElasticsearchException without cause]"; } + reason = "ElasticsearchException without cause"; } case 1 -> { original = new ElasticsearchException("ElasticsearchException with a cause", new FileNotFoundException("missing")); if (detailed) { addHeadersOrMetadata = randomBoolean(); - type = "exception"; - reason = "ElasticsearchException with a cause"; cause = new ElasticsearchException("Elasticsearch exception [type=file_not_found_exception, reason=missing]"); - } else { - reason = "ElasticsearchException[ElasticsearchException with a cause]"; } + type = "exception"; + reason = "ElasticsearchException with a cause"; } case 2 -> { original = new ResourceNotFoundException("ElasticsearchException with custom status"); status = RestStatus.NOT_FOUND; if (detailed) { addHeadersOrMetadata = randomBoolean(); - type = "resource_not_found_exception"; - reason = "ElasticsearchException with custom status"; - } else { - reason = "ResourceNotFoundException[ElasticsearchException with custom status]"; } + type = "resource_not_found_exception"; + reason = "ElasticsearchException with custom status"; } case 3 -> { TransportAddress address = buildNewFakeTransportAddress(); @@ -265,12 +247,8 @@ public void testErrorToAndFromXContent() throws IOException { new ResourceAlreadyExistsException("ElasticsearchWrapperException with a cause that has a custom status") ); status = RestStatus.BAD_REQUEST; - if (detailed) { - type = "resource_already_exists_exception"; - reason = "ElasticsearchWrapperException with a cause that has a custom status"; - } else { - reason = "RemoteTransportException[[remote][" + address.toString() + "][action]]"; - } + type = "resource_already_exists_exception"; + reason = "ElasticsearchWrapperException with a cause that has a custom status"; } case 4 -> { original = new RemoteTransportException( @@ -278,23 +256,17 @@ public void testErrorToAndFromXContent() throws IOException { new IllegalArgumentException("wrong") ); status = RestStatus.BAD_REQUEST; - if (detailed) { - type = "illegal_argument_exception"; - reason = "wrong"; - } else { - reason = "RemoteTransportException[[ElasticsearchWrapperException with a cause that has a special treatment]]"; - } + type = "illegal_argument_exception"; + reason = "wrong"; } case 5 -> { status = randomFrom(RestStatus.values()); original = new ElasticsearchStatusException("ElasticsearchStatusException with random status", status); if (detailed) { addHeadersOrMetadata = randomBoolean(); - type = "status_exception"; - reason = "ElasticsearchStatusException with random status"; - } else { - reason = "ElasticsearchStatusException[ElasticsearchStatusException with random status]"; } + type = "status_exception"; + reason = "ElasticsearchStatusException with random status"; } default -> throw new UnsupportedOperationException("Failed to generate random exception"); } From f3d60c83e1f6067162c2c0d29c266031153a1222 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 30 Sep 2022 09:57:48 +0100 Subject: [PATCH 05/28] PR comments --- .../ElasticsearchExceptionTests.java | 29 +++++++------------ 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 2cf1236ede62a..602fdb22cf457 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -612,12 +612,9 @@ public void testToXContent() throws IOException { public void testGenerateFailureToXContentWithNoDetails() throws IOException { { - Exception ex; - if (randomBoolean()) { - // just a wrapper which is omitted - ex = new RemoteTransportException("foobar", new FileNotFoundException("foo not found")); - } else { - ex = new FileNotFoundException("foo not found"); + Exception ex = new FileNotFoundException("foo not found"); + for (int i=0; i Date: Fri, 30 Sep 2022 10:32:31 +0100 Subject: [PATCH 06/28] spotless --- .../java/org/elasticsearch/ElasticsearchExceptionTests.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 602fdb22cf457..5530cf5afb0e1 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -613,7 +613,7 @@ public void testToXContent() throws IOException { public void testGenerateFailureToXContentWithNoDetails() throws IOException { { Exception ex = new FileNotFoundException("foo not found"); - for (int i=0; i Date: Fri, 30 Sep 2022 11:17:49 +0100 Subject: [PATCH 07/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 7b3fe06d7ba02..25a663c541858 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -2,4 +2,4 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API type: bug -issues: [89387] +issues: [] From 75326932f22592a8ba6edb8bad0601275ce0c513 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 30 Sep 2022 11:27:38 +0100 Subject: [PATCH 08/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 25a663c541858..91969a00a48e7 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -2,4 +2,5 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API type: bug -issues: [] +issues: + - 89387 From 1e75fce8af2855bada1aba54ad70880c7ad46eb2 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 30 Sep 2022 11:34:25 +0100 Subject: [PATCH 09/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 91969a00a48e7..efa5aa15ca4e8 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,6 +1,13 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: bug +type: "breaking, bug" issues: - 89387 +breaking: + title: Output a consistent format when generating error json + area: Infra/REST API + details: Please describe the details of this change for the release notes. You can + use asciidoc. + impact: Please describe the impact of this change to users + notable: false From 750eb7d41ec1805b98d556868b566c8838f35a52 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 30 Sep 2022 11:51:07 +0100 Subject: [PATCH 10/28] Update changelog with change details --- docs/changelog/90529.yaml | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index efa5aa15ca4e8..7ca134ae272a0 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,13 +1,24 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking, bug" +type: bug issues: - 89387 breaking: - title: Output a consistent format when generating error json - area: Infra/REST API - details: Please describe the details of this change for the release notes. You can - use asciidoc. - impact: Please describe the impact of this change to users + title: >- + Error JSON structure has changed when detailed errors are disabled + area: REST API + details: |- + This change modifies the JSON format of error messages returned to REST clients + when detailed messages are turned off. + Previously, JSON returned when an exception occurred, and `http.detailed_errors.enabled: false` was set, + just consisted of a single `"error"` text field with some basic information. + Setting `http.detailed_errors.enabled: true` (the default) changed this field + to an object with more detailed information. + With this change, non-detailed errors now have the same structure as detailed errors. `"error"` will now always + be an object with, at a minimum, a `"type"` and `"reason"` field. Additional fields are included when detailed + errors are enabled. + impact: >- + If you have set `http.detailed_errors.enabled: false` (the default is `true`) the structure of JSON + when any exceptions occur now matches the structure when detailed errors are enabled. notable: false From 93dc953911f7d7647a3891489f6e44b4d9187233 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Mon, 3 Oct 2022 14:55:38 +0100 Subject: [PATCH 11/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 7ca134ae272a0..3e037a1d5d286 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,12 +1,11 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: bug +type: "breaking, bug" issues: - 89387 breaking: - title: >- - Error JSON structure has changed when detailed errors are disabled + title: Error JSON structure has changed when detailed errors are disabled area: REST API details: |- This change modifies the JSON format of error messages returned to REST clients @@ -18,7 +17,7 @@ breaking: With this change, non-detailed errors now have the same structure as detailed errors. `"error"` will now always be an object with, at a minimum, a `"type"` and `"reason"` field. Additional fields are included when detailed errors are enabled. - impact: >- - If you have set `http.detailed_errors.enabled: false` (the default is `true`) the structure of JSON - when any exceptions occur now matches the structure when detailed errors are enabled. + impact: "If you have set `http.detailed_errors.enabled: false` (the default is `true`)\ + \ the structure of JSON when any exceptions occur now matches the structure when\ + \ detailed errors are enabled." notable: false From d293dcbe5647f39c72e097c741ffbeafd5e2bc7d Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Mon, 3 Oct 2022 16:49:51 +0100 Subject: [PATCH 12/28] Match the method parameter types --- .../java/org/elasticsearch/ElasticsearchExceptionTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 5530cf5afb0e1..36afe510d73fe 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -1229,7 +1229,7 @@ private static void assertToXContentAsJson(ToXContent e, String expectedJson) th assertToXContentEquivalent(new BytesArray(expectedJson), actual, XContentType.JSON); } - private static void assertThrowableAsJson(Exception e, String expectedJson) throws IOException { + private static void assertThrowableAsJson(Throwable e, String expectedJson) throws IOException { assertToXContentAsJson((builder, params) -> { ElasticsearchException.generateThrowableXContent(builder, params, e); return builder; From ab3041c38915b132fb369485f8ab4e73a85d9133 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 17 Nov 2022 11:58:06 +0000 Subject: [PATCH 13/28] Update docs --- docs/changelog/90529.yaml | 2 +- docs/reference/modules/http.asciidoc | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 3e037a1d5d286..10bc6331691d4 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking, bug" +type: "breaking" issues: - 89387 breaking: diff --git a/docs/reference/modules/http.asciidoc b/docs/reference/modules/http.asciidoc index c677eb377316f..323469c2de2ca 100644 --- a/docs/reference/modules/http.asciidoc +++ b/docs/reference/modules/http.asciidoc @@ -134,11 +134,9 @@ NOTE: This header is only returned when the setting is set to `true`. `http.detailed_errors.enabled`:: (<>, boolean) -Configures whether detailed error reporting in HTTP responses is enabled. -Defaults to `true`, which means that HTTP requests that include the -<> will return a -detailed error message including a stack trace if they encounter an exception. -If set to `false`, requests with the `?error_trace` parameter are rejected. +Configures whether detailed error reporting in HTTP responses is enabled. Defaults to `true`. +When this option is set to `false`, only basic information is returned if an error occurs in the request, +and requests with <> set are rejected. `http.pipelining.max_events`:: (<>, integer) From 459acf407c0e4f16fa23bb49fd98634972a49e1c Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 8 Feb 2023 19:51:47 +0000 Subject: [PATCH 14/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 10bc6331691d4..3e037a1d5d286 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking" +type: "breaking, bug" issues: - 89387 breaking: From 02862e9cd8f918f6f21a68c00566fc02bd0bad23 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 10 Feb 2023 11:41:47 +0000 Subject: [PATCH 15/28] Revert "Update docs/changelog/90529.yaml" This reverts commit 459acf407c0e4f16fa23bb49fd98634972a49e1c. --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 3e037a1d5d286..10bc6331691d4 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking, bug" +type: "breaking" issues: - 89387 breaking: From 14b55f9c6b00b95d65d948a386c317132f0d518a Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 26 Apr 2023 13:24:22 +0100 Subject: [PATCH 16/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 10bc6331691d4..3e037a1d5d286 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking" +type: "breaking, bug" issues: - 89387 breaking: From a2548abc0e09a7bec35c237d5d8b8f1418c00eb6 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 10 Oct 2024 16:21:51 +0100 Subject: [PATCH 17/28] Revert "Update docs/changelog/90529.yaml" This reverts commit 14b55f9c6b00b95d65d948a386c317132f0d518a. --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 3e037a1d5d286..10bc6331691d4 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking, bug" +type: "breaking" issues: - 89387 breaking: From 6f253c4fe7a3fa8fea0f8d751a7070e73b60a4f9 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Fri, 18 Oct 2024 13:38:17 +0100 Subject: [PATCH 18/28] Add V8 rest API formats for compatibility --- .../elasticsearch/ElasticsearchException.java | 26 +++++ .../common/xcontent/XContentHelper.java | 18 ++- .../ElasticsearchExceptionTests.java | 106 +++++++++++++++--- .../org/elasticsearch/test/ESTestCase.java | 18 ++- 4 files changed, 149 insertions(+), 19 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 5bdb8a30d3654..3c5c365654206 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -25,7 +25,9 @@ import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Tuple; +import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.health.node.action.HealthNodeNotDiscoveredException; import org.elasticsearch.index.Index; import org.elasticsearch.index.mapper.DocumentParsingException; @@ -611,6 +613,16 @@ protected static void generateThrowableXContent(XContentBuilder builder, Params */ public static XContentBuilder generateFailureXContent(XContentBuilder builder, Params params, @Nullable Exception e, boolean detailed) throws IOException { + if (builder.getRestApiVersion() == RestApiVersion.V_8) { + if (e == null) { + return builder.field(ERROR, "unknown"); + } + if (detailed == false) { + return generateNonDetailedFailureXContentV8(builder, e); + } + // else fallthrough + } + if (e == null) { // No exception to render as an error builder.startObject(ERROR); @@ -644,6 +656,20 @@ public static XContentBuilder generateFailureXContent(XContentBuilder builder, P return builder.endObject(); } + @UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA) // remove V8 API + private static XContentBuilder generateNonDetailedFailureXContentV8(XContentBuilder builder, @Nullable Exception e) throws IOException { + String message = "No ElasticsearchException found"; + Throwable t = e; + for (int counter = 0; counter < 10 && t != null; counter++) { + if (t instanceof ElasticsearchException) { + message = t.getClass().getSimpleName() + "[" + t.getMessage() + "]"; + break; + } + t = t.getCause(); + } + return builder.field(ERROR, message); + } + /** * Parses the output of {@link #generateFailureXContent(XContentBuilder, Params, Exception, boolean)} */ diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java b/server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java index 9cfa22d0a3cfb..9464ccbcc7aa3 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/XContentHelper.java @@ -20,6 +20,7 @@ import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.core.CheckedFunction; import org.elasticsearch.core.Nullable; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Tuple; import org.elasticsearch.plugins.internal.XContentParserDecorator; import org.elasticsearch.xcontent.DeprecationHandler; @@ -626,7 +627,22 @@ public static BytesReference toXContent(ChunkedToXContent toXContent, XContentTy */ public static BytesReference toXContent(ToXContent toXContent, XContentType xContentType, Params params, boolean humanReadable) throws IOException { - try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent())) { + return toXContent(toXContent, xContentType, RestApiVersion.current(), params, humanReadable); + } + + /** + * Returns the bytes that represent the XContent output of the provided {@link ToXContent} object, using the provided + * {@link XContentType}. Wraps the output into a new anonymous object according to the value returned + * by the {@link ToXContent#isFragment()} method returns. + */ + public static BytesReference toXContent( + ToXContent toXContent, + XContentType xContentType, + RestApiVersion restApiVersion, + Params params, + boolean humanReadable + ) throws IOException { + try (XContentBuilder builder = XContentBuilder.builder(xContentType.xContent(), restApiVersion)) { builder.humanReadable(humanReadable); if (toXContent.isFragment()) { builder.startObject(); diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index 4d0afb3de19d1..f5a23cf68a26e 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -29,6 +29,7 @@ import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.util.Maps; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.Index; import org.elasticsearch.index.IndexNotFoundException; @@ -1066,11 +1067,13 @@ public void testThrowableToAndFromXContent() throws IOException { public void testUnknownFailureToAndFromXContent() throws IOException { final XContent xContent = randomFrom(XContentType.values()).xContent(); - BytesReference failureBytes = toShuffledXContent((builder, params) -> { - // Prints a null failure using generateFailureXContent() - ElasticsearchException.generateFailureXContent(builder, params, null, randomBoolean()); - return builder; - }, xContent.type(), ToXContent.EMPTY_PARAMS, randomBoolean()); + // Prints a null failure using generateFailureXContent() + BytesReference failureBytes = toShuffledXContent( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, null, randomBoolean()), + xContent.type(), + ToXContent.EMPTY_PARAMS, + randomBoolean() + ); ElasticsearchException parsedFailure; try (XContentParser parser = createParser(xContent, failureBytes)) { @@ -1087,14 +1090,43 @@ public void testUnknownFailureToAndFromXContent() throws IOException { assertEquals(0, parsedFailure.getMetadata().size()); } + public void testUnknownFailureToAndFromXContentV8() throws IOException { + final XContent xContent = randomFrom(XContentType.values()).xContent(); + + // Prints a null failure using generateFailureXContent() + BytesReference failureBytes = toShuffledXContent( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, null, randomBoolean()), + xContent.type(), + RestApiVersion.V_8, + ToXContent.EMPTY_PARAMS, + randomBoolean() + ); + + ElasticsearchException parsedFailure; + try (XContentParser parser = createParser(xContent, failureBytes)) { + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); + parsedFailure = ElasticsearchException.failureFromXContent(parser); + assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); + assertNull(parser.nextToken()); + } + + // Failure was null, expecting a "unknown" reason + assertEquals("Elasticsearch exception [type=exception, reason=unknown]", parsedFailure.getMessage()); + assertEquals(0, parsedFailure.getHeaders().size()); + assertEquals(0, parsedFailure.getMetadata().size()); + } + public void testFailureToAndFromXContentWithNoDetails() throws IOException { final XContent xContent = randomFrom(XContentType.values()).xContent(); final Exception failure = (Exception) randomExceptions().v1(); - BytesReference failureBytes = toShuffledXContent((builder, params) -> { - ElasticsearchException.generateFailureXContent(builder, params, failure, false); - return builder; - }, xContent.type(), ToXContent.EMPTY_PARAMS, randomBoolean()); + BytesReference failureBytes = toShuffledXContent( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, failure, false), + xContent.type(), + ToXContent.EMPTY_PARAMS, + randomBoolean() + ); try (XContentParser parser = createParser(xContent, failureBytes)) { failureBytes = BytesReference.bytes(shuffleXContent(parser, randomBoolean())); @@ -1118,6 +1150,44 @@ public void testFailureToAndFromXContentWithNoDetails() throws IOException { assertNull(parsedFailure.getCause()); } + public void testFailureToAndFromXContentWithNoDetailsV8() throws IOException { + final XContent xContent = randomFrom(XContentType.values()).xContent(); + + final Exception failure = (Exception) randomExceptions().v1(); + BytesReference failureBytes = toShuffledXContent( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, failure, false), + xContent.type(), + RestApiVersion.V_8, + ToXContent.EMPTY_PARAMS, + randomBoolean() + ); + + try (XContentParser parser = createParser(xContent, failureBytes)) { + failureBytes = BytesReference.bytes(shuffleXContent(parser, randomBoolean())); + } + + ElasticsearchException parsedFailure; + try (XContentParser parser = createParser(xContent, failureBytes)) { + assertEquals(XContentParser.Token.START_OBJECT, parser.nextToken()); + assertEquals(XContentParser.Token.FIELD_NAME, parser.nextToken()); + parsedFailure = ElasticsearchException.failureFromXContent(parser); + assertEquals(XContentParser.Token.END_OBJECT, parser.nextToken()); + assertNull(parser.nextToken()); + } + assertNotNull(parsedFailure); + + String reason; + if (failure instanceof ElasticsearchException) { + reason = failure.getClass().getSimpleName() + "[" + failure.getMessage() + "]"; + } else { + reason = "No ElasticsearchException found"; + } + assertEquals(ElasticsearchException.buildMessage("exception", reason, null), parsedFailure.getMessage()); + assertEquals(0, parsedFailure.getHeaders().size()); + assertEquals(0, parsedFailure.getMetadata().size()); + assertNull(parsedFailure.getCause()); + } + public void testFailureToAndFromXContentWithDetails() throws IOException { final XContent xContent = randomFrom(XContentType.values()).xContent(); @@ -1231,10 +1301,12 @@ public void testFailureToAndFromXContentWithDetails() throws IOException { } Exception finalFailure = failure; - BytesReference failureBytes = toShuffledXContent((builder, params) -> { - ElasticsearchException.generateFailureXContent(builder, params, finalFailure, true); - return builder; - }, xContent.type(), ToXContent.EMPTY_PARAMS, randomBoolean()); + BytesReference failureBytes = toShuffledXContent( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, finalFailure, true), + xContent.type(), + ToXContent.EMPTY_PARAMS, + randomBoolean() + ); try (XContentParser parser = createParser(xContent, failureBytes)) { failureBytes = BytesReference.bytes(shuffleXContent(parser, randomBoolean())); @@ -1271,10 +1343,10 @@ private static void assertThrowableAsJson(Throwable e, String expectedJson) thro } private static void assertFailureAsJson(Exception e, String expectedJson, boolean detailed) throws IOException { - assertToXContentAsJson((builder, params) -> { - ElasticsearchException.generateFailureXContent(builder, params, e, detailed); - return builder; - }, expectedJson); + assertToXContentAsJson( + (builder, params) -> ElasticsearchException.generateFailureXContent(builder, params, e, detailed), + expectedJson + ); } public static void assertDeepEquals(ElasticsearchException expected, ElasticsearchException actual) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 31c8e5bc3d457..30d1ad6be3ef5 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -1699,7 +1699,23 @@ protected final BytesReference toShuffledXContent( boolean humanReadable, String... exceptFieldNames ) throws IOException { - BytesReference bytes = XContentHelper.toXContent(toXContent, xContentType, params, humanReadable); + return toShuffledXContent(toXContent, xContentType, RestApiVersion.current(), params, humanReadable, exceptFieldNames); + } + + /** + * Returns the bytes that represent the XContent output of the provided {@link ToXContent} object, using the provided + * {@link XContentType}. Wraps the output into a new anonymous object according to the value returned + * by the {@link ToXContent#isFragment()} method returns. Shuffles the keys to make sure that parsing never relies on keys ordering. + */ + protected final BytesReference toShuffledXContent( + ToXContent toXContent, + XContentType xContentType, + RestApiVersion restApiVersion, + ToXContent.Params params, + boolean humanReadable, + String... exceptFieldNames + ) throws IOException { + BytesReference bytes = XContentHelper.toXContent(toXContent, xContentType, restApiVersion, params, humanReadable); try (XContentParser parser = createParser(xContentType.xContent(), bytes)) { try (XContentBuilder builder = shuffleXContent(parser, rarely(), exceptFieldNames)) { return BytesReference.bytes(builder); From 93da694981694606d7b5420a1abbbb6ead29e9df Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Mon, 14 Oct 2024 16:46:35 +0100 Subject: [PATCH 19/28] Add a basic deprecation warning that the JSON format is changing in v9 --- .../rest/AbstractRestChannel.java | 13 ++++++++++ .../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 | 4 ++++ .../rest/action/RestBuilderListenerTests.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 ++-- 27 files changed, 71 insertions(+), 54 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..25ba248a8ee7a 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -13,7 +13,10 @@ 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.core.RestApiVersion; import org.elasticsearch.xcontent.ParsedMediaType; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; @@ -32,6 +35,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 +65,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 (request.getRestApiVersion() == RestApiVersion.V_8 && 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 cf623e77f740a..19d92568e6528 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 8f1904ce42438..afdad1045b4de 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -161,7 +161,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 @@ -180,7 +180,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()); } @@ -211,7 +211,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), @@ -235,7 +235,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))); } @@ -257,7 +257,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))); } @@ -280,7 +280,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))); } @@ -290,7 +290,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))); } @@ -314,7 +314,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), @@ -340,7 +340,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()); } @@ -831,7 +831,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")); @@ -1115,7 +1115,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)); }); } @@ -1137,7 +1137,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 63dd65de8897b..98dc81d75bef9 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -467,6 +467,10 @@ 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)); + 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 void testSupressedLogging() throws IOException { 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/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 24f59a8c3abe7..4822b1c64cf41 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 @@ -51,7 +51,7 @@ public void testEnableFieldsEmulationNoErrors() throws Exception { .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 cc2bc6a9ece4d66cf8bca6fd19fd839d99061856 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:18:00 +0000 Subject: [PATCH 20/28] Revert "Add a deprecation warning that the JSON format of non-detailed errors is changing in v9 (#116330)" This reverts commit bd091d3d96b33adb4121f980dc4f9f7a2a87b043. --- .../org/elasticsearch/rest/RestResponse.java | 13 ---------- .../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 | 6 ++--- .../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 ++-- 27 files changed, 56 insertions(+), 87 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/RestResponse.java b/server/src/main/java/org/elasticsearch/rest/RestResponse.java index 29cae343fb09e..fd8b90a99e7f6 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/RestResponse.java @@ -16,8 +16,6 @@ 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; @@ -45,7 +43,6 @@ 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; @@ -145,16 +142,6 @@ 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); 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 a1b9c59571496..0cb4a56794c22 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, true, 0); + FakeRestChannel channel = new FakeRestChannel(request, false, 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 4dce73fcf0e89..54dff48788f52 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, true, 0); + FakeRestChannel channel = new FakeRestChannel(request, false, 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 19d92568e6528..cf623e77f740a 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, - true, + false, 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, true, RestStatus.BAD_REQUEST); + RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel(fakeRequest, false, 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 8a8bed9ca73db..9f82911ed121f 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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, true, 1); + final RestChannel channel = new FakeRestChannel(request, randomBoolean(), 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 907c16aad5fdc..eece90ed94cf9 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(), - true, + randomBoolean(), 1 ) ); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index afdad1045b4de..8f1904ce42438 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -161,7 +161,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, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, 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 @@ -180,7 +180,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, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); restController.dispatchRequest(fakeRequest, channel, threadContext); assertTrue(channel.getSendResponseCalled()); } @@ -211,7 +211,7 @@ public String getName() { }); } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy( eq(1L), @@ -235,7 +235,7 @@ public MethodHandlers next() { return null; } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -257,7 +257,7 @@ public MethodHandlers next() { } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -280,7 +280,7 @@ public String getName() { })); when(spyRestController.getAllHandlers(any(), eq(fakeRequest.rawPath()))).thenAnswer(x -> handlers.iterator()); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.METHOD_NOT_ALLOWED); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.METHOD_NOT_ALLOWED); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 405))); } @@ -290,7 +290,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, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); restController.dispatchBadRequest(channel, threadContext, new Exception()); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -314,7 +314,7 @@ public MethodHandlers next() { return new MethodHandlers("/").addMethod(GET, RestApiVersion.current(), (request, channel, client) -> {}); } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); restController.dispatchRequest(fakeRequest, channel, threadContext); verify(tracer).startTrace( eq(threadContext), @@ -340,7 +340,7 @@ public void testRequestWithDisallowedMultiValuedHeaderButSameValues() { new RestResponse(RestStatus.OK, RestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY) ) ); - AssertingChannel channel = new AssertingChannel(fakeRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK); restController.dispatchRequest(fakeRequest, channel, threadContext); assertTrue(channel.getSendResponseCalled()); } @@ -831,7 +831,7 @@ public void testFavicon() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod(GET) .withPath("/favicon.ico") .build(); - final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + final AssertingChannel channel = new AssertingChannel(fakeRestRequest, false, RestStatus.OK); restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext()); assertTrue(channel.getSendResponseCalled()); assertThat(channel.getRestResponse().contentType(), containsString("image/x-icon")); @@ -1115,7 +1115,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, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(request, false, RestStatus.OK); restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY)); }); } @@ -1137,7 +1137,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, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(request, false, 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 4345f3c5e3fb4..3b839896bc34f 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, true, 1); + FakeRestChannel restChannel = new FakeRestChannel(restRequest, false, 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 67e84be76b474..ecabe2e8af498 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -93,7 +93,6 @@ 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() { @@ -118,7 +117,6 @@ 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 { @@ -145,7 +143,6 @@ 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 { @@ -177,7 +174,6 @@ 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); } } } @@ -202,7 +198,6 @@ public void testStackTrace() throws IOException { } else { assertThat(response.content().utf8ToString(), not(containsString(ElasticsearchException.STACK_TRACE))); } - assertChannelWarnings(channel); } } } @@ -235,7 +230,6 @@ public void testNullThrowable() throws Exception { assertThat(text, containsString("\"type\":\"unknown\"")); assertThat(text, containsString("\"reason\":\"unknown\"")); assertThat(text, not(containsString("error_trace"))); - assertChannelWarnings(channel); } public void testConvert() throws IOException { @@ -420,7 +414,6 @@ public void testErrorToAndFromXContent() throws IOException { assertEquals(expected.status(), parsedError.status()); assertDeepEquals(expected, parsedError); - assertChannelWarnings(channel); } public void testNoErrorFromXContent() throws IOException { @@ -487,7 +480,6 @@ 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 { @@ -519,7 +511,6 @@ public void testSupressedLogging() throws IOException { "401", "unauthorized" ); - assertChannelWarnings(channel); } private void assertLogging( @@ -545,15 +536,6 @@ 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 03ae366050646..827a07b89b2b8 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(), true, 1) + new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 1) ) { @Override public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws Exception { @@ -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(), true, 1) + new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 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(), true, 1) + new FakeRestChannel(new FakeRestRequest(), randomBoolean(), 1) ) { @Override public RestResponse buildResponse(Empty empty, XContentBuilder builder) throws Exception { 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 8104ecfc31c3d..880a0bc9eabd7 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, true, 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, false, 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 0d35e4311032d..3b6b280565da5 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, true, 1); + FakeRestChannel channel = new FakeRestChannel(request, false, 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 4822b1c64cf41..24f59a8c3abe7 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 @@ -51,7 +51,7 @@ public void testEnableFieldsEmulationNoErrors() throws Exception { .withParams(params) .build(); - action.handleRequest(request, new FakeRestChannel(request, true, 1), verifyingClient); + action.handleRequest(request, new FakeRestChannel(request, false, 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 33978b4cd6b9f..0c95340fdb6f7 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, true, isLicensed ? 0 : 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), 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 8509a6475aa71..5d4ea0f30cb15 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, true, securityEnabled ? 0 : 1); + FakeRestChannel fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), 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 b734e602ec291..6ff05faf22d11 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, true, requiredSettingsEnabled ? 0 : 1); + final var fakeRestChannel = new FakeRestChannel(fakeRestRequest, randomBoolean(), 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 79dba637d53d0..9a05230d82ae6 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 a47855731b37a..812354986d5bc 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 c65634a76b532..d88a217cd0949 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 2cb1b6a66b02b..ac472378d4874 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 7005b5158e626..d5aa249b1d0f5 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 879e1ac8ad157..ddeffc0675498 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 bd665560f425f..2ac33a780313e 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, true) { + RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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 38405a2167808..4a593eeb24ac6 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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @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, true) { + final RestChannel restChannel = new AbstractRestChannel(restRequest, randomBoolean()) { @Override public void sendResponse(RestResponse restResponse) { responseSetOnce.set(restResponse); From 980a44971ceec56707a21b20ce4a9ca8110fc467 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:26:26 +0000 Subject: [PATCH 21/28] Propagate changes from deprecation PRs --- docs/changelog/90529.yaml | 9 ++++++--- .../elasticsearch/rest/AbstractRestChannel.java | 11 ----------- .../java/org/elasticsearch/rest/RestResponse.java | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 10bc6331691d4..a014c82259a9e 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -17,7 +17,10 @@ breaking: With this change, non-detailed errors now have the same structure as detailed errors. `"error"` will now always be an object with, at a minimum, a `"type"` and `"reason"` field. Additional fields are included when detailed errors are enabled. - impact: "If you have set `http.detailed_errors.enabled: false` (the default is `true`)\ - \ the structure of JSON when any exceptions occur now matches the structure when\ - \ detailed errors are enabled." + To use the previous structure for non-detailed errors, 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 now matches the structure when + detailed errors are enabled. + To use the previous structure for non-detailed errors, use the v8 REST API. notable: false diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index 25ba248a8ee7a..dbeae9f17075e 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -13,10 +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.core.RestApiVersion; import org.elasticsearch.xcontent.ParsedMediaType; import org.elasticsearch.xcontent.XContentBuilder; import org.elasticsearch.xcontent.XContentFactory; @@ -65,15 +63,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 (request.getRestApiVersion() == RestApiVersion.V_8 && 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..d043974055667 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/RestResponse.java @@ -16,10 +16,13 @@ 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; import org.elasticsearch.core.Releasables; +import org.elasticsearch.core.RestApiVersion; import org.elasticsearch.xcontent.ToXContent; import org.elasticsearch.xcontent.XContentBuilder; @@ -43,6 +46,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 +146,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.request().getRestApiVersion() == RestApiVersion.V_8 && channel.detailedErrorsEnabled() == false) { + deprecationLogger.warn( + DeprecationCategory.API, + "http_detailed_errors", + "The JSON format of non-detailed errors has changed in Elasticsearch 9.0 to match the JSON structure" + + " used for detailed errors." + ); + } + try (XContentBuilder builder = channel.newErrorBuilder()) { build(builder, params, status, channel.detailedErrorsEnabled(), e); this.content = BytesReference.bytes(builder); From 76f918515f4f3c8d5ecb4b1424de11077e06a61b Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:30:51 +0000 Subject: [PATCH 22/28] Update tests --- .../elasticsearch/rest/AbstractRestChannel.java | 2 -- .../elasticsearch/rest/RestResponseTests.java | 16 +++------------- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java index dbeae9f17075e..1b506b80c5496 100644 --- a/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java +++ b/server/src/main/java/org/elasticsearch/rest/AbstractRestChannel.java @@ -13,7 +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.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.xcontent.ParsedMediaType; import org.elasticsearch.xcontent.XContentBuilder; @@ -33,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(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java index ecabe2e8af498..cf9de7d986afb 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -132,19 +132,6 @@ public void testDetailedExceptionMessage() throws Exception { {"type":"file_not_found_exception","reason":"/foo/bar"}""")); } - public void testNonElasticsearchExceptionIsNotShownAsSimpleMessage() throws Exception { - RestRequest request = new FakeRestRequest(); - RestChannel channel = new SimpleExceptionRestChannel(request); - - Exception t = new UnknownException("an error occurred reading data", new FileNotFoundException("/foo/bar")); - RestResponse response = new RestResponse(channel, t); - String text = response.content().utf8ToString(); - assertThat(text, not(containsString("UnknownException[an error occurred reading data]"))); - assertThat(text, not(containsString("FileNotFoundException[/foo/bar]"))); - assertThat(text, not(containsString("error_trace"))); - assertThat(text, containsString("\"error\":\"No ElasticsearchException found\"")); - } - public void testErrorTrace() throws Exception { RestRequest request = new FakeRestRequest(); request.params().put("error_trace", "true"); @@ -480,6 +467,9 @@ 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)); + assertWarnings( + "The JSON format of non-detailed errors has changed in Elasticsearch 9.0 to match the JSON structure used for detailed errors." + ); } public void testSupressedLogging() throws IOException { From 47263635ed7755acbab6d69468b59c81a9936b42 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:34:20 +0000 Subject: [PATCH 23/28] Update docs/changelog/90529.yaml --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index a014c82259a9e..6b24ca930cf9c 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking" +type: "breaking, bug" issues: - 89387 breaking: From cb964f0c1c4e2449e4f626c58111b13b39c3ee94 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:40:04 +0000 Subject: [PATCH 24/28] Revert "Update docs/changelog/90529.yaml" This reverts commit 47263635ed7755acbab6d69468b59c81a9936b42. --- docs/changelog/90529.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/90529.yaml b/docs/changelog/90529.yaml index 6b24ca930cf9c..a014c82259a9e 100644 --- a/docs/changelog/90529.yaml +++ b/docs/changelog/90529.yaml @@ -1,7 +1,7 @@ pr: 90529 summary: Output a consistent format when generating error json area: Infra/REST API -type: "breaking, bug" +type: "breaking" issues: - 89387 breaking: From aef3b2df0797bb52f3e27016ddcfa0923caf9b4f Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Wed, 13 Nov 2024 14:46:46 +0000 Subject: [PATCH 25/28] Use more randomness in detailed parameter --- .../synonyms/PutSynonymRuleActionTests.java | 2 +- .../synonyms/PutSynonymsActionTests.java | 2 +- .../AbstractHttpServerTransportTests.java | 4 +- .../rest/RestControllerTests.java | 82 +++++++++---------- .../rest/RestHttpResponseHeadersTests.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 +- 11 files changed, 52 insertions(+), 52 deletions(-) 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..303b75098ab67 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, randomBoolean(), 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..915c338195c86 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, randomBoolean(), 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 cf623e77f740a..f209d99bceb7a 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, + randomBoolean(), 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, randomBoolean(), RestStatus.BAD_REQUEST); try ( AbstractHttpServerTransport transport = new AbstractHttpServerTransport( diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index 8f1904ce42438..b7d38f6f299c7 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -161,7 +161,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, randomBoolean(), 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 @@ -180,7 +180,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, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchRequest(fakeRequest, channel, threadContext); assertTrue(channel.getSendResponseCalled()); } @@ -211,7 +211,7 @@ public String getName() { }); } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRequest, randomBoolean(), RestStatus.OK); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy( eq(1L), @@ -235,7 +235,7 @@ public MethodHandlers next() { return null; } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, randomBoolean(), RestStatus.BAD_REQUEST); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -257,7 +257,7 @@ public MethodHandlers next() { } }); - AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRequest, randomBoolean(), RestStatus.BAD_REQUEST); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -280,7 +280,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, randomBoolean(), RestStatus.METHOD_NOT_ALLOWED); spyRestController.dispatchRequest(fakeRequest, channel, threadContext); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 405))); } @@ -290,7 +290,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, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchBadRequest(channel, threadContext, new Exception()); verify(requestsCounter).incrementBy(eq(1L), eq(Map.of(STATUS_CODE_KEY, 400))); } @@ -314,7 +314,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, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchRequest(fakeRequest, channel, threadContext); verify(tracer).startTrace( eq(threadContext), @@ -340,7 +340,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, randomBoolean(), RestStatus.OK); restController.dispatchRequest(fakeRequest, channel, threadContext); assertTrue(channel.getSendResponseCalled()); } @@ -466,7 +466,7 @@ public void testRestInterceptor() throws Exception { ); restController.registerHandler(new Route(GET, "/wrapped"), handler); RestRequest request = testRestRequest("/wrapped", "{}", XContentType.JSON); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); httpServerTransport.start(); assertThat(wrapperCalled.get(), is(true)); @@ -477,7 +477,7 @@ public void testDispatchRequestAddsAndFreesBytesOnSuccess() { int contentLength = BREAKER_LIMIT.bytesAsInt(); String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, XContentType.JSON); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.OK); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); @@ -489,7 +489,7 @@ public void testDispatchRequestAddsAndFreesBytesOnError() { int contentLength = BREAKER_LIMIT.bytesAsInt(); String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/error", content, XContentType.JSON); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); @@ -502,7 +502,7 @@ public void testDispatchRequestAddsAndFreesBytesOnlyOnceOnError() { String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); // we will produce an error in the rest handler and one more when sending the error response RestRequest request = testRestRequest("/error", content, XContentType.JSON); - ExceptionThrowingChannel channel = new ExceptionThrowingChannel(request, true); + ExceptionThrowingChannel channel = new ExceptionThrowingChannel(request, randomBoolean()); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); @@ -521,7 +521,7 @@ public void testDispatchRequestAddsAndFreesBytesOnlyOnceOnErrorDuringSend() { ); // we will produce an error in the rest handler and one more when sending the error response RestRequest request = testRestRequest("/foo", content, XContentType.JSON); - ExceptionThrowingChannel channel = new ExceptionThrowingChannel(request, true) { + ExceptionThrowingChannel channel = new ExceptionThrowingChannel(request, randomBoolean()) { @Override protected BytesStream newBytesOutput() { return new RecyclerBytesStreamOutput(recycler); @@ -538,7 +538,7 @@ public void testDispatchRequestLimitsBytes() { int contentLength = BREAKER_LIMIT.bytesAsInt() + 1; String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, XContentType.JSON); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.TOO_MANY_REQUESTS); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.TOO_MANY_REQUESTS); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); @@ -549,7 +549,7 @@ public void testDispatchRequestLimitsBytes() { public void testDispatchRequiresContentTypeForRequestsWithContent() { String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, null); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.NOT_ACCEPTABLE); restController = new RestController(null, null, circuitBreakerService, usageService, telemetryProvider); restController.registerHandler( new Route(GET, "/"), @@ -566,7 +566,7 @@ public void testDispatchDoesNotRequireContentTypeForRequestsWithoutContent() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); assertFalse(channel.getSendResponseCalled()); restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext()); @@ -582,7 +582,7 @@ public void testDispatchFailsWithPlainText() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); restController.registerHandler( new Route(GET, "/foo"), (request, channel1, client) -> channel1.sendResponse( @@ -603,7 +603,7 @@ public void testDispatchUnsupportedContentType() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); assertFalse(channel.getSendResponseCalled()); restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext()); @@ -620,7 +620,7 @@ public void testDispatchWorksWithNewlineDelimitedJson() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { @@ -659,7 +659,7 @@ public void testDispatchWithContentStream() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { @@ -683,7 +683,7 @@ public void testDispatchWithContentStreamNoContentType() { RestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withContent(new BytesArray("{}"), null) .withPath("/foo") .build(); - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } @@ -712,7 +712,7 @@ public void testNonStreamingXContentCausesErrorResponse() throws IOException { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { @@ -737,7 +737,7 @@ public void testUnknownContentWithContentStream() { if (randomBoolean()) { fakeRestRequest = new RestRequest(fakeRestRequest); } - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { @@ -756,7 +756,7 @@ public boolean supportsBulkContent() { public void testDispatchBadRequest() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); - final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); + final AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchBadRequest( channel, client.threadPool().getThreadContext(), @@ -789,7 +789,7 @@ public boolean canTripCircuitBreaker() { .withContent(BytesReference.bytes(content), content.contentType()) .build(); - final AssertingChannel channel = new AssertingChannel(restRequest, true, RestStatus.OK); + final AssertingChannel channel = new AssertingChannel(restRequest, randomBoolean(), RestStatus.OK); assertFalse(channel.getSendResponseCalled()); assertFalse(restRequest.isContentConsumed()); @@ -801,7 +801,7 @@ public boolean canTripCircuitBreaker() { public void testDispatchBadRequestUnknownCause() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); - final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); + final AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.BAD_REQUEST); restController.dispatchBadRequest(channel, client.threadPool().getThreadContext(), null); assertTrue(channel.getSendResponseCalled()); assertThat(channel.getRestResponse().content().utf8ToString(), containsString("unknown cause")); @@ -813,14 +813,14 @@ public void testDispatchBadRequestWithValidationException() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); // it's always a 400 bad request when dispatching "regular" {@code ElasticsearchException} - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.BAD_REQUEST); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.BAD_REQUEST); assertFalse(channel.getSendResponseCalled()); restController.dispatchBadRequest(channel, client.threadPool().getThreadContext(), exception); assertTrue(channel.getSendResponseCalled()); assertThat(channel.getRestResponse().content().utf8ToString(), containsString("bad bad exception")); // but {@code HttpHeadersValidationException} do carry over the rest response code - channel = new AssertingChannel(fakeRestRequest, true, status); + channel = new AssertingChannel(fakeRestRequest, randomBoolean(), status); assertFalse(channel.getSendResponseCalled()); restController.dispatchBadRequest(channel, client.threadPool().getThreadContext(), new HttpHeadersValidationException(exception)); assertTrue(channel.getSendResponseCalled()); @@ -831,7 +831,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, randomBoolean(), RestStatus.OK); restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext()); assertTrue(channel.getSendResponseCalled()); assertThat(channel.getRestResponse().contentType(), containsString("image/x-icon")); @@ -841,7 +841,7 @@ public void testFaviconWithWrongHttpMethod() { final FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).withMethod( randomValueOtherThanMany(m -> m == GET || m == OPTIONS, () -> randomFrom(RestRequest.Method.values())) ).withPath("/favicon.ico").build(); - final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.METHOD_NOT_ALLOWED); + final AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.METHOD_NOT_ALLOWED); restController.dispatchRequest(fakeRestRequest, channel, client.threadPool().getThreadContext()); assertTrue(channel.getSendResponseCalled()); assertThat(channel.getRestResponse().getHeaders().containsKey("Allow"), equalTo(true)); @@ -917,7 +917,7 @@ public Exception getInboundException() { } }, null); - final AssertingChannel channel = new AssertingChannel(request, true, RestStatus.METHOD_NOT_ALLOWED); + final AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.METHOD_NOT_ALLOWED); assertFalse(channel.getSendResponseCalled()); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); assertTrue(channel.getSendResponseCalled()); @@ -937,7 +937,7 @@ public Method method() { } }; - final AssertingChannel channel = new AssertingChannel(request, true, RestStatus.METHOD_NOT_ALLOWED); + final AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.METHOD_NOT_ALLOWED); restController.dispatchRequest(request, channel, client.threadPool().getThreadContext()); verify(tracer).startTrace(any(), any(RestRequest.class), anyString(), anyMap()); verify(tracer).addError(any(RestRequest.class), any(IllegalArgumentException.class)); @@ -951,7 +951,7 @@ public void testDispatchCompatibleHandler() { final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); // dispatch to a compatible handler restController.registerHandler(GET, "/foo", RestApiVersion.minimumSupported(), (request, channel1, client) -> { @@ -975,7 +975,7 @@ public void testDispatchCompatibleRequestToNewlyAddedHandler() { final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); // dispatch to a CURRENT newly added handler restController.registerHandler(new Route(GET, "/foo"), (request, channel1, client) -> { @@ -1018,7 +1018,7 @@ public void testCurrentVersionVNDMediaTypeIsNotUsingCompatibility() { final String mediaType = randomCompatibleMediaType(version); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); // dispatch to a CURRENT newly added handler restController.registerHandler(new Route(GET, "/foo"), (request, channel1, client) -> { @@ -1041,7 +1041,7 @@ public void testCustomMediaTypeValidation() { final String mediaType = "application/x-protobuf"; FakeRestRequest fakeRestRequest = requestWithContent(mediaType); - AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.OK); + AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.OK); // register handler that handles custom media type validation restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @@ -1068,7 +1068,7 @@ public void testBrowserSafelistedContentTypesAreRejected() { final String mediaType = randomFrom(RestController.SAFELISTED_MEDIA_TYPES); FakeRestRequest fakeRestRequest = requestWithContent(mediaType); - final AssertingChannel channel = new AssertingChannel(fakeRestRequest, true, RestStatus.NOT_ACCEPTABLE); + final AssertingChannel channel = new AssertingChannel(fakeRestRequest, randomBoolean(), RestStatus.NOT_ACCEPTABLE); restController.registerHandler(new Route(GET, "/foo"), new RestHandler() { @Override @@ -1115,7 +1115,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, randomBoolean(), RestStatus.OK); restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY)); }); } @@ -1137,12 +1137,12 @@ 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, randomBoolean(), RestStatus.OK); restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY)); }); final Consumer> checkProtected = paths -> paths.forEach(path -> { RestRequest request = new FakeRestRequest.Builder(xContentRegistry()).withPath(path).build(); - AssertingChannel channel = new AssertingChannel(request, true, RestStatus.GONE); + AssertingChannel channel = new AssertingChannel(request, randomBoolean(), RestStatus.GONE); restController.dispatchRequest(request, channel, new ThreadContext(Settings.EMPTY)); RestResponse restResponse = channel.getRestResponse(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java b/server/src/test/java/org/elasticsearch/rest/RestHttpResponseHeadersTests.java index 3b839896bc34f..7fe2388ec5113 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, randomBoolean(), 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/action/cat/RestTasksActionTests.java b/server/src/test/java/org/elasticsearch/rest/action/cat/RestTasksActionTests.java index 880a0bc9eabd7..dad6885a08fa8 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, randomBoolean(), 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..f83ba1704f954 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, randomBoolean(), 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 24f59a8c3abe7..d6953e79a0c3f 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 @@ -51,7 +51,7 @@ public void testEnableFieldsEmulationNoErrors() throws Exception { .withParams(params) .build(); - action.handleRequest(request, new FakeRestChannel(request, false, 1), verifyingClient); + action.handleRequest(request, new FakeRestChannel(request, randomBoolean(), 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..d91b4430e4f94 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 Date: Wed, 13 Nov 2024 15:28:50 +0000 Subject: [PATCH 26/28] splotless --- .../http/AbstractHttpServerTransportTests.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java index f209d99bceb7a..fa774c0bcfd12 100644 --- a/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java +++ b/server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java @@ -361,7 +361,11 @@ 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, randomBoolean(), RestStatus.BAD_REQUEST); + RestControllerTests.AssertingChannel channel = new RestControllerTests.AssertingChannel( + fakeRequest, + randomBoolean(), + RestStatus.BAD_REQUEST + ); try ( AbstractHttpServerTransport transport = new AbstractHttpServerTransport( From 108dd01444ffae8cd4a33bb95940b0a81352c19b Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 14 Nov 2024 10:45:37 +0000 Subject: [PATCH 27/28] Update test --- .../test/java/org/elasticsearch/rest/RestResponseTests.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java index cf9de7d986afb..b85ad31288c8c 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -113,8 +113,9 @@ public void testSimpleExceptionMessage() throws Exception { Exception t = new ElasticsearchException("an error occurred reading data", new FileNotFoundException("/foo/bar")); RestResponse response = new RestResponse(channel, t); String text = response.content().utf8ToString(); - assertThat(text, containsString("an error occurred reading data")); - assertThat(text, not(containsString("FileNotFoundException"))); + assertThat(text, containsString(""" + {"type":"exception","reason":"an error occurred reading data"}""")); + assertThat(text, not(containsString("file_not_found_exception"))); assertThat(text, not(containsString("/foo/bar"))); assertThat(text, not(containsString("error_trace"))); } From 9ccb15ce17f61ae744a8aa678559ee6d78910042 Mon Sep 17 00:00:00 2001 From: Simon Cooper Date: Thu, 14 Nov 2024 11:24:19 +0000 Subject: [PATCH 28/28] Don't want stray warnings here --- .../java/org/elasticsearch/test/rest/RestActionTestCase.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java index 941ec816ee3c0..7da0bb247bd8d 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/RestActionTestCase.java @@ -66,7 +66,7 @@ protected RestController controller() { * Sends the given request to the test controller in {@link #controller()}. */ protected void dispatchRequest(RestRequest request) { - FakeRestChannel channel = new FakeRestChannel(request, randomBoolean(), 1); + FakeRestChannel channel = new FakeRestChannel(request, true, 1); ThreadContext threadContext = verifyingClient.threadPool().getThreadContext(); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { controller.dispatchRequest(request, channel, threadContext);