Better error messages for unserializable annotations (#1885)

3 files changed, 35 insertions, 3 deletions

diff --git a/src/main/scala/firrtl/annotations/AnnotationUtils.scala b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
index a1276e0e..eccb4e13 100644
--- a/src/main/scala/firrtl/annotations/AnnotationUtils.scala
+++ b/src/main/scala/firrtl/annotations/AnnotationUtils.scala
@@ -18,6 +18,16 @@ case class AnnotationClassNotFoundException(className: String)
     extends FirrtlUserException(
       s"Annotation class $className not found! Please check spelling and classpath"
     )
+class UnserializableAnnotationException private (msg: String) extends FirrtlUserException(msg)
+object UnserializableAnnotationException {
+  private def toMessage(pair: (Annotation, Throwable)): String =
+    s"Failed to serialiaze annotation of type ${pair._1.getClass.getName} because '${pair._2.getMessage}'"
+  private[firrtl] def apply(badAnnos: Seq[(Annotation, Throwable)]) = {
+    require(badAnnos.nonEmpty)
+    val msg = badAnnos.map(toMessage).mkString("\n  ", "\n  ", "\n")
+    new UnserializableAnnotationException(msg)
+  }
+}
 
 object AnnotationUtils {
 
diff --git a/src/main/scala/firrtl/annotations/JsonProtocol.scala b/src/main/scala/firrtl/annotations/JsonProtocol.scala
index 0ef8b020..144cd894 100644
--- a/src/main/scala/firrtl/annotations/JsonProtocol.scala
+++ b/src/main/scala/firrtl/annotations/JsonProtocol.scala
@@ -10,7 +10,7 @@ import scala.util.{Failure, Try}
 import org.json4s._
 import org.json4s.native.JsonMethods._
 import org.json4s.native.Serialization
-import org.json4s.native.Serialization.{read, writePretty}
+import org.json4s.native.Serialization.{read, write, writePretty}
 
 trait HasSerializationHints {
   // For serialization of complicated constructor arguments, let the annotation
@@ -220,6 +220,13 @@ object JsonProtocol {
   /** Serialize annotations to a String for emission */
   def serialize(annos: Seq[Annotation]): String = serializeTry(annos).get
 
+  private def findUnserializeableAnnos(
+    annos: Seq[Annotation]
+  )(
+    implicit formats: Formats
+  ): Seq[(Annotation, Throwable)] =
+    annos.map(a => a -> Try(write(a))).collect { case (a, Failure(e)) => (a, e) }
+
   def serializeTry(annos: Seq[Annotation]): Try[String] = {
     val tags = annos
       .flatMap({
@@ -229,7 +236,11 @@ object JsonProtocol {
       .distinct
 
     implicit val formats = jsonFormat(tags)
-    Try(writePretty(annos))
+    Try(writePretty(annos)).recoverWith {
+      case e: org.json4s.MappingException =>
+        val badAnnos = findUnserializeableAnnos(annos)
+        Failure(if (badAnnos.isEmpty) e else UnserializableAnnotationException(badAnnos))
+    }
   }
 
   def deserialize(in: JsonInput): Seq[Annotation] = deserializeTry(in).get
diff --git a/src/test/scala/firrtlTests/annotationTests/JsonProtocolSpec.scala b/src/test/scala/firrtlTests/annotationTests/JsonProtocolSpec.scala
index 54a94edb..44ad7d1f 100644
--- a/src/test/scala/firrtlTests/annotationTests/JsonProtocolSpec.scala
+++ b/src/test/scala/firrtlTests/annotationTests/JsonProtocolSpec.scala
@@ -3,12 +3,14 @@
 package firrtlTests.annotationTests
 
 import firrtl._
-import firrtl.annotations.{JsonProtocol, NoTargetAnnotation}
+import firrtl.annotations.{JsonProtocol, NoTargetAnnotation, UnserializableAnnotationException}
 import firrtl.ir._
 import firrtl.options.Dependency
+import scala.util.Failure
 import _root_.logger.{LogLevel, LogLevelAnnotation, Logger}
 import org.scalatest.flatspec.AnyFlatSpec
 import org.scalatest.matchers.should._
+import org.scalatest.Inside._
 
 case class AnAnnotation(
   info:       Info,
@@ -66,4 +68,13 @@ class JsonProtocolSpec extends AnyFlatSpec with Matchers {
       compiler.execute(CircuitState(circuit, Nil))
     }
   }
+  "Trying to serialize annotations that cannot be serialized" should "tell you why" in {
+    case class MyAnno(x: Int) extends NoTargetAnnotation
+    inside(JsonProtocol.serializeTry(MyAnno(3) :: Nil)) {
+      case Failure(e: UnserializableAnnotationException) =>
+        e.getMessage should include("MyAnno")
+        // From json4s Exception
+        e.getMessage should include("Classes defined in method bodies are not supported")
+    }
+  }
 }