From 647d0a1e9518a02670087003145e40e9ee1620a1 Mon Sep 17 00:00:00 2001
From: Jason Tedor <jason@tedor.me>
Date: Tue, 13 Mar 2018 23:42:16 -0400
Subject: [PATCH] Do not swallow fail to convert exceptions (#29043)

When converting the source for an indexing request to JSON, the
conversion can throw an I/O exception which we swallow and proceed with
logging to the slow log. The cause of the I/O exception is lost. This
commit changes this behavior and chooses to drop the entry from the slow
logs and instead lets an exception percolate up to the indexing
operation listener loop. Here, the exception will be caught and logged
at the warn level.
---
 .../org/elasticsearch/index/IndexingSlowLog.java     |  8 ++++++++
 .../elasticsearch/index/IndexingSlowLogTests.java    | 12 ++++++++++--
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java
index 94c3892ef36..b75cda5b6ca 100644
--- a/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java
+++ b/server/src/main/java/org/elasticsearch/index/IndexingSlowLog.java
@@ -33,6 +33,8 @@ import org.elasticsearch.index.shard.IndexingOperationListener;
 import org.elasticsearch.index.shard.ShardId;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.util.Locale;
 import java.util.concurrent.TimeUnit;
 
 public final class IndexingSlowLog implements IndexingOperationListener {
@@ -194,6 +196,12 @@ public final class IndexingSlowLog implements IndexingOperationListener {
                 sb.append(", source[").append(Strings.cleanTruncate(source, maxSourceCharsToLog)).append("]");
             } catch (IOException e) {
                 sb.append(", source[_failed_to_convert_[").append(e.getMessage()).append("]]");
+                /*
+                 * We choose to fail to write to the slow log and instead let this percolate up to the post index listener loop where this
+                 * will be logged at the warn level.
+                 */
+                final String message = String.format(Locale.ROOT, "failed to convert source for slow log entry [%s]", sb.toString());
+                throw new UncheckedIOException(message, e);
             }
             return sb.toString();
         }
diff --git a/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java b/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java
index 45b0d0aa247..ff5166e8f1a 100644
--- a/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java
+++ b/server/src/test/java/org/elasticsearch/index/IndexingSlowLogTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.index;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import org.apache.lucene.document.NumericDocValuesField;
 import org.elasticsearch.Version;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
@@ -34,6 +35,7 @@ import org.elasticsearch.index.mapper.SeqNoFieldMapper;
 import org.elasticsearch.test.ESTestCase;
 
 import java.io.IOException;
+import java.io.UncheckedIOException;
 
 import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasToString;
@@ -70,9 +72,15 @@ public class IndexingSlowLogTests extends ESTestCase {
             "test", null, null, source, XContentType.JSON, null);
         p = new SlowLogParsedDocumentPrinter(index, pd, 10, true, 3);
 
-        assertThat(p.toString(), containsString("_failed_to_convert_[Unrecognized token 'invalid':"
+        final UncheckedIOException e = expectThrows(UncheckedIOException.class, p::toString);
+        assertThat(e, hasToString(containsString("_failed_to_convert_[Unrecognized token 'invalid':"
             + " was expecting ('true', 'false' or 'null')\n"
-            + " at [Source: org.elasticsearch.common.bytes.BytesReference$MarkSupportingStreamInputWrapper"));
+            + " at [Source: org.elasticsearch.common.bytes.BytesReference$MarkSupportingStreamInputWrapper")));
+        assertNotNull(e.getCause());
+        assertThat(e.getCause(), instanceOf(JsonParseException.class));
+        assertThat(e.getCause(), hasToString(containsString("Unrecognized token 'invalid':"
+                + " was expecting ('true', 'false' or 'null')\n"
+                + " at [Source: org.elasticsearch.common.bytes.BytesReference$MarkSupportingStreamInputWrapper")));
     }
 
     public void testReformatSetting() {