Uploaded image for project: 'Apache Drill'
  1. Apache Drill
  2. DRILL-2626

org.apache.drill.common.StackTrace seems to have duplicate code; should we re-use Throwable's code?



    • Bug
    • Status: Closed
    • Major
    • Resolution: Invalid
    • 0.8.0
    • 1.3.0
    • Execution - Flow
    • None


      It seems that class org.apache.drill.common.StackTrace needlessly duplicates code that's already in the JDK.

      In particular, it has code to format the stack trace. That seems at least mostly redundant with the formatting code already in java.lang.Throwable.

      StackTrace does have a comment about eliminating the StackTrace constructor from the stack trace. However, StackTrace does not actuallly eliminate its contructor from the stack trace (e.g., its stack traces start with "org.apache.drill.common.StackTrace.<init>:...").

      Should StackTrace be implemented by simply subclassing Throwable?

      That would eliminate StackTrace's current formatting code (which would also eliminate the difference between StackTrace's format and the standard format).

      That should also eliminate having the StackTrace constructor's stack frame show up in the stack trace. (Throwable's constructor/fillInStackTrace already handles that.)

      (Having "StackTrace extends Throwable" isn't ideal, since StackTrace is not intended to be a kind of exception, but that would probably be better than the current form, given the bugs StackTrace has/has had (DRILL-2624, DRILL-2625).

      That non-ideal subclassing could be eliminated by having a member variable of type Throwable that is constructed during StackTrace's construction, although that would either cause the StackTrace constructor to re-appear in the stack trace or require a non-trivial workaround to re-eliminate it.

      Perhaps client code should simply use "new Throwable()" to capture the stack trace and a static methods on a utility class to format the stack trace into a String.)


        Issue Links



              cwestin Chris Westin
              dsbos Daniel Barclay
              0 Vote for this issue
              2 Start watching this issue