From a869342910ccee00752d366d5dee2a23b3c065c0 Mon Sep 17 00:00:00 2001
From: Tanguy Leroux <tlrx.dev@gmail.com>
Date: Wed, 7 Aug 2019 10:35:53 +0200
Subject: [PATCH] Restore DefaultShardOperationFailedException's reason after
 deserialization (#45203)

The reason field of DefaultShardOperationFailedException is lost during serialization.
This is sad because this field is checked for nullity during xcontent generation and it
means that the cause won't be included in the generated xcontent and won't be
printed in two REST API responses (Close Index API and Indices Shard Stores API).

This commit simply restores the reason from the cause during deserialization.
---
 .../DefaultShardOperationFailedException.java |  1 +
 .../close/CloseIndexResponseTests.java        |  1 +
 ...ultShardOperationFailedExceptionTests.java | 69 +++++++++++++++++++
 3 files changed, 71 insertions(+)

diff --git a/server/src/main/java/org/elasticsearch/action/support/DefaultShardOperationFailedException.java b/server/src/main/java/org/elasticsearch/action/support/DefaultShardOperationFailedException.java
index cfdc463892f..7ac9965fa88 100644
--- a/server/src/main/java/org/elasticsearch/action/support/DefaultShardOperationFailedException.java
+++ b/server/src/main/java/org/elasticsearch/action/support/DefaultShardOperationFailedException.java
@@ -79,6 +79,7 @@ public class DefaultShardOperationFailedException extends ShardOperationFailedEx
         f.shardId = in.readVInt();
         f.cause = in.readException();
         f.status = RestStatus.readFrom(in);
+        f.reason = detailedMessage(f.cause);
     }
 
     @Override
diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java
index 7ca56ecf14f..21f9844182f 100644
--- a/server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java
+++ b/server/src/test/java/org/elasticsearch/action/admin/indices/close/CloseIndexResponseTests.java
@@ -113,6 +113,7 @@ public class CloseIndexResponseTests extends AbstractWireSerializingTestCase<Clo
                             assertThat(actualFailure.getNodeId(), equalTo(expectedFailure.getNodeId()));
                             assertThat(actualFailure.index(), equalTo(expectedFailure.index()));
                             assertThat(actualFailure.shardId(), equalTo(expectedFailure.shardId()));
+                            assertThat(actualFailure.reason(), equalTo(expectedFailure.reason()));
                             assertThat(actualFailure.getCause().getMessage(), equalTo(expectedFailure.getCause().getMessage()));
                             assertThat(actualFailure.getCause().getClass(), equalTo(expectedFailure.getCause().getClass()));
                             assertArrayEquals(actualFailure.getCause().getStackTrace(), expectedFailure.getCause().getStackTrace());
diff --git a/server/src/test/java/org/elasticsearch/action/support/DefaultShardOperationFailedExceptionTests.java b/server/src/test/java/org/elasticsearch/action/support/DefaultShardOperationFailedExceptionTests.java
index 8241628b550..08761dff2fc 100644
--- a/server/src/test/java/org/elasticsearch/action/support/DefaultShardOperationFailedExceptionTests.java
+++ b/server/src/test/java/org/elasticsearch/action/support/DefaultShardOperationFailedExceptionTests.java
@@ -19,10 +19,18 @@
 
 package org.elasticsearch.action.support;
 
+import org.apache.lucene.index.CorruptIndexException;
+import org.apache.lucene.index.IndexFormatTooNewException;
+import org.apache.lucene.index.IndexFormatTooOldException;
+import org.apache.lucene.store.AlreadyClosedException;
+import org.apache.lucene.store.LockObtainFailedException;
 import org.elasticsearch.ElasticsearchException;
 import org.elasticsearch.action.support.broadcast.BroadcastShardOperationFailedException;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.io.stream.BytesStreamOutput;
+import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
 import org.elasticsearch.common.xcontent.XContent;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
@@ -32,7 +40,12 @@ import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.test.ESTestCase;
 
+import java.io.EOFException;
+import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.util.function.Supplier;
+
+import static org.hamcrest.Matchers.equalTo;
 
 public class DefaultShardOperationFailedExceptionTests extends ESTestCase {
 
@@ -110,4 +123,60 @@ public class DefaultShardOperationFailedExceptionTests extends ESTestCase {
         assertEquals(parsed.status(), RestStatus.INTERNAL_SERVER_ERROR);
         assertEquals(parsed.getCause().getMessage(), "Elasticsearch exception [type=exception, reason=foo]");
     }
+
+    public void testSerialization() throws Exception {
+        final DefaultShardOperationFailedException exception = randomInstance();
+        try (BytesStreamOutput out = new BytesStreamOutput()) {
+            exception.writeTo(out);
+
+            try (StreamInput in = out.bytes().streamInput()) {
+                final DefaultShardOperationFailedException deserializedException = new DefaultShardOperationFailedException(in);
+                assertNotSame(exception, deserializedException);
+                assertThat(deserializedException.index(), equalTo(exception.index()));
+                assertThat(deserializedException.shardId(), equalTo(exception.shardId()));
+                assertThat(deserializedException.reason(), equalTo(exception.reason()));
+                assertThat(deserializedException.getCause().getMessage(), equalTo(exception.getCause().getMessage()));
+                assertThat(deserializedException.getCause().getClass(), equalTo(exception.getCause().getClass()));
+                assertArrayEquals(deserializedException.getCause().getStackTrace(), exception.getCause().getStackTrace());
+            }
+        }
+    }
+
+    private static DefaultShardOperationFailedException randomInstance() {
+        final Exception cause = randomException();
+        if (cause instanceof ElasticsearchException) {
+            return new DefaultShardOperationFailedException((ElasticsearchException) cause);
+        } else {
+            return new DefaultShardOperationFailedException(randomAlphaOfLengthBetween(1, 5), randomIntBetween(0, 10), cause);
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private static Exception randomException() {
+        Supplier<Exception> supplier = randomFrom(
+            () -> new CorruptIndexException(randomAlphaOfLengthBetween(1, 5), randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new NullPointerException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new NumberFormatException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new IllegalArgumentException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new AlreadyClosedException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new EOFException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new SecurityException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new StringIndexOutOfBoundsException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new ArrayIndexOutOfBoundsException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new StringIndexOutOfBoundsException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new FileNotFoundException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new IllegalStateException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new LockObtainFailedException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new InterruptedException(randomAlphaOfLengthBetween(1, 5)),
+            () -> new IOException(randomAlphaOfLengthBetween(1, 5), randomExceptionOrNull()),
+            () -> new EsRejectedExecutionException(randomAlphaOfLengthBetween(1, 5), randomBoolean()),
+            () -> new IndexFormatTooNewException(randomAlphaOfLengthBetween(1, 10), randomInt(), randomInt(), randomInt()),
+            () -> new IndexFormatTooOldException(randomAlphaOfLengthBetween(1, 5), randomAlphaOfLengthBetween(1, 5))
+        );
+        return supplier.get();
+    }
+
+    private static Exception randomExceptionOrNull() {
+        return randomBoolean() ? randomException() : null;
+    }
 }