[
https://issues.apache.org/jira/browse/DRILL-2626?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14692758#comment-14692758
]
Chris Westin commented on DRILL-2626:
-------------------------------------
Answer to the subject question is "No." I'm going to fix the formatting, as per
DRILL-2625. However, even with that, the functionality isn't quite exactly the
same as Throwable() (which is regrettable on the JDK's part). Before I created
this, I did actually look at using Throwable() directly, as it already appeared
in some of the code. StackTrace supports the use of an indentation parameter
when formatting the stack trace, which makes it a lot easier to read in the
midst of a larger trace/debug message. Also, the JDK form doesn't provide a
good way to incorporate such a stack trace into a larger message. toString()
doesn't allow the inclusion of indentation. And the other forms, (various
overloads of printStackTrace()), take PrintStreams and PrintWriters, which
aren't convenient for use with Logger; I made one attempt, but it meant
wrapping Logger with an implementation of PrintWriter, and that caused the
stack trace to appear as a separate message from the "real one" that was being
logged -- in a multi-threaded context, it wasn't always adjacent to the message
that it needed to go with. It was these shortcomings that led me to create this
class. StackTrace.writeToBuilder() takes a StringBuilder and an indentation so
that the trace can be incorporated into a single message that can be given to
Logger.
> org.apache.drill.common.StackTrace seems to have duplicate code; should we
> re-use Throwable's code?
> ---------------------------------------------------------------------------------------------------
>
> Key: DRILL-2626
> URL: https://issues.apache.org/jira/browse/DRILL-2626
> Project: Apache Drill
> Issue Type: Bug
> Components: Execution - Flow
> Affects Versions: 0.8.0
> Reporter: Daniel Barclay (Drill)
> Assignee: Chris Westin
> Fix For: 1.3.0
>
>
> 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.)
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
