distage-framework: Add test to ensure that TerminatingHandler doesn't…

… swallow exceptions
7mind · Nov 25, 2024 · b5c26c5 · b5c26c5
1 parent cc9c233
commit b5c26c5
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 13 deletions.
diff --git a/...ge/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala b/...ge/distage-core-api/src/main/scala/izumi/distage/model/provisioning/PlanInterpreter.scala
@@ -74,7 +74,7 @@ object PlanInterpreter {
       new ProvisioningException(
         s"""Interpreter stopped; out of $ccTotal operations: $ccFailed failed, $ccDone succeeded, $ccPending ignored
            |$repr
-           |""".stripMargin,
+           |""".stripMargin
       ).addAllSuppressed(allExceptions)
     }
   }
@@ -96,7 +96,12 @@ object PlanInterpreter {
         failure match {
           case ProvisioningFailure.AggregateFailure(_, failures, _) =>
             def stackTrace(exception: Throwable): String = {
-              if (fullStackTraces) exception.stacktraceString else exception.getMessage
+              exception match {
+                case nestedProvisioning: ProvisioningException =>
+                  nestedProvisioning.getMessage
+                case _ =>
+                  if (fullStackTraces) exception.stacktraceString else exception.getMessage
+              }
             }
 
             val messages = failures

diff --git a/...tage-extension-config/.jvm/src/main/scala/izumi/distage/config/codec/DIConfigReader.scala b/...tage-extension-config/.jvm/src/main/scala/izumi/distage/config/codec/DIConfigReader.scala
@@ -86,7 +86,7 @@ trait DIConfigReader[A] extends AbstractDIConfigReader[A] {
     Try(t).flatten match {
       case Failure(exception) =>
         throw new DIConfigReadException(
-          s"""Couldn't read configuration type at path="$path" as type `${Tag[T].tag}` due to error:
+          s"""Couldn't read configuration at path="$path" as type `${Tag[T].tag}` due to error:
              |
              |- ${exception.getMessage}
              |

diff --git a/...e/distage-framework/.jvm/src/test/scala/com/github/pshirshov/test3/plugins/Fixture3.scala b/...e/distage-framework/.jvm/src/test/scala/com/github/pshirshov/test3/plugins/Fixture3.scala
@@ -3,6 +3,8 @@ package com.github.pshirshov.test3.plugins
 import com.github.pshirshov.test3.bootstrap.BootstrapFixture3.{BasicConfig, BootstrapComponent, UnsatisfiedDep}
 import izumi.distage.plugins.{PluginConfig, PluginDef}
 import izumi.distage.roles.RoleAppMain
+import izumi.distage.roles.launcher.AppFailureHandler
+import izumi.distage.roles.launcher.AppFailureHandler.TerminatingHandler
 import izumi.distage.roles.model.definition.RoleModuleDef
 import izumi.distage.roles.model.{RoleDescriptor, RoleTask}
 import izumi.fundamentals.platform.cli.model.raw.RawEntrypointParams
@@ -13,6 +15,7 @@ object Fixture3 {
   object TestRoleAppMain extends RoleAppMain.LauncherIdentity {
     override protected def pluginConfig: PluginConfig = PluginConfig.cachedThisPkg
     override protected def bootstrapPluginConfig: PluginConfig = PluginConfig.cached("com.github.pshirshov.test3.bootstrap")
+    override protected def earlyFailureHandler(args: RoleAppMain.ArgV): AppFailureHandler = new TerminatingHandler(sysExit = _ => ())
   }
 
   object TestRoleAppMainFailing extends RoleAppMain.LauncherIdentity {

diff --git a/distage/distage-framework/.jvm/src/test/scala/izumi/distage/roles/test/RoleAppTest.scala b/distage/distage-framework/.jvm/src/test/scala/izumi/distage/roles/test/RoleAppTest.scala
@@ -10,6 +10,7 @@ import izumi.distage.framework.config.PlanningOptions
 import izumi.distage.framework.services.RoleAppPlanner
 import izumi.distage.model.PlannerInput
 import izumi.distage.model.definition.{Activation, BootstrapModule, Lifecycle}
+import izumi.distage.model.exceptions.runtime.ProvisioningException
 import izumi.distage.model.provisioning.IntegrationCheck
 import izumi.distage.modules.DefaultModule
 import izumi.distage.plugins.{PluginBase, PluginConfig}
@@ -23,7 +24,9 @@ import izumi.logstage.api.IzLogger
 import izumi.logstage.api.logger.LogSink
 import org.scalatest.wordspec.AnyWordSpec
 
-import java.io.File
+import java.io.{File, OutputStream, PrintStream}
+import java.nio.{BufferOverflowException, ByteBuffer}
+import java.nio.charset.StandardCharsets
 import java.nio.charset.StandardCharsets.UTF_8
 import java.nio.file.{Files, Paths}
 import java.util.UUID
@@ -570,6 +573,35 @@ class RoleAppTest extends AnyWordSpec with WithProperties {
       Fixture3.TestRoleAppMain.main(Array(":fixture3"))
     }
 
+    "TerminatingHandler reports error when exiting if an exception interrupts provisioning" in {
+      val oldErr = System.err
+      val errBuf = ByteBuffer.allocate(10 * 1024 * 1024)
+      val interceptErr = new PrintStream(
+        new OutputStream {
+          override def write(b: Int): Unit = {
+            oldErr.write(b)
+            try errBuf.put(b.toByte)
+            catch { case _: BufferOverflowException => () }
+            ()
+          }
+        },
+        true,
+      )
+      try {
+        System.setErr(interceptErr)
+        intercept[ProvisioningException] {
+          Fixture3.TestRoleAppMain.main(Array("--ignore-all-reference-configs", ":fixture3"))
+        }
+      } finally {
+        System.setErr(oldErr)
+      }
+      errBuf.flip()
+      val errString = new String(errBuf.array(), errBuf.arrayOffset(), errBuf.limit(), StandardCharsets.UTF_8)
+      assert(errString.contains("""Couldn't read configuration at path="basicConfig""""))
+      // error appears only once
+      assert(errString.indexOf("""Couldn't read configuration at path="basicConfig"""") == errString.lastIndexOf("""Couldn't read configuration at path="basicConfig""""))
+    }
+
     "LogIO2 binding is available in LauncherBIO for ZIO & MonixBIO" in {
       withProperties(
         DebugProperties.`izumi.distage.roles.activation.ignore-unknown`.name -> "true",

diff --git a/...age/distage-framework/src/main/scala/izumi/distage/roles/launcher/AppFailureHandler.scala b/...age/distage-framework/src/main/scala/izumi/distage/roles/launcher/AppFailureHandler.scala
@@ -9,13 +9,14 @@ trait AppFailureHandler {
 
 object AppFailureHandler {
 
-  object TerminatingHandler extends AppFailureHandler {
+  open class TerminatingHandler(sysExit: Int => Unit) extends AppFailureHandler {
     override def onError(t: Throwable): Nothing = {
       report(t)
-      System.exit(1)
+      sysExit(1)
       rethrow(t)
     }
   }
+  object TerminatingHandler extends TerminatingHandler(sysExit = System.exit)
 
   object NullHandler extends AppFailureHandler {
     override def onError(t: Throwable): Nothing = {
@@ -25,26 +26,30 @@ object AppFailureHandler {
 
   private def rethrow(t: Throwable): Nothing = {
     t match {
-      case d: ProvisioningException =>
-        // here we remove suppressed exceptions to make output more readable
-        throw new ProvisioningException(d.getMessage, captureStackTrace = false)
+      case p: ProvisioningException =>
+        throw formatProvisioningException(p)
       case o =>
         throw o
     }
   }
 
   private def report(t: Throwable): Unit = {
     t match {
-      case d: ProvisioningException =>
-        d.getSuppressed.toList match {
-          case (d: DIAppBootstrapException) :: Nil=>
+      case p: ProvisioningException =>
+        p.getSuppressed.toList match {
+          case (d: DIAppBootstrapException) :: Nil =>
             System.err.println(d.getMessage)
           case _ =>
-            d.printStackTrace()
+            formatProvisioningException(p).printStackTrace()
         }
       case _ =>
         t.printStackTrace()
     }
   }
 
+  private def formatProvisioningException(p: ProvisioningException): ProvisioningException = {
+    // here we remove suppressed exceptions to make output more readable (ProvisioningException already includes other exceptions' stacktraces in getMessage)
+    new ProvisioningException(p.getMessage, captureStackTrace = false)
+  }
+
 }