util-core: Return suppressed exception information in Closable.sequence

Problem

Closable.sequence returns the first failed Future[Unit] result of closing the Closables, and in the case of multiple failures, the subsequent exceptions are lost.

Solution

Add the subsequent failures to the list of suppressed exceptions attached to the first failed Future[Unit] result.

Result

Users will be able to see all failures in the case that there are multiple failures in Closable.sequence.

JIRA Issues: CSL-11522

Differential Revision: https://phabricator.twitter.biz/D809561

CHANGELOG.rst

@@ -20,6 +20,8 @@ Runtime Behavior Changes
 
 * util: Bump version of Jackson to 2.13.1. ``PHAB_ID=D808049``
 
+* util-core: Return suppressed exception information in Closable.sequence ``PHAB_ID=D809561``
+
 21.12.0
 -------
 

util-core/src/main/scala/com/twitter/util/Closable.scala

@@ -110,7 +110,8 @@ object Closable {
    * resource ''n+1'' is not closed until resource ''n'' is.
    *
    * @return the first failed [[Future]] should any of the `Closables`
-   *         result in a failed [[Future]].
+   *         result in a failed [[Future]]. Any subsequent failures are
+   *         included as suppressed exceptions.
    *
    * @note as with all `Closables`, the `deadline` passed to `close`
    *       is advisory.
@@ -121,8 +122,17 @@ object Closable {
 
       def onFirstComplete(t: Try[Unit]): Future[Unit] = {
         val second = safeClose(b, deadline)
-        if (t.isThrow) if (second.isDefined) first else second.transform(_ => first)
-        else second
+        if (t.isThrow) {
+          if (second eq Future.Done) first
+          else {
+            second.transform {
+              case Return(_) => first
+              case Throw(secondThrow) =>
+                t.throwable.addSuppressed(secondThrow)
+                first
+            }
+          }
+        } else second
       }
 
       if (first eq Future.Done) onFirstComplete(Try.Unit)
@@ -136,7 +146,8 @@ object Closable {
    * resource ''n+1'' is not closed until resource ''n'' is.
    *
    * @return the first failed [[Future]] should any of the `Closables`
-   *         result in a failed [[Future]].
+   *         result in a failed [[Future]]. Any subsequent failures are
+   *         included as suppressed exceptions.
    *
    * @note as with all `Closables`, the `deadline` passed to `close`
    *       is advisory.
@@ -146,11 +157,11 @@ object Closable {
     private final def closeSeq(
       deadline: Time,
       closables: Seq[Closable],
-      firstFailure: Option[Future[Unit]]
+      firstFailure: Option[Throw[Unit]]
     ): Future[Unit] = closables match {
       case Seq() =>
         firstFailure match {
-          case Some(f) => f
+          case Some(t) => Future.const(t)
           case None => Future.Done
         }
       case Seq(hd, tl @ _*) =>
@@ -159,8 +170,10 @@ object Closable {
             case Return(_) => firstFailure
             case t @ Throw(_) =>
               firstFailure match {
-                case Some(_) => firstFailure
-                case None => Some(Future.const(t))
+                case Some(firstThrow) =>
+                  firstThrow.throwable.addSuppressed(t.throwable)
+                  firstFailure
+                case None => Some(t)
               }
           }
           closeSeq(deadline, tl, failure)

util-core/src/test/scala/com/twitter/util/ClosableTest.scala

@@ -188,13 +188,14 @@ abstract class ClosableSequenceTest extends AnyFunSuite with Eventually with Int
     }
     assert(x2.getMessage.contains("n1=2"))
 
-    // multiple failures, returns the first failure
+    // multiple failures, returns the first failure, and a list of suppressed failures.
     val f3 = sequenceTwo(c1, c1).close()
     assert(n1 == 4)
     val x3 = intercept[RuntimeException] {
       Await.result(f3, 5.seconds)
     }
     assert(x3.getMessage.contains("n1=3"))
+    assert(x3.getSuppressed.head.getMessage.contains("n1=4"))
 
     // verify that first failure is returned, but only after the
     // last closable is satisfied