Merge pull request #338 from dotta/issue/handling-debugger-runtime-ex…

…ceptions-1001531

Comply to the debugger interfaces by wrapping JDI runtime exceptions

org.scala-ide.sdt.debug.tests/src/scala/tools/eclipse/debug/ScalaDebugComputeDetailTest.scala

@@ -8,7 +8,6 @@ import org.junit.After
 import org.junit.Test
 import org.junit.Assert._
 import scala.tools.eclipse.debug.model.ScalaDebugModelPresentation
-import scala.tools.eclipse.debug.model.ScalaLogicalStructureProvider
 import scala.tools.eclipse.debug.model.ScalaCollectionLogicalStructureType
 import scala.tools.eclipse.debug.model.ScalaArrayReference
 import org.junit.internal.matchers.StringContains
@@ -116,7 +115,7 @@ class ScalaDebugComputeDetailTest {
 
     assertThat("Wrong type for the logical structure", logicalStructure.getValueString(), StringContains.containsString("Array[Object](3)"))
 
-    val elements= logicalStructure.asInstanceOf[ScalaArrayReference].getVariables
+    val elements = logicalStructure.asInstanceOf[ScalaArrayReference].getVariables()
     assertThat("Wrong value for first element", elements(0).getValue().getValueString(), StringContains.containsString("Integer 4"))
     assertThat("Wrong value for second element", elements(1).getValue().getValueString(), StringContains.containsString("Integer 5"))
     assertThat("Wrong value for third element", elements(2).getValue().getValueString(), StringContains.containsString("Integer 6"))

org.scala-ide.sdt.debug.tests/src/scala/tools/eclipse/debug/ScalaDebugTestSession.scala

@@ -294,7 +294,7 @@ class ScalaDebugTestSession(launchConfiguration: ILaunchConfiguration) extends H
   def getLocalVariable(name: String): ScalaValue = {
     assertEquals("Bad state before getLocalVariable", SUSPENDED, state)
 
-    currentStackFrame.variables.find(_.getName == name).get.getValue.asInstanceOf[ScalaValue]
+    currentStackFrame.getVariables.find(_.getName == name).get.getValue.asInstanceOf[ScalaValue]
   }
 
 }

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/breakpoints/BreakpointSupport.scala

@@ -33,17 +33,21 @@ private[debug] object BreakpointSupport {
    */
   final val ATTR_VM_REQUESTS_ENABLED = "org.scala-ide.sdt.debug.breakpoint.vm_enabled"
 
-  /** Create the breakpoint support actor. */
+  /** Create the breakpoint support actor.
+   *  
+   *  @note `BreakpointSupportActor` instances are created only by the `ScalaDebugBreakpointManagerActor`, hence 
+   *        any uncaught exception that may occur during initialization (i.e., in `BreakpointSupportActor.apply`) 
+   *        will be caught by the `ScalaDebugBreakpointManagerActor` default exceptions' handler.
+   */
   def apply(breakpoint: IBreakpoint, debugTarget: ScalaDebugTarget): Actor = {
     BreakpointSupportActor(breakpoint, debugTarget)
   }
 }
 
-private[debug] object BreakpointSupportActor {
+private object BreakpointSupportActor {
   // specific events
   case class Changed(delta: IMarkerDelta)
 
-
   def apply(breakpoint: IBreakpoint, debugTarget: ScalaDebugTarget): Actor = {
     val typeName= breakpoint.typeName
 

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/model/ScalaDebugCache.scala

@@ -35,7 +35,7 @@ object ScalaDebugCache {
     val debugCache = new ScalaDebugCache(debugTarget) {
       val actor = new ScalaDebugCacheActor(this, debugTarget, scalaDebugTargetActor)
     }
-    debugCache.actor.start
+    debugCache.actor.start()
     debugCache
   }
 
@@ -148,9 +148,11 @@ abstract class ScalaDebugCache(val debugTarget: ScalaDebugTarget) extends HasLog
   /** Returns the anon function for the given type, if it exists.
    */
   private def findAnonFunction(refType: ReferenceType): Option[Method] = {
-    // TODO: check super type at some point
+    val allMethods = refType.methods
+
     import scala.collection.JavaConverters._
-    val methods = refType.methods.asScala.filter(method => !method.isBridge && method.name.startsWith("apply"))
+    // TODO: check super type at some point
+    val methods = allMethods.asScala.filter(method => !method.isBridge && method.name.startsWith("apply"))
 
     methods.size match {
       case 1 =>

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/model/ScalaDebugElement.scala

@@ -4,14 +4,19 @@ import scala.tools.eclipse.debug.ScalaDebugPlugin
 import scala.tools.eclipse.debug.ScalaDebugger
 import scala.tools.eclipse.debug.ScalaDebugger.modelProvider
 import scala.tools.eclipse.logging.HasLogger
-
 import org.eclipse.debug.core.model.{ITerminate, DebugElement}
-
 import com.sun.jdi.ClassType
 import com.sun.jdi.Field
 import com.sun.jdi.Method
 import com.sun.jdi.ReferenceType
 import com.sun.jdi.Value
+import org.eclipse.debug.core.DebugException
+import org.eclipse.jdi.TimeoutException
+import com.sun.jdi.VMDisconnectedException
+import org.eclipse.core.runtime.Status
+import org.eclipse.core.runtime.IStatus
+import scala.util.control.Exception
+import scala.util.control.Exception.Catch
 
 /**
  * Base class for debug elements in the Scala debug model
@@ -44,23 +49,47 @@ abstract class ScalaDebugElement(debugTarget: ScalaDebugTarget) extends DebugEle
 
   // ----
 
+  def wrapJDIException[T](msg: String): Catch[T] = 
+    Exception.handling(classOf[RuntimeException]) by (targetRequestFailed(msg, _))
+
+  /**
+    * Throws a new debug exception with a status code of `TARGET_REQUEST_FAILED`
+    * with the given underlying exception. If the underlying exception is not a JDI
+    * exception, the original exception is thrown.
+    * 
+    * @param message Failure message
+    * @param e underlying exception that has occurred
+    * @throws DebugException The exception with a status code of `TARGET_REQUEST_FAILED`
+    */
+  private def targetRequestFailed(message: String, t: Throwable): Nothing = {
+    if (t == null || t.getClass().getName().startsWith("com.sun.jdi") || t.isInstanceOf[TimeoutException])
+      throw new DebugException(new Status(IStatus.ERROR, ScalaDebugPlugin.id, DebugException.TARGET_REQUEST_FAILED, message, t))
+    else
+      throw t
+  }
 }
 
 trait HasFieldValue {
   self: ScalaDebugElement =>
 
-  protected[model] def referenceType(): ReferenceType
+  final protected[model] def referenceType(): ReferenceType =
+    wrapJDIException("Exception while retrieving reference type") { getReferenceType() }
 
-  /** Return the JDI value for the given field.
-   */
-  protected[model] def jdiFieldValue(field: Field): Value
+  /** Return the JDI value for the given field. */
+  final protected[model] def jdiFieldValue(field: Field): Value =
+    wrapJDIException("Exception while retrieving JDI field value") { getJdiFieldValue(field) }
+
+  protected def getReferenceType(): ReferenceType
+  protected def getJdiFieldValue(field: Field): Value
 
   /** Return the value of the field with the given name.
    *
    *  @throws IllegalArgumentException if the no field with the given name exists.
+   *  @throws DebugException
    */
-  def fieldValue(fieldName: String): ScalaValue = {
+  def fieldValue(fieldName: String): ScalaValue = wrapJDIException("Exception while retrieving field " + fieldName + " in reference type") {
     val field = referenceType().fieldByName(fieldName)
+
     if (field == null) {
       throw new IllegalArgumentException("Field '%s' doesn't exist for '%s'".format(fieldName, referenceType().name()))
     }
@@ -80,28 +109,36 @@ trait HasMethodInvocation {
   /** Invoke the method with given name, using the given arguments.
    *
    *  @throws IllegalArgumentException if no method with given name exists, or more than one.
+   *  @throws DebugException
    */
   def invokeMethod(methodName: String, thread: ScalaThread, args: ScalaValue*): ScalaValue = {
-    val methods = classType().methodsByName(methodName)
-    methods.size match {
-      case 0 =>
-        throw new IllegalArgumentException("Method '%s(..)' doesn't exist for '%s'".format(methodName, classType.name()))
-      case 1 =>
-        ScalaValue(jdiInvokeMethod(methods.get(0), thread, args.map(_.underlying): _*), getDebugTarget)
-      case _ =>
-        throw new IllegalArgumentException("More than on method '%s(..)' for '%s'".format(methodName, classType.name()))
+    wrapJDIException("Exception while retrieving method " + methodName + " in reference type") {
+      val methods = classType().methodsByName(methodName)
+
+      methods.size match {
+        case 0 =>
+          throw new IllegalArgumentException("Method '%s(..)' doesn't exist for '%s'".format(methodName, classType.name()))
+        case 1 =>
+          ScalaValue(jdiInvokeMethod(methods.get(0), thread, args.map(_.underlying): _*), getDebugTarget)
+        case _ =>
+          throw new IllegalArgumentException("More than on method '%s(..)' for '%s'".format(methodName, classType.name()))
+      }
     }
   }
 
   /** Invoke the method with given name and signature, using the given arguments.
    *
    *  @throws IllegalArgumentException if no method with given name and signature exists.
+   *  @throws DebugException
    */
   def invokeMethod(methodName: String, methodSignature: String, thread: ScalaThread, args: ScalaValue*): ScalaValue = {
-    val method = classType().concreteMethodByName(methodName, methodSignature)
-    if (method == null) {
-      throw new IllegalArgumentException("Method '%s%s' doesn't exist for '%s'".format(methodName, methodSignature, classType().name()))
+    wrapJDIException("Exception while retrieving method " + methodName + " in reference type") {
+      val method = classType().concreteMethodByName(methodName, methodSignature)
+
+      if (method == null) {
+        throw new IllegalArgumentException("Method '%s%s' doesn't exist for '%s'".format(methodName, methodSignature, classType().name()))
+      }
+      ScalaValue(jdiInvokeMethod(method, thread, args.map(_.underlying): _*), getDebugTarget)
     }
-    ScalaValue(jdiInvokeMethod(method, thread, args.map(_.underlying): _*), getDebugTarget)
   }
 }

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/model/ScalaDebugModelPresentation.scala

@@ -1,7 +1,6 @@
 package scala.tools.eclipse.debug.model
 
 import scala.tools.eclipse.debug.ScalaDebugger
-
 import org.eclipse.core.runtime.IProgressMonitor
 import org.eclipse.core.runtime.IStatus
 import org.eclipse.core.runtime.Status
@@ -11,6 +10,7 @@ import org.eclipse.debug.internal.ui.views.variables.IndexedVariablePartition
 import org.eclipse.debug.ui.{ IValueDetailListener, IDebugUIConstants, IDebugModelPresentation, DebugUITools }
 import org.eclipse.jdt.internal.ui.javaeditor.EditorUtility
 import org.eclipse.ui.IEditorInput
+import org.eclipse.jface.viewers.ILabelProviderListener
 
 /**
  * Utility methods for the ScalaDebugModelPresentation class
@@ -74,10 +74,10 @@ class ScalaDebugModelPresentation extends IDebugModelPresentation {
 
   // Members declared in org.eclipse.jface.viewers.IBaseLabelProvider
 
-  override def addListener(x$1: org.eclipse.jface.viewers.ILabelProviderListener): Unit = ???
+  override def addListener(listener: ILabelProviderListener): Unit = ???
   override def dispose(): Unit = {} // TODO: need real logic
-  override def isLabelProperty(x$1: Any, x$2: String): Boolean = ???
-  override def removeListener(x$1: org.eclipse.jface.viewers.ILabelProviderListener): Unit = ???
+  override def isLabelProperty(element: Any, property: String): Boolean = ???
+  override def removeListener(listener: ILabelProviderListener): Unit = ???
 
   // Members declared in org.eclipse.debug.ui.IDebugModelPresentation
 
@@ -88,7 +88,7 @@ class ScalaDebugModelPresentation extends IDebugModelPresentation {
         listener.detailComputed(value, ScalaDebugModelPresentation.computeDetail(value))
         Status.OK_STATUS
       }
-    }.schedule
+    }.schedule()
   }
 
   override def getImage(element: Any): org.eclipse.swt.graphics.Image = {

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/model/ScalaDebugTarget.scala

@@ -232,9 +232,13 @@ abstract class ScalaDebugTarget private (val virtualMachine: VirtualMachine, lau
    * 
    * @throws ClassNotLoadedException if the class was not loaded yet.
    * @throws IllegalArgumentException if there is no object of the given name.
+   * @throws DebugException
    */
   def objectByName(objectName: String, tryForceLoad: Boolean, thread: ScalaThread): ScalaObjectReference = {
-    classByName(objectName + '$', tryForceLoad: Boolean, thread: ScalaThread).fieldValue("MODULE$").asInstanceOf[ScalaObjectReference]
+    val moduleClassName = objectName + '$'
+    wrapJDIException("Exception while retrieving module debug element `" + moduleClassName + "`") {
+      classByName(moduleClassName, tryForceLoad: Boolean, thread: ScalaThread).fieldValue("MODULE$").asInstanceOf[ScalaObjectReference]
+    }
   }
 
   /** Return a reference to the type with the given name in the debugged VM.
@@ -245,7 +249,7 @@ abstract class ScalaDebugTarget private (val virtualMachine: VirtualMachine, lau
    * 
    * @throws ClassNotLoadedException if the class was not loaded yet.
    */
-  def classByName(typeName: String, tryForceLoad: Boolean, thread: ScalaThread): ScalaReferenceType = {
+  private def classByName(typeName: String, tryForceLoad: Boolean, thread: ScalaThread): ScalaReferenceType = {
     import scala.collection.JavaConverters._
     // TODO: need toList?
     virtualMachine.classesByName(typeName).asScala.toList match {
@@ -315,7 +319,7 @@ abstract class ScalaDebugTarget private (val virtualMachine: VirtualMachine, lau
 
   private def disposeThreads() {
      threads.foreach { _.dispose() }
-     threads= Nil
+     threads = Nil
   }
 
   /**
@@ -342,7 +346,7 @@ abstract class ScalaDebugTarget private (val virtualMachine: VirtualMachine, lau
    * FOR THE COMPANION ACTOR ONLY.
    */
   private[model] def initializeThreads(t: List[ThreadReference]) {
-    threads= t.map(ScalaThread(this, _))
+    threads = t.map(ScalaThread(this, _))
   }
 
   /**

org.scala-ide.sdt.debug/src/scala/tools/eclipse/debug/model/ScalaLogicalStructureProvider.scala

@@ -10,29 +10,7 @@ import org.eclipse.debug.core.model.IValue
 
 import com.sun.jdi.ClassType
 
-object ScalaLogicalStructureProvider {
-
-  def isScalaCollection(objectReference: ScalaObjectReference): Boolean = {
-    objectReference.underlying.referenceType match {
-      case classType: ClassType =>
-        implements(classType, "scala.collection.TraversableOnce")
-      case _ => // TODO: ScalaObjectReference should always reference objects of class type, never of array type. Can we just cast?
-        false
-    }
-  }
-
-  /**
-   * Checks 'implements' with Java meaning
-   */
-  def implements(classType: ClassType, interfaceName: String): Boolean = {
-    import scala.collection.JavaConverters._
-    classType.allInterfaces.asScala.exists(_.name == interfaceName)
-  }
-
-}
-
-class ScalaLogicalStructureProvider extends ILogicalStructureProvider {
-  import ScalaLogicalStructureProvider._
+class ScalaLogicalStructureProviders extends ILogicalStructureProvider {
 
   override def getLogicalStructureTypes(value: IValue) : Array[ILogicalStructureType] = {
     value match {
@@ -46,7 +24,25 @@ class ScalaLogicalStructureProvider extends ILogicalStructureProvider {
         Array() // TODO: return fixed empty Array
     }
   }
+
+  private def isScalaCollection(objectReference: ScalaObjectReference): Boolean = {
+    objectReference.wrapJDIException("Exception while checking if passed object reference is a scala collection type") {
+      objectReference.referenceType match {
+        case classType: ClassType =>
+          implements(classType, "scala.collection.TraversableOnce")
+        case _ => // TODO: ScalaObjectReference should always reference objects of class type, never of array type. Can we just cast?
+          false
+      }
+    }
+  }
 
+  /**
+   * Checks 'implements' with Java meaning
+   */
+  private def implements(classType: ClassType, interfaceName: String): Boolean = {
+    import scala.collection.JavaConverters._
+    classType.allInterfaces.asScala.exists(_.name == interfaceName)
+  }
 }
 
 object ScalaCollectionLogicalStructureType extends ILogicalStructureType with HasLogger {