Wrap component constructor exceptions and handle separately.

2 files changed, 26 insertions, 17 deletions

diff --git a/container-di/src/main/scala/com/yahoo/container/di/Container.scala b/container-di/src/main/scala/com/yahoo/container/di/Container.scala
index cd5339cc313..bc3f9fbe79e 100644
--- a/container-di/src/main/scala/com/yahoo/container/di/Container.scala
+++ b/container-di/src/main/scala/com/yahoo/container/di/Container.scala
@@ -1,8 +1,8 @@
 // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.di
 
-import com.yahoo.container.di.ConfigRetriever.{ComponentsConfigs, BootstrapConfigs}
-import com.yahoo.container.di.componentgraph.core.{JerseyNode, ComponentGraph, ComponentNode}
+import com.yahoo.container.di.ConfigRetriever.{BootstrapConfigs, ComponentsConfigs}
+import com.yahoo.container.di.componentgraph.core.{ComponentGraph, ComponentNode, JerseyNode}
 import com.yahoo.container.di.config.{RestApiContext, SubscriberFactory}
 import Container._
 
@@ -11,10 +11,13 @@ import scala.math.max
 import com.yahoo.config._
 import com.yahoo.vespa.config.ConfigKey
 import java.util.IdentityHashMap
-import java.util.logging.Logger
+import java.util.logging.{Level, Logger}
+
 import com.yahoo.container.bundle.BundleInstantiationSpecification
-import com.google.inject.{Injector, Guice}
+import com.google.inject.{Guice, Injector}
+import com.yahoo.container.di.componentgraph.core.ComponentNode.ComponentConstructorException
 import com.yahoo.container.{BundlesConfig, ComponentsConfig}
+import com.yahoo.log.LogLevel
 
 
 /**
@@ -49,20 +52,23 @@ class Container(
     }
 
     try {
-      //TODO: wrap user exceptions.
       val newGraph = createNewGraph(oldGraph, fallbackInjector)
       newGraph.reuseNodes(oldGraph)
       constructComponents(newGraph)
       deconstructObsoleteComponents(oldGraph, newGraph)
       newGraph
     } catch {
-      case e : Throwable =>
-        invalidateGeneration()
-        throw e
+      case userException : ComponentConstructorException =>
+        invalidateGeneration("Failed to set up new component graph due to error when constructing one of the components.", userException)
+        throw userException
+      case t : Throwable =>
+        invalidateGeneration("Failed to set up new component graph.", t)
+        throw t
     }
   }
 
-  private def invalidateGeneration() {
+  private def invalidateGeneration(message: String, cause: Throwable) {
+    log.log(Level.WARNING, message, cause)
     leastGeneration = max(configurer.getComponentsGeneration, configurer.getBootstrapGeneration) + 1
   }
 
diff --git a/container-di/src/main/scala/com/yahoo/container/di/componentgraph/core/ComponentNode.scala b/container-di/src/main/scala/com/yahoo/container/di/componentgraph/core/ComponentNode.scala
index 16856930e54..e689b7b7b3d 100644
--- a/container-di/src/main/scala/com/yahoo/container/di/componentgraph/core/ComponentNode.scala
+++ b/container-di/src/main/scala/com/yahoo/container/di/componentgraph/core/ComponentNode.scala
@@ -99,19 +99,19 @@ class ComponentNode(componentId: ComponentId,
       constructor.newInstance(actualArguments: _*)
     } catch {
       case e: InvocationTargetException =>
-        throw removeStackTrace(constructThrowable(cutStackTraceAtConstructor(e.getCause), s"Error constructing $idAndType"))
+        throw removeStackTrace(ErrorOrComponentConstructorException(cutStackTraceAtConstructor(e.getCause), s"Error constructing $idAndType"))
     }
 
     initId(instance)
   }
   
-  private def constructThrowable(cause: Throwable, message: String) : Throwable = {
+  private def ErrorOrComponentConstructorException(cause: Throwable, message: String) : Throwable = {
     if (cause != null && cause.isInstanceOf[Error]) // don't convert Errors to RuntimeExceptions
       new Error(message, cause)
     else
-      new RuntimeException(message, cause)
+      new ComponentConstructorException(message, cause)
   }
-  
+
   private def initId(component: AnyRef) = {
     def checkAndSetId(c: AbstractComponent) {
       if (c.hasInitializedId && c.getId != componentId )
@@ -180,7 +180,7 @@ object ComponentNode {
       publicConstructors filter {_.getAnnotation(classOf[Inject]) != null} match {
         case Array() => None
         case Array(single) => Some(single)
-        case _ => throwRuntimeExceptionRemoveStackTrace("Multiple constructors annotated with inject in class " + clazz.getName)
+        case _ => throwComponentConstructorException("Multiple constructors annotated with inject in class " + clazz.getName)
       }
     }
 
@@ -188,7 +188,7 @@ object ComponentNode {
       def isConfigInstance(clazz: Class[_]) = classOf[ConfigInstance].isAssignableFrom(clazz)
 
       publicConstructors match {
-        case Array() => throwRuntimeExceptionRemoveStackTrace("No public constructors in class " + clazz.getName)
+        case Array() => throwComponentConstructorException("No public constructors in class " + clazz.getName)
         case Array(single) => single
         case _ =>
           log.warning("Multiple public constructors found in class %s, there should only be one. ".format(clazz.getName) +
@@ -202,9 +202,12 @@ object ComponentNode {
     constructorAnnotatedWithInject getOrElse constructorWithMostConfigParameters
   }
 
-  private def throwRuntimeExceptionRemoveStackTrace(message: String) =
-    throw removeStackTrace(new RuntimeException(message))
+  private def throwComponentConstructorException(message: String) =
+    throw removeStackTrace(new ComponentConstructorException(message))
 
+  class ComponentConstructorException(message: String, cause: Throwable) extends RuntimeException(message, cause) {
+    def this(message: String) = this(message, null)
+  }
 
   def isAbstract(clazz: Class[_ <: AnyRef]) = Modifier.isAbstract(clazz.getModifiers)
 }