diff --git a/docs/changelog/124936.yaml b/docs/changelog/124936.yaml new file mode 100644 index 0000000000000..2316b29310062 --- /dev/null +++ b/docs/changelog/124936.yaml @@ -0,0 +1,5 @@ +pr: 124936 +summary: Indicate when errors represent timeouts +area: Infra/REST API +type: enhancement +issues: [] diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 56083902c3cc2..995b18a5f3ace 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -108,6 +108,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final String TYPE = "type"; private static final String REASON = "reason"; + private static final String TIMED_OUT = "timed_out"; private static final String CAUSED_BY = "caused_by"; private static final ParseField SUPPRESSED = new ParseField("suppressed"); public static final String STACK_TRACE = "stack_trace"; @@ -115,6 +116,8 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final String ERROR = "error"; private static final String ROOT_CAUSE = "root_cause"; + static final String TIMED_OUT_HEADER = "X-Timed-Out"; + private static final Map> ID_TO_SUPPLIER; private static final Map, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE; private final Map> metadata = new HashMap<>(); @@ -123,8 +126,10 @@ public class ElasticsearchException extends RuntimeException implements ToXConte /** * Construct a ElasticsearchException with the specified cause exception. */ + @SuppressWarnings("this-escape") public ElasticsearchException(Throwable cause) { super(cause); + maybePutTimeoutHeader(); } /** @@ -136,8 +141,10 @@ public ElasticsearchException(Throwable cause) { * @param msg the detail message * @param args the arguments for the message */ + @SuppressWarnings("this-escape") public ElasticsearchException(String msg, Object... args) { super(LoggerMessageFormat.format(msg, args)); + maybePutTimeoutHeader(); } /** @@ -151,8 +158,10 @@ public ElasticsearchException(String msg, Object... args) { * @param cause the nested exception * @param args the arguments for the message */ + @SuppressWarnings("this-escape") public ElasticsearchException(String msg, Throwable cause, Object... args) { super(LoggerMessageFormat.format(msg, args), cause); + maybePutTimeoutHeader(); } @SuppressWarnings("this-escape") @@ -163,6 +172,13 @@ public ElasticsearchException(StreamInput in) throws IOException { metadata.putAll(in.readMapOfLists(StreamInput::readString)); } + private void maybePutTimeoutHeader() { + if (isTimeout()) { + // see https://www.rfc-editor.org/rfc/rfc8941.html#section-4.1.9 for booleans in structured headers + headers.put(TIMED_OUT_HEADER, List.of("?1")); + } + } + /** * Adds a new piece of metadata with the given key. * If the provided key is already present, the corresponding metadata will be replaced @@ -253,6 +269,13 @@ public RestStatus status() { } } + /** + * Returns whether this exception represents a timeout. + */ + public boolean isTimeout() { + return false; + } + /** * Unwraps the actual cause from the exception for cases when the exception is a * {@link ElasticsearchWrapperException}. @@ -386,6 +409,15 @@ protected static void innerToXContent( builder.field(TYPE, type); builder.field(REASON, message); + boolean timedOut = false; + if (throwable instanceof ElasticsearchException exception) { + // TODO: we could walk the exception chain to see if _any_ causes are timeouts? + timedOut = exception.isTimeout(); + } + if (timedOut) { + builder.field(TIMED_OUT, timedOut); + } + for (Map.Entry> entry : metadata.entrySet()) { headerToXContent(builder, entry.getKey().substring("es.".length()), entry.getValue()); } diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchTimeoutException.java b/server/src/main/java/org/elasticsearch/ElasticsearchTimeoutException.java index 06ae43144476e..7e4735bf7b3b0 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchTimeoutException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchTimeoutException.java @@ -41,4 +41,9 @@ public RestStatus status() { // closest thing to "your request took longer than you asked for" return RestStatus.TOO_MANY_REQUESTS; } + + @Override + public boolean isTimeout() { + return true; + } } diff --git a/server/src/main/java/org/elasticsearch/cluster/metadata/ProcessClusterEventTimeoutException.java b/server/src/main/java/org/elasticsearch/cluster/metadata/ProcessClusterEventTimeoutException.java index 2a273f7f81e0f..085b415a4579d 100644 --- a/server/src/main/java/org/elasticsearch/cluster/metadata/ProcessClusterEventTimeoutException.java +++ b/server/src/main/java/org/elasticsearch/cluster/metadata/ProcessClusterEventTimeoutException.java @@ -30,4 +30,9 @@ public ProcessClusterEventTimeoutException(StreamInput in) throws IOException { public RestStatus status() { return RestStatus.TOO_MANY_REQUESTS; } + + @Override + public boolean isTimeout() { + return true; + } } diff --git a/server/src/main/java/org/elasticsearch/search/query/SearchTimeoutException.java b/server/src/main/java/org/elasticsearch/search/query/SearchTimeoutException.java index e5caa00537c67..1ad063995dc60 100644 --- a/server/src/main/java/org/elasticsearch/search/query/SearchTimeoutException.java +++ b/server/src/main/java/org/elasticsearch/search/query/SearchTimeoutException.java @@ -18,7 +18,7 @@ /** * Specific instance of {@link SearchException} that indicates that a search timeout occurred. - * Always returns http status 504 (Gateway Timeout) + * Always returns http status 429 (Too Many Requests) */ public class SearchTimeoutException extends SearchException { public SearchTimeoutException(SearchShardTarget shardTarget, String msg) { @@ -34,6 +34,11 @@ public RestStatus status() { return RestStatus.TOO_MANY_REQUESTS; } + @Override + public boolean isTimeout() { + return true; + } + /** * Propagate a timeout according to whether partial search results are allowed or not. * In case partial results are allowed, a flag will be set to the provided {@link QuerySearchResult} to indicate that there was a diff --git a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index f5a23cf68a26e..76e7be430b012 100644 --- a/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/server/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -1552,4 +1552,35 @@ private void testExceptionLoop(Exception rootException) throws IOException { assertThat(ser.getMessage(), equalTo(rootException.getMessage())); assertArrayEquals(ser.getStackTrace(), rootException.getStackTrace()); } + + static class ExceptionSubclass extends ElasticsearchException { + @Override + public boolean isTimeout() { + return true; + } + + ExceptionSubclass(String message) { + super(message); + } + } + + public void testTimeout() throws IOException { + var e = new ExceptionSubclass("some timeout"); + assertThat(e.getHeaderKeys(), hasItem(ElasticsearchException.TIMED_OUT_HEADER)); + + XContentBuilder builder = XContentFactory.jsonBuilder(); + builder.startObject(); + e.toXContent(builder, ToXContent.EMPTY_PARAMS); + builder.endObject(); + String expected = """ + { + "type": "exception_subclass", + "reason": "some timeout", + "timed_out": true, + "header": { + "X-Timed-Out": "?1" + } + }"""; + 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 31f54f9a16359..35fce1b20241b 100644 --- a/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -146,7 +146,8 @@ public void testExceptionRegistration() throws IOException, URISyntaxException { final Set> ignore = Sets.newHashSet( CancellableThreadsTests.CustomException.class, RestResponseTests.WithHeadersException.class, - AbstractClientHeadersTestCase.InternalException.class + AbstractClientHeadersTestCase.InternalException.class, + ElasticsearchExceptionTests.ExceptionSubclass.class ); FileVisitor visitor = new FileVisitor() { private Path pkgPrefix = PathUtils.get(path).getParent();