This is an automated email from the ASF dual-hosted git repository. wenchen pushed a commit to branch error in repository https://gitbox.apache.org/repos/asf/spark.git
commit dca962b202b81f5b31009f5d8a1096ed6ae2c3e6 Author: Wenchen Fan <[email protected]> AuthorDate: Tue Jan 27 19:50:47 2026 +0800 [SPARK-XXXXX][CORE] Refactor LazyTry stacktrace handling to use wrapper instead of suppressed exception This PR refactors how `doTryWithCallerStacktrace` and `getTryWithCallerStacktrace` handle stacktrace stitching. Instead of adding an `OriginalTryStackTraceException` as a suppressed exception on the original exception, we now wrap the original exception inside `OriginalTryStackTraceException`. Key changes: - `OriginalTryStackTraceException` now wraps the original exception (as its cause) and stores the pre-computed "below" stacktrace portion - `doTryWithCallerStacktrace` returns `Failure(wrapper)` instead of modifying the original exception's suppressed list - `getTryWithCallerStacktrace` unwraps the original exception, stitches the stacktrace, and throws the original (not the wrapper) Benefits: - Cleaner exception output - no suppressed exceptions visible to users - Simpler code - no need to track first access or manage suppressed exception lifecycle - Original exception type is preserved when thrown --- .../main/scala/org/apache/spark/util/LazyTry.scala | 6 -- .../main/scala/org/apache/spark/util/Utils.scala | 76 +++++++++------------- .../scala/org/apache/spark/util/UtilsSuite.scala | 39 +++-------- 3 files changed, 38 insertions(+), 83 deletions(-) diff --git a/core/src/main/scala/org/apache/spark/util/LazyTry.scala b/core/src/main/scala/org/apache/spark/util/LazyTry.scala index 7edc08672c26..910a18a7edc3 100644 --- a/core/src/main/scala/org/apache/spark/util/LazyTry.scala +++ b/core/src/main/scala/org/apache/spark/util/LazyTry.scala @@ -48,12 +48,6 @@ private[spark] class LazyTry[T](initialize: => T) extends Serializable { /** * Get the lazy value. If the initialization block threw an exception, it will be re-thrown here. * The exception will be re-thrown with the current caller's stacktrace. - * An exception with stack trace from when the exception was first thrown can be accessed with - * ``` - * ex.getSuppressed.find { e => - * e.getMessage == org.apache.spark.util.Utils.TRY_WITH_CALLER_STACKTRACE_FULL_STACKTRACE - * } - * ``` */ def get: T = Utils.getTryWithCallerStacktrace(tryT) } diff --git a/core/src/main/scala/org/apache/spark/util/Utils.scala b/core/src/main/scala/org/apache/spark/util/Utils.scala index 1f82f9af1a9b..c116566a5368 100644 --- a/core/src/main/scala/org/apache/spark/util/Utils.scala +++ b/core/src/main/scala/org/apache/spark/util/Utils.scala @@ -1365,13 +1365,17 @@ private[spark] object Utils } } - val TRY_WITH_CALLER_STACKTRACE_FULL_STACKTRACE = - "Full stacktrace of original doTryWithCallerStacktrace caller" - - class OriginalTryStackTraceException() - extends Exception(TRY_WITH_CALLER_STACKTRACE_FULL_STACKTRACE) { - var doTryWithCallerStacktraceDepth: Int = 0 - } + /** + * Wrapper exception used to carry the original exception along with the portion of the + * stacktrace to preserve during stitching. This is used internally by doTryWithCallerStacktrace + * and getTryWithCallerStacktrace, and is never exposed to users. + * + * @param cause The original exception + * @param belowStacktrace The stacktrace portion below doTryWithCallerStacktrace to preserve + */ + class OriginalTryStackTraceException( + cause: Throwable, + val belowStacktrace: Array[StackTraceElement]) extends Exception(cause) /** * Use Try with stacktrace substitution for the caller retrieving the error. @@ -1379,9 +1383,8 @@ private[spark] object Utils * Normally in case of failure, the exception would have the stacktrace of the caller that * originally called doTryWithCallerStacktrace. However, we want to replace the part above * this function with the stacktrace of the caller who calls getTryWithCallerStacktrace. - * So here we save the part of the stacktrace below doTryWithCallerStacktrace, and - * getTryWithCallerStacktrace will stitch it with the new stack trace of the caller. - * The full original stack trace is kept in ex.getSuppressed. + * So here we wrap the exception with metadata, and getTryWithCallerStacktrace will + * unwrap it and stitch the stacktrace with the new caller's stack trace. * * @param f Code block to be wrapped in Try * @return Try with Success or Failure of the code block. Use with getTryWithCallerStacktrace. @@ -1392,6 +1395,10 @@ private[spark] object Utils } t match { case Failure(ex) => + // If already wrapped, return as-is (nested call) + if (ex.isInstanceOf[OriginalTryStackTraceException]) { + return t + } // Note: we remove the common suffix instead of e.g. finding the call to this function, to // account for recursive calls with multiple doTryWithCallerStacktrace on the stack trace. val origStackTrace = ex.getStackTrace @@ -1399,22 +1406,12 @@ private[spark] object Utils val commonSuffixLen = origStackTrace.reverse.zip(currentStackTrace.reverse).takeWhile { case (exElem, currentElem) => exElem == currentElem }.length - // Add the full stack trace of the original caller as the suppressed exception. - // It may already be there if it's a nested call to doTryWithCallerStacktrace. - val origEx = ex.getSuppressed.find { e => - e.isInstanceOf[OriginalTryStackTraceException] - }.getOrElse { - val fullEx = new OriginalTryStackTraceException() - fullEx.setStackTrace(origStackTrace) - ex.addSuppressed(fullEx) - fullEx - }.asInstanceOf[OriginalTryStackTraceException] - // Update the depth of the stack of the current doTryWithCallerStacktrace, for stitching - // it with the stack of getTryWithCallerStacktrace. - origEx.doTryWithCallerStacktraceDepth = origStackTrace.size - commonSuffixLen - case Success(_) => // nothing + // Pre-compute the "below" portion to preserve during stitching + val belowStacktrace = origStackTrace.take(origStackTrace.size - commonSuffixLen) + val wrapper = new OriginalTryStackTraceException(ex, belowStacktrace) + Failure(wrapper) + case Success(_) => t } - t } /** @@ -1424,32 +1421,19 @@ private[spark] object Utils * below the original doTryWithCallerStacktrace which triggered it, with the caller stack trace * of the current caller of getTryWithCallerStacktrace. * - * Full stack trace of the original doTryWithCallerStacktrace caller can be retrieved with - * ``` - * ex.getSuppressed.find { e => - * e.isInstanceOf[Utils.OriginalTryStackTraceException] - * } - * ``` - * - * * @param t Try from doTryWithCallerStacktrace * @return Result of the Try or rethrows the failure exception with modified stacktrace. */ def getTryWithCallerStacktrace[T](t: Try[T]): T = t match { + case Failure(wrapper: OriginalTryStackTraceException) => + // Unwrap to get the original exception + val originalEx = wrapper.getCause + // Stitch the stacktrace: keep the "below" part, replace "above" with current caller + originalEx.setStackTrace( + wrapper.belowStacktrace ++ Thread.currentThread().getStackTrace.drop(1)) + throw originalEx case Failure(ex) => - val originalStacktraceEx = ex.getSuppressed.find { e => - // added in doTryWithCallerStacktrace - e.isInstanceOf[OriginalTryStackTraceException] - }.getOrElse { - // If we don't have the expected stacktrace information, just rethrow - throw ex - }.asInstanceOf[OriginalTryStackTraceException] - val belowStacktrace = originalStacktraceEx.getStackTrace - .take(originalStacktraceEx.doTryWithCallerStacktraceDepth) - // We are modifying and throwing the original exception. It would be better if we could - // return a copy, but we can't easily clone it and preserve. If this is accessed from - // multiple threads that then look at the stack trace, this could break. - ex.setStackTrace(belowStacktrace ++ Thread.currentThread().getStackTrace.drop(1)) + // Not wrapped (shouldn't happen if used correctly), just rethrow throw ex case Success(s) => s } diff --git a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala index 1cc16f206146..0dacbdda5e7d 100644 --- a/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala +++ b/core/src/test/scala/org/apache/spark/util/UtilsSuite.scala @@ -1669,26 +1669,8 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { assert(!st1.exists(_.getMethodName == "callDoTry")) assert(st1.exists(_.getMethodName == "callGetTry")) - // The original stack trace with callDoTry should be in the suppressed exceptions. - // Example: - // scalastyle:off line.size.limit - // Suppressed: java.lang.Exception: Full stacktrace of original doTryWithCallerStacktrace caller - // at org.apache.spark.util.UtilsSuite.throwException(UtilsSuite.scala:1640) - // at org.apache.spark.util.UtilsSuite.$anonfun$callDoTry$1(UtilsSuite.scala:1645) - // at scala.util.Try$.apply(Try.scala:213) - // at org.apache.spark.util.Utils$.doTryWithCallerStacktrace(Utils.scala:1586) - // at org.apache.spark.util.UtilsSuite.callDoTry(UtilsSuite.scala:1645) - // at org.apache.spark.util.UtilsSuite.$anonfun$new$165(UtilsSuite.scala:1658) - // ... 56 more - // scalastyle:on line.size.limit - val origSt = e1.getSuppressed.find(_.isInstanceOf[Utils.OriginalTryStackTraceException]) - assert(origSt.isDefined) - assert(origSt.get.getStackTrace.exists(_.getMethodName == "throwException")) - assert(origSt.get.getStackTrace.exists(_.getMethodName == "callDoTry")) - - // Should save the depth of the stack trace under doTryWithCallerStacktrace. - assert(origSt.get.asInstanceOf[Utils.OriginalTryStackTraceException] - .doTryWithCallerStacktraceDepth == 4) + // No suppressed exception should be added - the wrapper is used internally only + assert(!e1.getSuppressed.exists(_.isInstanceOf[Utils.OriginalTryStackTraceException])) val e2 = intercept[Exception] { callGetTryAgain(t) @@ -1760,20 +1742,13 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { // at org.apache.spark.util.UtilsSuite.callDoTryNestedNested(UtilsSuite.scala:1654) // at org.apache.spark.util.UtilsSuite.$anonfun$new$172(UtilsSuite.scala:1674) // ... - // Suppressed: org.apache.spark.util.Utils$OriginalTryStackTraceException: Full stacktrace of original doTryWithCallerStacktrace caller - // at org.apache.spark.util.UtilsSuite.throwException(UtilsSuite.scala:1529) - // at org.apache.spark.util.UtilsSuite.$anonfun$callDoTry$1(UtilsSuite.scala:1534) - // at scala.util.Try$.apply(Try.scala:217) - // at org.apache.spark.util.Utils$.doTryWithCallerStacktrace(Utils.scala:1377) - // at org.apache.spark.util.UtilsSuite.callDoTry(UtilsSuite.scala:1534) - // at org.apache.spark.util.UtilsSuite.$anonfun$callDoTryNested$1(UtilsSuite.scala:1631) - // ... // scalastyle:on line.size.limit assert(e.getStackTrace.exists(_.getMethodName == "callGetTryFromNested")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTryFromNestedNested")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTry")) - assert(e.getSuppressed.length == 1) + // No suppressed exception - the wrapper is used internally only + assert(e.getSuppressed.length == 0) Utils.getTryWithCallerStacktrace(t) } @@ -1821,7 +1796,8 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { assert(e.getStackTrace.exists(_.getMethodName == "callGetTryFromNestedNested")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTryFromNested")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTry")) - assert(e.getSuppressed.length == 1) + // No suppressed exception - the wrapper is used internally only + assert(e.getSuppressed.length == 0) Utils.getTryWithCallerStacktrace(t) } @@ -1865,7 +1841,8 @@ class UtilsSuite extends SparkFunSuite with ResetSystemProperties { assert(e.getStackTrace.exists(_.getMethodName == "callGetTry")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTryFromNested")) assert(!e.getStackTrace.exists(_.getMethodName == "callGetTryFromNestedNested")) - assert(e.getSuppressed.length == 1) + // No suppressed exception - the wrapper is used internally only + assert(e.getSuppressed.length == 0) } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
