Skip to content

Add exception type response header for 5xx errors #130226

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jul 3, 2025
20 changes: 16 additions & 4 deletions server/src/main/java/org/elasticsearch/ElasticsearchException.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<Integer, CheckedFunction<StreamInput, ? extends ElasticsearchException, IOException>> ID_TO_SUPPLIER;
private static final Map<Class<? extends ElasticsearchException>, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE;
Expand All @@ -131,7 +132,6 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
@SuppressWarnings("this-escape")
public ElasticsearchException(Throwable cause) {
super(cause);
maybePutTimeoutHeader();
}

/**
Expand All @@ -146,7 +146,6 @@ public ElasticsearchException(Throwable cause) {
@SuppressWarnings("this-escape")
public ElasticsearchException(String msg, Object... args) {
super(LoggerMessageFormat.format(msg, args));
maybePutTimeoutHeader();
}

/**
Expand All @@ -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")
Expand All @@ -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()));
}
}
}

/**
Expand Down Expand Up @@ -244,6 +250,7 @@ public void addBodyHeader(String key, String... value) {
* Returns a set of all body header keys on this exception
*/
public Set<String> getBodyHeaderKeys() {
maybeAddErrorHeaders();
return bodyHeaders.keySet();
}

Expand All @@ -252,10 +259,12 @@ public Set<String> getBodyHeaderKeys() {
* given key exists.
*/
public List<String> getBodyHeader(String key) {
maybeAddErrorHeaders();
return bodyHeaders.get(key);
}

protected Map<String, List<String>> getBodyHeaders() {
maybeAddErrorHeaders();
return bodyHeaders;
}

Expand All @@ -279,6 +288,7 @@ public void addHttpHeader(String key, String... value) {
* Returns a set of all body header keys on this exception
*/
public Set<String> getHttpHeaderKeys() {
maybeAddErrorHeaders();
return httpHeaders.keySet();
}

Expand All @@ -287,10 +297,12 @@ public Set<String> getHttpHeaderKeys() {
* given key exists.
*/
public List<String> getHttpHeader(String key) {
maybeAddErrorHeaders();
return httpHeaders.get(key);
}

protected Map<String, List<String>> getHttpHeaders() {
maybeAddErrorHeaders();
return httpHeaders;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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": {
Expand All @@ -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");
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Path> visitor = new FileVisitor<Path>() {
private Path pkgPrefix = PathUtils.get(path).getParent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down