Indicate when errors represent timeouts (elastic#124936) (elastic#125056)

This commit adds a `timed_out` key to the error responses that represent a
timeout condition. It also adds an `X-Timed-Out` header to the response
indicating the same outside the response body.

Author: rjernst

docs/changelog/124936.yaml

@@ -0,0 +1,5 @@
+pr: 124936
+summary: Indicate when errors represent timeouts
+area: Infra/REST API
+type: enhancement
+issues: []

server/src/main/java/org/elasticsearch/ElasticsearchException.java

@@ -108,13 +108,16 @@ 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";
     private static final String HEADER = "header";
     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<Integer, CheckedFunction<StreamInput, ? extends ElasticsearchException, IOException>> ID_TO_SUPPLIER;
     private static final Map<Class<? extends ElasticsearchException>, ElasticsearchExceptionHandle> CLASS_TO_ELASTICSEARCH_EXCEPTION_HANDLE;
     private final Map<String, List<String>> metadata = new HashMap<>();
@@ -123,8 +126,10 @@ public class ElasticsearchException extends RuntimeException implements ToXConte
     /**
      * Construct a <code>ElasticsearchException</code> 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<String, List<String>> entry : metadata.entrySet()) {
             headerToXContent(builder, entry.getKey().substring("es.".length()), entry.getValue());
         }

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;
+    }
 }

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;
+    }
 }

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

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));
+    }
 }

server/src/test/java/org/elasticsearch/ExceptionSerializationTests.java

@@ -146,7 +146,8 @@ public void testExceptionRegistration() throws IOException, URISyntaxException {
         final Set<? extends Class<?>> ignore = Sets.newHashSet(
             CancellableThreadsTests.CustomException.class,
             RestResponseTests.WithHeadersException.class,
-            AbstractClientHeadersTestCase.InternalException.class
+            AbstractClientHeadersTestCase.InternalException.class,
+            ElasticsearchExceptionTests.ExceptionSubclass.class
         );
         FileVisitor<Path> visitor = new FileVisitor<Path>() {
             private Path pkgPrefix = PathUtils.get(path).getParent();