From ac92b6a7a5347495b63a1b76f244541b2858b6a8 Mon Sep 17 00:00:00 2001 From: Matt Jacobs Date: Tue, 6 Jan 2015 18:26:16 -0800 Subject: [PATCH] Removed ExceptionThreadingUtility. Stack traces are not helpful in an event-loop world. --- .../exception/HystrixRuntimeException.java | 3 - .../util/ExceptionThreadingUtility.java | 120 ------------------ .../util/ExceptionThreadingUtilityTest.java | 52 -------- 3 files changed, 175 deletions(-) delete mode 100644 hystrix-core/src/main/java/com/netflix/hystrix/util/ExceptionThreadingUtility.java delete mode 100644 hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionThreadingUtilityTest.java diff --git a/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java b/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java index b70a877f2..17feccbdb 100644 --- a/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java +++ b/hystrix-core/src/main/java/com/netflix/hystrix/exception/HystrixRuntimeException.java @@ -17,7 +17,6 @@ import com.netflix.hystrix.HystrixCommand; import com.netflix.hystrix.HystrixInvokable; -import com.netflix.hystrix.util.ExceptionThreadingUtility; /** * RuntimeException that is thrown when a {@link HystrixCommand} fails and does not have a fallback. @@ -40,7 +39,6 @@ public HystrixRuntimeException(FailureType failureCause, Class commandClass, String message, Throwable cause, Throwable fallbackException) { @@ -48,7 +46,6 @@ public HystrixRuntimeException(FailureType failureCause, Class - * This is used to make the exceptions thrown from an isolation thread useful, otherwise they see a stacktrace on a thread but never see who was calling it. - */ -public class ExceptionThreadingUtility { - - private final static String messageForCause = "Calling Thread included as the last 'caused by' on the chain."; - - /* package */ static void attachCallingThreadStack(Throwable e, StackTraceElement[] stack) { - Set seenCauses = new HashSet(); - - Throwable callingThrowable = new Throwable(messageForCause); - if (stack[0].toString().startsWith("java.lang.Thread.getStackTrace")) { - // get rid of the first item on the stack that says "java.lang.Thread.getStackTrace" - StackTraceElement newStack[] = Arrays.copyOfRange(stack, 1, stack.length); - stack = newStack; - } - callingThrowable.setStackTrace(stack); - - while (e.getCause() != null) { - e = e.getCause(); - if (seenCauses.contains(e)) { - break; - } else { - seenCauses.add(e); - } - } - // check that we're not recursively wrapping an exception that already had the cause set, and if not then add our artificial 'cause' - if (!messageForCause.equals(e.getMessage())) { - // we now have 'e' as the last in the chain - try { - e.initCause(callingThrowable); - } catch (Throwable t) { - // ignore - // the javadocs say that some Throwables (depending on how they're made) will never - // let me call initCause without blowing up even if it returns null - } - } - } - - @SuppressWarnings("unused") - private static String getStackTraceAsString(StackTraceElement[] stack) { - StringBuilder s = new StringBuilder(); - boolean firstLine = true; - for (StackTraceElement e : stack) { - if (e.toString().startsWith("java.lang.Thread.getStackTrace")) { - // we'll ignore this one - continue; - } - if (!firstLine) { - s.append("\n\t"); - } - s.append(e.toString()); - firstLine = false; - } - return s.toString(); - } - - public static void attachCallingThreadStack(Throwable e) { - try { - if (callingThreadCache.get() != null) { - /** - * benjchristensen -> I don't like this code, but it's been working in API prod for several months (as of Jan 2012) to satisfy debugability requirements. - * - * It works well when someone does Command.execute() since the calling threads blocks so its stacktrace shows exactly what is calling the command. - * - * It won't necessarily work very well with Command.queue() since the calling thread will be off doing something else when this occurs. - * - * I'm also somewhat concerned about the thread-safety of this. I have no idea what the guarantees are, but we haven't seen issues such as deadlocks, infinite loops, etc and no known - * data corruption. - * - * I'm not a fan of this solution, but it is better so far than not having it. - * - * The cleaner alternative would be to capture the stackTrace when the command is queued, but I'm concerned with the performance implications of capturing the stackTrace on every - * single invocation when we only need it in error events which are rare. I have not performed any testing to determine the cost, but it certainly has a cost based on looking at the - * java source code for getStackTrace(). - */ - attachCallingThreadStack(e, callingThreadCache.get().getStackTrace()); - } - } catch (Throwable ex) { - // ignore ... we don't want to blow up while trying to augment the stacktrace on a real exception - } - } - - /** - * Allow a parent thread to pass its reference in on a child thread and be remembered so that stacktraces can include the context of who called it. - *

- * ThreadVariable is a Netflix version of ThreadLocal that takes care of cleanup as part of the request/response loop. - */ - private static ThreadLocal callingThreadCache = new ThreadLocal(); - - // TODO this doesn't get cleaned up after moving away from the Netflix ThreadVariable - - public static void assignCallingThread(Thread callingThread) { - callingThreadCache.set(callingThread); - } - -} diff --git a/hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionThreadingUtilityTest.java b/hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionThreadingUtilityTest.java deleted file mode 100644 index a59bfc7a9..000000000 --- a/hystrix-core/src/test/java/com/netflix/hystrix/util/ExceptionThreadingUtilityTest.java +++ /dev/null @@ -1,52 +0,0 @@ -package com.netflix.hystrix.util; - -import static org.junit.Assert.assertEquals; - -import org.junit.Test; - -public class ExceptionThreadingUtilityTest { - private final Throwable ex1 = new Throwable("Ex1"); - private final Throwable ex2 = new Throwable("Ex2", ex1); - - public ExceptionThreadingUtilityTest() { - ex1.initCause(ex2); - } - - @Test - public void testAttachCallingThreadStackParentThenChild() { - ExceptionThreadingUtility.attachCallingThreadStack(ex1, ex2.getStackTrace()); - assertEquals("Ex1", ex1.getMessage()); - assertEquals("Ex2", ex1.getCause().getMessage()); - assertEquals("Ex2", ex2.getMessage()); - assertEquals("Ex1", ex2.getCause().getMessage()); - } - - @Test - public void testAttachCallingThreadStackChildThenParent() { - ExceptionThreadingUtility.attachCallingThreadStack(ex2, ex1.getStackTrace()); - assertEquals("Ex1", ex1.getMessage()); - assertEquals("Ex2", ex1.getCause().getMessage()); - assertEquals("Ex2", ex2.getMessage()); - assertEquals("Ex1", ex2.getCause().getMessage()); - } - - @Test - public void testAttachCallingThreadStackAddExceptionsToEachOther() { - ExceptionThreadingUtility.attachCallingThreadStack(ex1, ex2.getStackTrace()); - ExceptionThreadingUtility.attachCallingThreadStack(ex2, ex1.getStackTrace()); - assertEquals("Ex1", ex1.getMessage()); - assertEquals("Ex2", ex2.getMessage()); - assertEquals("Ex2", ex1.getCause().getMessage()); - assertEquals("Ex1", ex2.getCause().getMessage()); - } - - @Test - public void testAttachCallingThreadStackAddExceptionToItself() { - ExceptionThreadingUtility.attachCallingThreadStack(ex2, ex2.getStackTrace()); - assertEquals("Ex1", ex1.getMessage()); - assertEquals("Ex2", ex1.getCause().getMessage()); - assertEquals("Ex2", ex2.getMessage()); - assertEquals("Ex1", ex2.getCause().getMessage()); - } - -}