Repository: kudu Updated Branches: refs/heads/branch-1.6.x 353152d49 -> a3aa10978
KUDU-2230: java: sync client exception stack traces should make sense Previously the exceptions thrown by the synchronous client always had a stack trace somewhere deep inside our async callback chain. So, when the user eventually caught this exception, it would be very hard to even know what part of their code failed, since their own code would not even show up in the call chain. This patch simply replaces the exception's stack trace with the stack at the "join" point where the exception is transformed back to a KuduException. An exception which stores the original stack trace is attached as a "suppressed" exception. Change-Id: I874d47b5239bcc8493c15ef763ecfe0fd878d795 Reviewed-on: http://gerrit.cloudera.org:8080/9489 Tested-by: Kudu Jenkins Reviewed-by: Dan Burkert <[email protected]> Reviewed-by: Alexey Serbin <[email protected]> Reviewed-on: http://gerrit.cloudera.org:8080/9825 Reviewed-by: Grant Henke <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d602d30b Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d602d30b Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d602d30b Branch: refs/heads/branch-1.6.x Commit: d602d30b39c120e56a98dc61ce744e0c953af7b0 Parents: 353152d Author: Todd Lipcon <[email protected]> Authored: Mon Mar 5 00:00:55 2018 -0800 Committer: Will Berkeley <[email protected]> Committed: Wed Mar 28 01:41:34 2018 +0000 ---------------------------------------------------------------------- .../org/apache/kudu/client/KuduException.java | 27 +++++++++++++++++++- .../kudu/client/TestConnectToCluster.java | 5 +++- 2 files changed, 30 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/d602d30b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java index 0f1ea97..d6dacf2 100644 --- a/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java +++ b/java/kudu-client/src/main/java/org/apache/kudu/client/KuduException.java @@ -44,7 +44,6 @@ import org.apache.yetus.audience.InterfaceStability; @InterfaceStability.Evolving @SuppressWarnings("serial") public abstract class KuduException extends IOException { - private final Status status; /** @@ -76,6 +75,24 @@ public abstract class KuduException extends IOException { } /** + * When exceptions are thrown by the asynchronous Kudu client, the stack trace is + * typically deep within the internals of the Kudu client and/or Netty. + * Thus, when the synchronous Kudu client wraps and throws the exception, + * we suppress that stack trace and replace it with the stack trace of the user's + * calling thread. The original stack trace is added to the {@link KuduException} + * as a suppressed exception ({@see Throwable#addSuppressed(Throwable)}) of + * this + */ + @InterfaceAudience.Public + @InterfaceStability.Evolving + public static class OriginalException extends Throwable { + private OriginalException(Throwable e) { + super("Original asynchronous stack trace"); + setStackTrace(e.getStackTrace()); + } + } + + /** * Inspects the given exception and transforms it into a KuduException. * @param e generic exception we want to transform * @return a KuduException that's easier to handle @@ -84,6 +101,14 @@ public abstract class KuduException extends IOException { // The message may be null. String message = e.getMessage() == null ? "" : e.getMessage(); if (e instanceof KuduException) { + // The exception thrown inside the async code has a stack trace + // that doesn't correspond to where the user actually called + // some synchronous method. This can be very confusing for + // users, so we'll reset the stack trace back the call frame + // where we are transforming it. + e.addSuppressed(new OriginalException(e)); + StackTraceElement[] stack = new Exception().getStackTrace(); + e.setStackTrace(stack); return (KuduException) e; } else if (e instanceof DeferredGroupException) { // The cause of a DeferredGroupException is the first exception it sees, we're just going to http://git-wip-us.apache.org/repos/asf/kudu/blob/d602d30b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java ---------------------------------------------------------------------- diff --git a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java index d467c8c..907e987 100644 --- a/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java +++ b/java/kudu-client/src/test/java/org/apache/kudu/client/TestConnectToCluster.java @@ -24,15 +24,16 @@ import static org.junit.Assert.fail; import java.util.List; +import com.google.common.base.Joiner; import com.google.common.collect.ImmutableList; import com.google.common.net.HostAndPort; import com.stumbleupon.async.Callback; import org.junit.Assert; import org.junit.Test; - import org.apache.kudu.consensus.Metadata; import org.apache.kudu.master.Master.ConnectToMasterResponsePB; +import org.hamcrest.CoreMatchers; public class TestConnectToCluster { @@ -97,6 +98,8 @@ public class TestConnectToCluster { ".*Client configured with 1 master\\(s\\) " + "\\(.+?\\) but cluster indicates it expects 3 master\\(s\\) " + "\\(.+?,.+?,.+?\\).*")); + Assert.assertThat(Joiner.on("\n").join(e.getStackTrace()), + CoreMatchers.containsString("testConnectToOneOfManyMasters")); } finally { if (c != null) { c.close();
