diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 7f41aa61bf1e9..80a3cb77b9f56 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -118,6 +118,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final String ROOT_CAUSE = "root_cause"; static final String TIMED_OUT_HEADER = "X-Timed-Out"; + static final String EXCEPTION_TYPE_HEADER = "X-Elasticsearch-Exception"; private static final Map> ID_TO_SUPPLIER; private static final Map, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE; @@ -131,7 +132,6 @@ public class ElasticsearchException extends RuntimeException implements ToXConte @SuppressWarnings("this-escape") public ElasticsearchException(Throwable cause) { super(cause); - maybePutTimeoutHeader(); } /** @@ -146,7 +146,6 @@ public ElasticsearchException(Throwable cause) { @SuppressWarnings("this-escape") public ElasticsearchException(String msg, Object... args) { super(LoggerMessageFormat.format(msg, args)); - maybePutTimeoutHeader(); } /** @@ -163,7 +162,6 @@ public ElasticsearchException(String msg, Object... args) { @SuppressWarnings("this-escape") public ElasticsearchException(String msg, Throwable cause, Object... args) { super(LoggerMessageFormat.format(msg, args), cause); - maybePutTimeoutHeader(); } @SuppressWarnings("this-escape") @@ -174,11 +172,19 @@ public ElasticsearchException(StreamInput in) throws IOException { metadata.putAll(in.readMapOfLists(StreamInput::readString)); } - private void maybePutTimeoutHeader() { + private void maybeAddErrorHeaders() { if (isTimeout()) { // see https://www.rfc-editor.org/rfc/rfc8941.html#section-4.1.9 for booleans in structured headers bodyHeaders.put(TIMED_OUT_HEADER, List.of("?1")); } + if (httpHeaders.containsKey(EXCEPTION_TYPE_HEADER) == false) { + // TODO: cache unwrapping the cause? we do this in several places... + Throwable cause = unwrapCause(); + RestStatus status = ExceptionsHelper.status(cause); + if (status.getStatus() >= 500) { + httpHeaders.put(EXCEPTION_TYPE_HEADER, List.of(cause.getClass().getSimpleName())); + } + } } /** @@ -244,6 +250,7 @@ public void addBodyHeader(String key, String... value) { * Returns a set of all body header keys on this exception */ public Set getBodyHeaderKeys() { + maybeAddErrorHeaders(); return bodyHeaders.keySet(); } @@ -252,10 +259,12 @@ public Set getBodyHeaderKeys() { * given key exists. */ public List getBodyHeader(String key) { + maybeAddErrorHeaders(); return bodyHeaders.get(key); } protected Map> getBodyHeaders() { + maybeAddErrorHeaders(); return bodyHeaders; } @@ -279,6 +288,7 @@ public void addHttpHeader(String key, String... value) { * Returns a set of all body header keys on this exception */ public Set getHttpHeaderKeys() { + maybeAddErrorHeaders(); return httpHeaders.keySet(); } @@ -287,10 +297,12 @@ public Set getHttpHeaderKeys() { * given key exists. */ public List getHttpHeader(String key) { + maybeAddErrorHeaders(); return httpHeaders.get(key); } protected Map> getHttpHeaders() { + maybeAddErrorHeaders(); return httpHeaders; } diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index d0e21fad5b4c7..2741ac37ce519 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -1553,19 +1553,24 @@ private void testExceptionLoop(Exception rootException) throws IOException { assertArrayEquals(ser.getStackTrace(), rootException.getStackTrace()); } - static class ExceptionSubclass extends ElasticsearchException { + static class TimeoutSubclass extends ElasticsearchException { + TimeoutSubclass(String message) { + super(message); + } + @Override public boolean isTimeout() { return true; } - ExceptionSubclass(String message) { - super(message); + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; } } - public void testTimeout() throws IOException { - var e = new ExceptionSubclass("some timeout"); + public void testTimeoutHeader() throws IOException { + var e = new TimeoutSubclass("some timeout"); assertThat(e.getBodyHeaderKeys(), hasItem(ElasticsearchException.TIMED_OUT_HEADER)); XContentBuilder builder = XContentFactory.jsonBuilder(); @@ -1574,7 +1579,7 @@ public void testTimeout() throws IOException { builder.endObject(); String expected = """ { - "type": "exception_subclass", + "type": "timeout_subclass", "reason": "some timeout", "timed_out": true, "header": { @@ -1584,6 +1589,44 @@ public void testTimeout() throws IOException { assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder)); } + static class Exception5xx extends ElasticsearchException { + Exception5xx(String message) { + super(message); + } + + @Override + public RestStatus status() { + return RestStatus.INTERNAL_SERVER_ERROR; + } + } + + static class Exception4xx extends ElasticsearchException { + Exception4xx(String message) { + super(message); + } + + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } + } + + public void testExceptionTypeHeader() throws IOException { + var e = new Exception5xx("some exception"); + assertThat(e.getHttpHeaderKeys(), hasItem(ElasticsearchException.EXCEPTION_TYPE_HEADER)); + + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + e.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + String expected = """ + { + "type": "exception5xx", + "reason": "some exception" + }"""; + assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder)); + } + public void testHttpHeaders() throws IOException { var e = new ElasticsearchException("some exception"); e.addHttpHeader("My-Header", "value"); @@ -1592,13 +1635,19 @@ public void testHttpHeaders() throws IOException { assertThat(e.getHttpHeaders(), hasEntry("My-Header", List.of("value"))); // ensure http headers are not written to response body + } + + public void testNoExceptionTypeHeaderOn4xx() throws IOException { + var e = new Exception4xx("some exception"); + assertThat(e.getHttpHeaderKeys(), not(hasItem(ElasticsearchException.EXCEPTION_TYPE_HEADER))); + XContentBuilder builder = XContentFactory.jsonBuilder(); builder.startObject(); e.toXContent(builder, ToXContent.EMPTY_PARAMS); builder.endObject(); String expected = """ { - "type": "exception", + "type": "exception4xx", "reason": "some exception" }"""; assertEquals(XContentHelper.stripWhitespace(expected), Strings.toString(builder)); diff --git a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java index 8497871dad006..64c0b9a780cfe 100644 --- a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -148,7 +148,9 @@ public void testExceptionRegistration() throws IOException, URISyntaxException { CancellableThreadsTests.CustomException.class, RestResponseTests.WithHeadersException.class, AbstractClientHeadersTestCase.InternalException.class, - ElasticsearchExceptionTests.ExceptionSubclass.class + ElasticsearchExceptionTests.TimeoutSubclass.class, + ElasticsearchExceptionTests.Exception4xx.class, + ElasticsearchExceptionTests.Exception5xx.class ); FileVisitor visitor = new FileVisitor() { private Path pkgPrefix = PathUtils.get(path).getParent(); diff --git a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java index 2a8025d059cc3..c22698ec4f926 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestResponseTests.java @@ -539,6 +539,11 @@ public static class WithHeadersException extends ElasticsearchException { this.addHttpHeader("My-Header", "v1"); this.addMetadata("es.test", "value1", "value2"); } + + @Override + public RestStatus status() { + return RestStatus.BAD_REQUEST; + } } private static class SimpleExceptionRestChannel extends AbstractRestChannel {