Merge pull request #33 from palantir/feature/exception-stacktrace

SerializableErrorErrorDecoder wraps remote exception in local excepti…

http-clients/src/main/java/com/palantir/remoting/http/errors/SerializableErrorErrorDecoder.java

@@ -25,6 +25,7 @@
 import feign.codec.ErrorDecoder;
 import java.io.IOException;
 import java.io.Reader;
+import java.lang.reflect.InvocationTargetException;
 import java.util.Collection;
 import java.util.List;
 import javax.ws.rs.core.MediaType;
@@ -56,23 +57,32 @@ public Exception decode(String methodKey, Response response) {
             if (contentType.contains(MediaType.APPLICATION_JSON)) {
                 String bodyAsString = readBodyAsString(body);
 
+                SerializableError error;
                 try {
-                    SerializableError error = MAPPER.readValue(bodyAsString, SerializableError.class);
-                    Class<? extends Exception> exceptionClass = error.getExceptionClass();
-                    Exception exception = exceptionClass.getConstructor(String.class).newInstance(error.getMessage());
-                    List<StackTraceElement> stackTrace = error.getStackTrace();
-                    if (stackTrace != null) {
-                        exception.setStackTrace(stackTrace.toArray(new StackTraceElement[stackTrace.size()]));
-                    }
-                    return exception;
+                    error = MAPPER.readValue(bodyAsString, SerializableError.class);
                 } catch (Exception e) {
-                    log.error("Failed to parse error body as GenericError.", e);
-                    String message =
-                            String.format("Error %s. Reason: %s. Failed to parse error body: %s. Body:%n%s",
-                                    response.status(),
-                                    response.reason(), e.getMessage(), bodyAsString);
+                    log.error("Failed to parse error body as SerializableError.", e);
+                    String message = String.format(
+                            "Error %s. Reason: %s. Failed to parse error body and instantiate exception: %s. Body:%n%s",
+                            response.status(), response.reason(), e.getMessage(), bodyAsString);
                     return new RuntimeException(message);
                 }
+
+                // Construct remote exception and fill with remote stacktrace
+                Exception remoteException = constructException(error.getExceptionClass(), error.getMessage());
+                List<StackTraceElement> stackTrace = error.getStackTrace();
+                if (stackTrace != null) {
+                    remoteException.setStackTrace(stackTrace.toArray(new StackTraceElement[stackTrace.size()]));
+                }
+
+                // Construct local exception that wraps the remote exception and fill with stack trace of local
+                // call (yet without the reflection overhead).
+                Exception localException =
+                        constructException(error.getExceptionClass(), error.getMessage(), remoteException);
+                localException.fillInStackTrace();
+
+                return localException;
+
             } else if (contentType.contains(MediaType.TEXT_HTML) || contentType.contains(MediaType.TEXT_PLAIN)
                     || contentType.contains(MediaType.TEXT_XML)) {
                 String bodyAsString = readBodyAsString(body);
@@ -90,6 +100,35 @@ public Exception decode(String methodKey, Response response) {
         }
     }
 
+    private static Exception constructException(Class<? extends Exception> exceptionClass, String message) {
+        try {
+            return exceptionClass.getConstructor(String.class).newInstance(message);
+        } catch (Exception e1) {
+            return new RuntimeException(String.format(
+                    "Failed to construction exception of type %s, constructing RuntimeException instead: %s%n%s",
+                    exceptionClass.toString(), e1.toString(), message));
+        }
+    }
+
+    private static Exception constructException(
+            Class<? extends Exception> exceptionClass, String message, Throwable wrappedException) {
+        // Note: If another constructor is added, then we should refactor the construction logic in order to avoid
+        // nested try/catch
+        try {
+            return exceptionClass.getConstructor(String.class, Throwable.class).newInstance(message, wrappedException);
+        } catch (NoSuchMethodException | InstantiationException
+                | IllegalAccessException | InvocationTargetException e) {
+            try {
+                return exceptionClass.getConstructor(String.class).newInstance(message);
+            } catch (Exception e1) {
+                return new RuntimeException(String.format(
+                        "Failed to wrap exception as %s, wrapping exception as RuntimeException instead: %s%n%s",
+                        exceptionClass.toString(), e1.toString(), message),
+                        wrappedException);
+            }
+        }
+    }
+
     /*
      * Reads the response body fully into a string so that if there are exceptions parsing the body we can at least show
      * the string representation of it.

http-clients/src/test/java/com/palantir/remoting/http/errors/SerializableErrorErrorDecoderTests.java

@@ -23,9 +23,10 @@
 import com.fasterxml.jackson.core.JsonProcessingException;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Lists;
 import feign.Response;
-import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import javax.annotation.CheckForNull;
 import javax.ws.rs.core.HttpHeaders;
 import javax.ws.rs.core.MediaType;
@@ -75,7 +76,7 @@ public void testExceptionInErrorParsing() {
         Exception decode = decoder.decode("ignored", response);
         assertThat(decode, is(instanceOf(RuntimeException.class)));
         assertThat(decode.getMessage(), is(
-                "Error 400. Reason: reason. Failed to parse error body: "
+                "Error 400. Reason: reason. Failed to parse error body and instantiate exception: "
                         + "Unrecognized token 'notjsonifiable': was expecting 'null', 'true', 'false' or NaN\n "
                         + "at [Source: notjsonifiable!; line: 1, column: 15]. Body:\n"
                         + "notjsonifiable!"));
@@ -89,9 +90,25 @@ public void testNullBody() {
         assertThat(decode.getMessage(), is("400 reason"));
     }
 
+    @Test
+    public void testClientExceptionWrapsServerException() throws JsonProcessingException {
+        Object error = SerializableError.of("msg", IllegalArgumentException.class,
+                Lists.newArrayList(new RuntimeException().getStackTrace()));
+        String json = new ObjectMapper().writeValueAsString(error);
+        Response response = getResponse(MediaType.APPLICATION_JSON, json);
+        Exception decode = decoder.decode("ignored", response);
+
+        assertThat(decode, is(instanceOf(IllegalArgumentException.class)));
+        assertThat(decode.getMessage(), is("msg"));
+        assertThat(decode.getStackTrace()[0].getMethodName(), is("decode"));
+        assertThat(decode.getCause(), is(instanceOf(IllegalArgumentException.class)));
+        assertThat(decode.getCause().getMessage(), is("msg"));
+        assertThat(decode.getCause().getStackTrace()[0].getMethodName(), is("testClientExceptionWrapsServerException"));
+    }
+
     private static Response getResponse(String contentType, @CheckForNull String body) {
         return Response.create(400, "reason", ImmutableMap.<String, Collection<String>>of(HttpHeaders.CONTENT_TYPE,
-                Arrays.asList(contentType)), body, feign.Util.UTF_8);
+                Collections.singletonList(contentType)), body, feign.Util.UTF_8);
     }
 
 }