From 0ac8c1bc55886010d616ddf97cdde1d60f8d22f0 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Tue, 30 Jun 2015 14:43:42 +0200 Subject: [PATCH] Carry on stacktrace and suppressed exceptions if exception is not serializable --- .../NotSerializableExceptionWrapper.java | 9 +++++- .../common/io/stream/StreamInput.java | 2 ++ .../ElasticsearchExceptionTests.java | 7 +++-- .../ExceptionSerializationTests.java | 31 +++++++++++++++++-- .../hamcrest/ElasticsearchAssertions.java | 27 ++++++++++++++++ 5 files changed, 70 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/NotSerializableExceptionWrapper.java b/core/src/main/java/org/elasticsearch/common/io/stream/NotSerializableExceptionWrapper.java index c77233246ed..7fc63827757 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/NotSerializableExceptionWrapper.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/NotSerializableExceptionWrapper.java @@ -25,7 +25,10 @@ import java.io.IOException; /** * This exception can be used to wrap a given, not serializable exception - * to serialize via {@link StreamOutput#writeThrowable(Throwable)} + * to serialize via {@link StreamOutput#writeThrowable(Throwable)}. + * This class will perserve the stacktrace as well as the suppressed exceptions of + * the throwable it was created with instead of it's own. The stacktrace has no indication + * of where this exception was created. */ public final class NotSerializableExceptionWrapper extends ElasticsearchException { @@ -34,6 +37,10 @@ public final class NotSerializableExceptionWrapper extends ElasticsearchExceptio public NotSerializableExceptionWrapper(Throwable other) { super(other.getMessage(), other.getCause()); this.name = ElasticsearchException.getExceptionName(other); + setStackTrace(other.getStackTrace()); + for (Throwable otherSuppressed : other.getSuppressed()) { + addSuppressed(otherSuppressed); + } } public NotSerializableExceptionWrapper(StreamInput in) throws IOException { diff --git a/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java b/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java index 214b40d1218..8ad880a1c0d 100644 --- a/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java +++ b/core/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java @@ -19,6 +19,8 @@ package org.elasticsearch.common.io.stream; +import com.fasterxml.jackson.core.JsonLocation; +import com.fasterxml.jackson.core.JsonParseException; import org.apache.lucene.index.CorruptIndexException; import org.apache.lucene.index.IndexFormatTooNewException; import org.apache.lucene.index.IndexFormatTooOldException; diff --git a/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java b/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java index cb3f60285e9..acf490b4ce4 100644 --- a/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java +++ b/core/src/test/java/org/elasticsearch/ElasticsearchExceptionTests.java @@ -39,6 +39,8 @@ import org.elasticsearch.indices.IndexMissingException; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchShardTarget; import org.elasticsearch.test.ElasticsearchTestCase; +import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; import org.elasticsearch.transport.RemoteTransportException; import org.junit.Test; @@ -293,9 +295,10 @@ public class ElasticsearchExceptionTests extends ElasticsearchTestCase { assertEquals(e.getCause().getClass(), e.getCause().getClass()); assertArrayEquals(e.getStackTrace(), ex.getStackTrace()); assertTrue(e.getStackTrace().length > 1); + ElasticsearchAssertions.assertVersionSerializable(VersionUtils.randomVersion(getRandom()), t); + ElasticsearchAssertions.assertVersionSerializable(VersionUtils.randomVersion(getRandom()), ex); + ElasticsearchAssertions.assertVersionSerializable(VersionUtils.randomVersion(getRandom()), e); } - - } } \ No newline at end of file diff --git a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java index fa25846c12e..97efcf64d0b 100644 --- a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -18,14 +18,16 @@ */ package org.elasticsearch; +import com.fasterxml.jackson.core.JsonLocation; +import com.fasterxml.jackson.core.JsonParseException; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; +import org.codehaus.groovy.runtime.typehandling.GroovyCastException; import org.elasticsearch.action.FailedNodeException; import org.elasticsearch.action.RoutingMissingException; import org.elasticsearch.action.TimestampParsingException; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.ShardSearchFailure; -import org.elasticsearch.cluster.block.ClusterBlock; import org.elasticsearch.cluster.block.ClusterBlockException; import org.elasticsearch.cluster.metadata.SnapshotId; import org.elasticsearch.cluster.node.DiscoveryNode; @@ -33,7 +35,6 @@ import org.elasticsearch.cluster.routing.*; import org.elasticsearch.common.breaker.CircuitBreakingException; import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.io.stream.*; -import org.elasticsearch.common.transport.InetSocketTransportAddress; import org.elasticsearch.common.transport.LocalTransportAddress; import org.elasticsearch.common.unit.ByteSizeValue; import org.elasticsearch.common.xcontent.*; @@ -63,6 +64,8 @@ import org.elasticsearch.search.warmer.IndexWarmerMissingException; import org.elasticsearch.snapshots.SnapshotException; import org.elasticsearch.test.ElasticsearchTestCase; import org.elasticsearch.test.TestSearchContext; +import org.elasticsearch.test.VersionUtils; +import org.elasticsearch.test.hamcrest.ElasticsearchAssertions; import org.elasticsearch.transport.ActionNotFoundTransportException; import org.elasticsearch.transport.ActionTransportException; import org.elasticsearch.transport.ConnectTransportException; @@ -179,7 +182,8 @@ public class ExceptionSerializationTests extends ElasticsearchTestCase { } } - private T serialize(T exception) throws IOException { + private T serialize(T exception) throws IOException { + ElasticsearchAssertions.assertVersionSerializable(VersionUtils.randomVersion(random()), exception); BytesStreamOutput out = new BytesStreamOutput(); out.writeThrowable(exception); StreamInput in = StreamInput.wrap(out.bytes()); @@ -552,5 +556,26 @@ public class ExceptionSerializationTests extends ElasticsearchTestCase { assertEquals("{\"type\":\"null_pointer_exception\",\"reason\":null}", toXContent(ex)); ex = serialize(new NotSerializableExceptionWrapper(new IllegalArgumentException("nono!"))); assertEquals("{\"type\":\"illegal_argument_exception\",\"reason\":\"nono!\"}", toXContent(ex)); + + Throwable[] unknowns = new Throwable[] { + new JsonParseException("foobar", new JsonLocation(new Object(), 1,2,3,4)), + new GroovyCastException("boom boom boom"), + new IOException("booom") + }; + for (Throwable t : unknowns) { + if (randomBoolean()) { + t.addSuppressed(new IOException("suppressed")); + t.addSuppressed(new NullPointerException()); + } + Throwable deserialized = serialize(t); + assertTrue(deserialized instanceof NotSerializableExceptionWrapper); + assertArrayEquals(t.getStackTrace(), deserialized.getStackTrace()); + assertEquals(t.getSuppressed().length, deserialized.getSuppressed().length); + if (t.getSuppressed().length > 0) { + assertTrue(deserialized.getSuppressed()[0] instanceof NotSerializableExceptionWrapper); + assertArrayEquals(t.getSuppressed()[0].getStackTrace(), deserialized.getSuppressed()[0].getStackTrace()); + assertTrue(deserialized.getSuppressed()[1] instanceof NullPointerException); + } + } } } diff --git a/core/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java b/core/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java index 819c1d5ab1d..f9430edd6b9 100644 --- a/core/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java +++ b/core/src/test/java/org/elasticsearch/test/hamcrest/ElasticsearchAssertions.java @@ -60,6 +60,7 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.io.stream.BytesStreamOutput; import org.elasticsearch.common.io.stream.StreamInput; +import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Streamable; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchHit; @@ -650,6 +651,32 @@ public class ElasticsearchAssertions { } + public static void assertVersionSerializable(Version version, final Throwable t) { + ElasticsearchAssertions.assertVersionSerializable(version, new ThrowableWrapper(t)); + } + + private static final class ThrowableWrapper implements Streamable { + Throwable throwable; + public ThrowableWrapper(Throwable t) { + throwable = t; + } + + public ThrowableWrapper() { + throwable = null; + } + + @Override + public void readFrom(StreamInput in) throws IOException { + throwable = in.readThrowable(); + } + + @Override + public void writeTo(StreamOutput out) throws IOException { + out.writeThrowable(throwable); + } + } + + private static Streamable tryCreateNewInstance(Streamable streamable) throws NoSuchMethodException, InstantiationException, IllegalAccessException, InvocationTargetException { try {