From 7e05b34708c355e47b3bb59f52a7ed71467f9619 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 29 Jan 2016 16:22:37 -0700 Subject: [PATCH] Add Exception class name to message in `NotSerializableExceptionWrapper` Previously it's possible to get errors like: ``` Caused by: NotSerializableExceptionWrapper[d:\shared_data\afs-issue-1-1\23] ``` Which doesn't tell us what the underlying exception type was that could not be serialized. This changes the message to prepend the `ElasticsearchException.getExceptionName()` of the exception (which is a underscore case of the class with the leading 'Elasticsearch' removed) --- .../common/io/stream/NotSerializableExceptionWrapper.java | 3 ++- .../src/test/java/org/elasticsearch/ESExceptionTests.java | 8 +++++--- .../org/elasticsearch/ExceptionSerializationTests.java | 6 +++--- .../transport/AbstractSimpleTransportTestCase.java | 4 ++-- 4 files changed, 12 insertions(+), 9 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 8252fb6d97f..ec83546f571 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 @@ -38,7 +38,8 @@ public final class NotSerializableExceptionWrapper extends ElasticsearchExceptio private final RestStatus status; public NotSerializableExceptionWrapper(Throwable other) { - super(other.getMessage(), other.getCause()); + super(ElasticsearchException.getExceptionName(other) + + ": " + other.getMessage(), other.getCause()); this.name = ElasticsearchException.getExceptionName(other); this.status = ExceptionsHelper.status(other); setStackTrace(other.getStackTrace()); diff --git a/core/src/test/java/org/elasticsearch/ESExceptionTests.java b/core/src/test/java/org/elasticsearch/ESExceptionTests.java index aef38850efc..75a69cd3e55 100644 --- a/core/src/test/java/org/elasticsearch/ESExceptionTests.java +++ b/core/src/test/java/org/elasticsearch/ESExceptionTests.java @@ -286,12 +286,12 @@ public class ESExceptionTests extends ESTestCase { public void testSerializeUnknownException() throws IOException { BytesStreamOutput out = new BytesStreamOutput(); ParsingException ParsingException = new ParsingException(1, 2, "foobar", null); - Throwable ex = new Throwable("wtf", ParsingException); + Throwable ex = new Throwable("eggplant", ParsingException); out.writeThrowable(ex); StreamInput in = StreamInput.wrap(out.bytes()); Throwable throwable = in.readThrowable(); - assertEquals("wtf", throwable.getMessage()); + assertEquals("throwable: eggplant", throwable.getMessage()); assertTrue(throwable instanceof ElasticsearchException); ParsingException e = (ParsingException)throwable.getCause(); assertEquals(ParsingException.getIndex(), e.getIndex()); @@ -329,7 +329,9 @@ public class ESExceptionTests extends ESTestCase { StreamInput in = StreamInput.wrap(out.bytes()); ElasticsearchException e = in.readThrowable(); assertEquals(e.getMessage(), ex.getMessage()); - assertEquals(ex.getCause().getClass().getName(), e.getCause().getMessage(), ex.getCause().getMessage()); + assertTrue("Expected: " + e.getCause().getMessage() + " to contain: " + + ex.getCause().getClass().getName() + " but it didn't", + e.getCause().getMessage().contains(ex.getCause().getMessage())); if (ex.getCause().getClass() != Throwable.class) { // throwable is not directly mapped assertEquals(e.getCause().getClass(), ex.getCause().getClass()); } else { diff --git a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java index 9f8e861f9ce..50764eef65e 100644 --- a/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java +++ b/core/src/test/java/org/elasticsearch/ExceptionSerializationTests.java @@ -543,9 +543,9 @@ public class ExceptionSerializationTests extends ESTestCase { public void testNotSerializableExceptionWrapper() throws IOException { NotSerializableExceptionWrapper ex = serialize(new NotSerializableExceptionWrapper(new NullPointerException())); - assertEquals("{\"type\":\"null_pointer_exception\",\"reason\":null}", toXContent(ex)); + assertEquals("{\"type\":\"null_pointer_exception\",\"reason\":\"null_pointer_exception: null\"}", toXContent(ex)); ex = serialize(new NotSerializableExceptionWrapper(new IllegalArgumentException("nono!"))); - assertEquals("{\"type\":\"illegal_argument_exception\",\"reason\":\"nono!\"}", toXContent(ex)); + assertEquals("{\"type\":\"illegal_argument_exception\",\"reason\":\"illegal_argument_exception: nono!\"}", toXContent(ex)); Throwable[] unknowns = new Throwable[]{ new Exception("foobar"), @@ -586,7 +586,7 @@ public class ExceptionSerializationTests extends ESTestCase { ElasticsearchException serialize = serialize((ElasticsearchException) uhe); assertTrue(serialize instanceof NotSerializableExceptionWrapper); NotSerializableExceptionWrapper e = (NotSerializableExceptionWrapper) serialize; - assertEquals("msg", e.getMessage()); + assertEquals("unknown_header_exception: msg", e.getMessage()); assertEquals(2, e.getHeader("foo").size()); assertEquals("foo", e.getHeader("foo").get(0)); assertEquals("bar", e.getHeader("foo").get(1)); diff --git a/core/src/test/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java b/core/src/test/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java index 747b218b797..4688daec7d9 100644 --- a/core/src/test/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java +++ b/core/src/test/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java @@ -429,7 +429,7 @@ public abstract class AbstractSimpleTransportTestCase extends ESTestCase { @Override public void handleException(TransportException exp) { - assertThat("bad message !!!", equalTo(exp.getCause().getMessage())); + assertThat("runtime_exception: bad message !!!", equalTo(exp.getCause().getMessage())); } }); @@ -437,7 +437,7 @@ public abstract class AbstractSimpleTransportTestCase extends ESTestCase { res.txGet(); fail("exception should be thrown"); } catch (Exception e) { - assertThat(e.getCause().getMessage(), equalTo("bad message !!!")); + assertThat(e.getCause().getMessage(), equalTo("runtime_exception: bad message !!!")); } serviceA.removeHandler("sayHelloException");