Add ExceptionOps to extract exception details safely (#3822)

* Add ExceptionOps to extract exception details safely `Throwable.getMessage` can return `null` which caused unexpected NPE in JSON API error handling * Fix formatting * Remove show instance * Addressing code review comments, thanks Stefano!
2024-09-20 09:17:43 +03:00 · 2019-12-11 13:38:44 -05:00 · 2019-12-11 13:38:44 -05:00 · 0b172da49a
commit 0b172da49a
parent 2bd8b36571
12 changed files with 54 additions and 24 deletions
--- a/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/data/MapKOps.scala
+++ b/daml-lf/data/src/main/scala/com/digitalasset/daml/lf/data/MapKOps.scala
@ -8,7 +8,8 @@ import scala.collection.GenTraversableOnce
 import scala.collection.immutable.{Map, MapLike}

 /** Halfway between the *-kinded MapLike and *->*->*-kinded MapOps. */
-trait MapKOps[K, +V, +This[+TV] <: Map[K, TV] with MapKOps[K, TV, This]] extends MapLike[K, V, This[V]] { this: This[V] =>
+trait MapKOps[K, +V, +This[+TV] <: Map[K, TV] with MapKOps[K, TV, This]]
+    extends MapLike[K, V, This[V]] { this: This[V] =>
  override def updated[V1 >: V](key: K, value: V1): This[V1] = this + ((key, value))
  override def +[V1 >: V](kv: (K, V1)): This[V1]
  override def +[V1 >: V](elem1: (K, V1), elem2: (K, V1), elems: (K, V1)*): This[V1] =
--- a/language-support/scala/codegen-testing/src/test/scala/com/digitalasset/ledger/client/binding/ValueSpec.scala
+++ b/language-support/scala/codegen-testing/src/test/scala/com/digitalasset/ledger/client/binding/ValueSpec.scala
@ -90,7 +90,8 @@ object ValueSpec {

    override def valueTextMap[A](implicit vc: ValueCheck[A]) = {
      import vc._
-      implicit val arbTM: Arbitrary[P.TextMap[A]] = Arbitrary(GenEncoding.primitive.valueTextMap(TA.arbitrary))
+      implicit val arbTM: Arbitrary[P.TextMap[A]] = Arbitrary(
+        GenEncoding.primitive.valueTextMap(TA.arbitrary))
      ValueCheck[P.TextMap[A]](s"Map[$tName]")
    }

--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/Config.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/Config.scala
@ -6,10 +6,11 @@ package com.digitalasset.http
 import java.io.File
 import java.nio.file.Path

+import com.digitalasset.http.util.ExceptionOps._
 import com.digitalasset.ledger.api.refinements.ApiTypes.ApplicationId
-import scalaz.{Show, \/}
 import scalaz.std.option._
 import scalaz.syntax.traverse._
+import scalaz.{Show, \/}

 import scala.concurrent.duration.FiniteDuration
 import scala.util.Try
@ -53,14 +54,14 @@ private[http] abstract class ConfigCompanion[A](name: String) {

  protected def parseBoolean(k: String)(v: String): String \/ Boolean =
    \/.fromTryCatchNonFatal(v.toBoolean).leftMap(e =>
-      s"$k=$v must be a boolean value: ${e.getMessage}")
+      s"$k=$v must be a boolean value: ${e.description}")

  protected def requiredDirectoryField(m: Map[String, String])(k: String): Either[String, File] =
    requiredField(m)(k).flatMap(directory)

  protected def directory(s: String): Either[String, File] =
    Try(new File(s).getAbsoluteFile).toEither.left
-      .map(e => e.getMessage)
+      .map(e => e.description)
      .flatMap { d =>
        if (d.isDirectory) Right(d)
        else Left(s"Directory does not exist: ${d.getAbsolutePath}")
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsFetch.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsFetch.scala
@ -26,6 +26,7 @@ import com.digitalasset.http.json.JsonProtocol.LfValueDatabaseCodec.{
  apiValueToJsValue => lfValueToDbJsValue
 }
 import com.digitalasset.http.util.IdentifierConverters.apiIdentifier
+import com.digitalasset.http.util.ExceptionOps._
 import com.digitalasset.jwt.domain.Jwt
 import com.digitalasset.ledger.api.v1.transaction.Transaction
 import com.digitalasset.ledger.api.{v1 => lav1}
@ -81,10 +82,10 @@ private class ContractsFetch(
      logger.debug(s"contractsIo, maxAttempts: $maxAttempts")
      contractsIo_(jwt, party, templateId).exceptSql {
        case e if maxAttempts > 0 && retrySqlStates(e.getSQLState) =>
-          logger.debug(s"contractsIo, exception: ${e.getMessage}, state: ${e.getSQLState}")
+          logger.debug(s"contractsIo, exception: ${e.description}, state: ${e.getSQLState}")
          connection.rollback flatMap (_ => loop(maxAttempts - 1))
        case e @ _ =>
-          logger.error(s"contractsIo3 exception: ${e.getMessage}, state: ${e.getSQLState}")
+          logger.error(s"contractsIo3 exception: ${e.description}, state: ${e.getSQLState}")
          connection.raiseError(e)
      }
    }
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsService.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsService.scala
@ -14,6 +14,7 @@ import com.digitalasset.http.json.JsonProtocol.LfValueCodec
 import com.digitalasset.http.query.ValuePredicate
 import com.digitalasset.http.query.ValuePredicate.LfV
 import com.digitalasset.http.util.ApiValueToLfValueConverter
+import com.digitalasset.http.util.ExceptionOps._
 import com.digitalasset.http.util.FutureUtil.toFuture
 import com.digitalasset.http.util.IdentifierConverters.apiIdentifier
 import com.digitalasset.jwt.domain.Jwt
@ -300,7 +301,7 @@ class ContractsService(

  private def lfValueToJsValue(a: LfValue): Error \/ JsValue =
    \/.fromTryCatchNonFatal(LfValueCodec.apiValueToJsValue(a)).leftMap(e =>
-      Error('lfValueToJsValue, e.getMessage))
+      Error('lfValueToJsValue, e.description))
 }

 object ContractsService {
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala
@ -14,6 +14,7 @@ import com.digitalasset.daml.lf
 import com.digitalasset.http.Statement.discard
 import com.digitalasset.http.domain.JwtPayload
 import com.digitalasset.http.json.{DomainJsonDecoder, DomainJsonEncoder, ResponseFormats, SprayJson}
+import com.digitalasset.http.util.ExceptionOps._
 import com.digitalasset.http.util.FutureUtil.{either, eitherT}
 import com.digitalasset.http.util.{ApiValueToLfValueConverter, FutureUtil}
 import com.digitalasset.jwt.domain.{DecodedJwt, Jwt}
@ -105,14 +106,14 @@ class Endpoints(
    fa.map(a => a.leftMap(e => ServerError(e.shows))).recover {
      case NonFatal(e) =>
        logger.error("Future failed", e)
-        -\/(ServerError(e.getMessage))
+        -\/(ServerError(e.description))
    }

  private def handleFutureFailure[A](fa: Future[A]): Future[ServerError \/ A] =
    fa.map(a => \/-(a)).recover {
      case NonFatal(e) =>
        logger.error("Future failed", e)
-        -\/(ServerError(e.getMessage))
+        -\/(ServerError(e.description))
    }

  private def handleSourceFailure[E: Show, A]: Flow[E \/ A, ServerError \/ A, NotUsed] =
@ -121,7 +122,7 @@ class Endpoints(
      .recover {
        case NonFatal(e) =>
          logger.error("Source failed", e)
-          -\/(ServerError(e.getMessage))
+          -\/(ServerError(e.description))
      }

  private def encodeList(as: Seq[JsValue]): ServerError \/ JsValue =
@ -200,7 +201,7 @@ class Endpoints(

  private def lfValueToJsValue(a: LfValue): Error \/ JsValue =
    \/.fromTryCatchNonFatal(LfValueCodec.apiValueToJsValue(a)).leftMap(e =>
-      ServerError(e.getMessage))
+      ServerError(e.description))

  private def collectActiveContracts(
      predicates: Map[domain.TemplateId.RequiredPkg, LfValue => Boolean]): PartialFunction[
@ -230,7 +231,7 @@ class Endpoints(
        case -\/(e) => httpResponseError(e)
      }
      .recover {
-        case NonFatal(e) => httpResponseError(ServerError(e.getMessage))
+        case NonFatal(e) => httpResponseError(ServerError(e.description))
      }
  }

@ -242,7 +243,7 @@ class Endpoints(
        case \/-(source) => httpResponseFromSource(source)
      }
      .recover {
-        case NonFatal(e) => httpResponseError(ServerError(e.getMessage))
+        case NonFatal(e) => httpResponseError(ServerError(e.description))
      }

  private def httpResponseOk(data: JsValue): HttpResponse =
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/HttpService.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/HttpService.scala
@ -21,6 +21,7 @@ import com.digitalasset.http.json.{
  JsValueToApiValueConverter
 }
 import com.digitalasset.http.util.ApiValueToLfValueConverter
+import com.digitalasset.http.util.ExceptionOps._
 import com.digitalasset.http.util.FutureUtil._
 import com.digitalasset.http.util.IdentifierConverters.apiLedgerId
 import com.digitalasset.jwt.JwtDecoder
@ -33,8 +34,8 @@ import com.digitalasset.ledger.client.configuration.{
  LedgerIdRequirement
 }
 import com.digitalasset.ledger.client.services.pkg.PackageClient
-import com.digitalasset.ledger.service.LedgerReader.PackageStore
 import com.digitalasset.ledger.service.LedgerReader
+import com.digitalasset.ledger.service.LedgerReader.PackageStore
 import com.typesafe.scalalogging.StrictLogging
 import io.grpc.netty.NettyChannelBuilder
 import scalaz.Scalaz._
@ -231,6 +232,6 @@ object HttpService extends StrictLogging {
      .map(_.right)
      .recover {
        case NonFatal(e) =>
-          \/.left(Error(s"Cannot connect to the ledger server, error: ${e.getMessage}"))
+          \/.left(Error(s"Cannot connect to the ledger server, error: ${e.description}"))
      }
 }
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/HttpCodec.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/HttpCodec.scala
@ -6,11 +6,12 @@ package com.digitalasset.http.json
 import akka.http.scaladsl.marshalling.{Marshaller, ToEntityMarshaller}
 import akka.http.scaladsl.model.MediaTypes.`application/json`
 import akka.http.scaladsl.model.StatusCodes
-import akka.http.scaladsl.server.ExceptionHandler
 import akka.http.scaladsl.server.Directives._
+import akka.http.scaladsl.server.ExceptionHandler
 import akka.http.scaladsl.unmarshalling.{FromEntityUnmarshaller, Unmarshaller}
-import scalaz.{@@, Tag}
+import com.digitalasset.http.util.ExceptionOps._
 import scalaz.std.stream.unfold
+import scalaz.{@@, Tag}
 import spray.json._

 import scala.concurrent.{ExecutionContext, Future}
@ -27,7 +28,7 @@ object HttpCodec {
          StatusCodes.BadRequest,
          ResponseFormats.errorsJsObject(
            StatusCodes.BadRequest,
-            s"JSON parser error: ${e.msg}" +: unfoldCauses(e.cause).map(_.getMessage): _*)))
+            s"JSON parser error: ${e.msg}" +: unfoldCauses(e.cause).map(_.description): _*)))
  }

  private[this] def unfoldCauses(t: Throwable): Seq[Throwable] =
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/JsonError.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/JsonError.scala
@ -3,6 +3,7 @@

 package com.digitalasset.http.json

+import com.digitalasset.http.util.ExceptionOps._
 import scalaz.Show
 import scalaz.syntax.show._

@ -13,7 +14,7 @@ object JsonError {

  def toJsonError[E: Show](e: E) = JsonError(e.shows)

-  def toJsonError(e: Throwable) = JsonError(e.getMessage)
+  def toJsonError(e: Throwable) = JsonError(e.description)

  implicit val ShowInstance: Show[JsonError] = Show shows { f =>
    s"JsonError: ${f.message}"
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/SprayJson.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/json/SprayJson.scala
@ -3,6 +3,7 @@

 package com.digitalasset.http.json

+import com.digitalasset.http.util.ExceptionOps._
 import scalaz.{-\/, Show, \/, \/-}
 import spray.json.{JsValue, JsonReader, _}

@ -32,7 +33,7 @@ object SprayJson {
  }

  def parse(str: String): JsonReaderError \/ JsValue =
-    \/.fromTryCatchNonFatal(JsonParser(str)).leftMap(e => JsonReaderError(str, e.getMessage))
+    \/.fromTryCatchNonFatal(JsonParser(str)).leftMap(e => JsonReaderError(str, e.description))

  def decode[A: JsonReader](str: String): JsonReaderError \/ A =
    for {
@ -41,11 +42,11 @@ object SprayJson {
    } yield a

  def decode[A: JsonReader](a: JsValue): JsonReaderError \/ A =
-    \/.fromTryCatchNonFatal(a.convertTo[A]).leftMap(e => JsonReaderError(a.toString, e.getMessage))
+    \/.fromTryCatchNonFatal(a.convertTo[A]).leftMap(e => JsonReaderError(a.toString, e.description))

  def encode[A: JsonWriter](a: A): JsonWriterError \/ JsValue = {
    import spray.json._
-    \/.fromTryCatchNonFatal(a.toJson).leftMap(e => JsonWriterError(a, e.getMessage))
+    \/.fromTryCatchNonFatal(a.toJson).leftMap(e => JsonWriterError(a, e.description))
  }

  def mustBeJsObject(a: JsValue): JsonError \/ JsObject = a match {
--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/util/ApiValueToLfValueConverter.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/util/ApiValueToLfValueConverter.scala
@ -14,7 +14,8 @@ object ApiValueToLfValueConverter {

  object Error {
    implicit val ErrorShow: Show[Error] = Show shows { e =>
-      s"ApiValueToLfValueConverter.Error: ${e.cause.getMessage}"
+      import ExceptionOps._
+      s"ApiValueToLfValueConverter.Error: ${e.cause.description}"
    }
  }

--- a/ledger-service/http-json/src/main/scala/com/digitalasset/http/util/ExceptionOps.scala
+++ b/ledger-service/http-json/src/main/scala/com/digitalasset/http/util/ExceptionOps.scala
@ -0,0 +1,19 @@
+// Copyright (c) 2019 The DAML Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package com.digitalasset.http.util
+
+object ExceptionOps {
+
+  implicit class ExceptionOps(val underlying: Throwable) extends AnyVal {
+    def description: String = getDescription(underlying)
+  }
+
+  def getDescription(e: Throwable): String = {
+    val name: String = e.getClass.getName
+    Option(e.getMessage).filter(_.nonEmpty) match {
+      case Some(m) => s"$name: $m"
+      case None => name
+    }
+  }
+}