Add optional ledgerApiError details to responses (#13751)

* Add optional ledgerApiError details to responses * Finally get some proper error details * Cleaning up the code * Less bad words in the code * Better deserialization error msg changelog_begin - [JSON-API] The response of the json api now includes a field ledgerApiError that contains extended information in the case of ledger errors. See issue #9834 for more details. changelog_end * Reformat! * Remove debug logging statements * also update security evidence file * more cleanup * Improve Json decoder & encoder * Add test for json encoder&decoder * Add test which ensures that the LedgerApiError field in the response is filled with valid information * Fix failing failure tests
digital-asset · Apr 29, 2022 · 2e6dbef · 2e6dbef
1 parent 281de9a
commit 2e6dbef
Show file tree

Hide file tree

Showing 12 changed files with 223 additions and 46 deletions.
diff --git a/ledger-service/http-json/BUILD.bazel b/ledger-service/http-json/BUILD.bazel
@@ -65,6 +65,7 @@ hj_scalacopts = lf_scalacopts + [
             "//ledger-service/jwt",
             "//ledger-service/lf-value-json",
             "//ledger-service/utils",
+            "//ledger/error",
             "//ledger/ledger-api-auth",
             "//ledger/ledger-api-common",
             "//ledger/ledger-resources",
@@ -253,6 +254,7 @@ daml_compile(
             "//ledger-service/jwt",
             "//ledger-service/lf-value-json",
             "//ledger-service/utils",
+            "//ledger/error",
             "//ledger/ledger-api-common",
             "//libs-scala/contextualized-logging",
             "//libs-scala/db-utils",
@@ -312,6 +314,7 @@ alias(
             "//ledger-api/rs-grpc-bridge",
             "//ledger-api/testing-utils",
             "//ledger-service/fetch-contracts",
+            "//ledger/error",
             "//ledger/ledger-resources",
             "//ledger/metrics",
             "//ledger/sandbox-common",

diff --git a/ledger-service/http-json/src/it/scala/http/HttpServiceIntegrationTest.scala b/ledger-service/http-json/src/it/scala/http/HttpServiceIntegrationTest.scala
@@ -155,7 +155,7 @@ abstract class HttpServiceIntegrationTest
       val Status = StatusCodes.BadRequest
       discard { exerciseTest._1 should ===(Status) }
       inside(exerciseTest._2.convertTo[domain.ErrorResponse]) {
-        case domain.ErrorResponse(Seq(lookup), None, Status) =>
+        case domain.ErrorResponse(Seq(lookup), None, Status, _) =>
           lookup should include regex raw"Cannot resolve Template Key type, given: TemplateId\([0-9a-f]{64},IIou,IIou\)"
       }
     }

diff --git a/ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala b/ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala
@@ -7,6 +7,7 @@ import java.time.{Instant, LocalDate}
 import akka.http.scaladsl.Http
 import akka.http.scaladsl.model._
 import com.daml.api.util.TimestampConversion
+import com.daml.error.utils.ErrorDetails.{ErrorInfoDetail, RequestInfoDetail, ResourceInfoDetail}
 import com.daml.lf.data.Ref
 import com.daml.http.domain.ContractId
 import com.daml.http.domain.TemplateId.OptionalPkg
@@ -347,14 +348,15 @@ abstract class AbstractHttpServiceIntegrationTestTokenIndependent
           encoder,
           headers,
         ).map { response =>
-          inside(response) { case domain.ErrorResponse(errors, warnings, StatusCodes.BadRequest) =>
-            errors shouldBe List(ErrorMessages.cannotResolveAnyTemplateId)
-            inside(warnings) { case Some(domain.UnknownTemplateIds(unknownTemplateIds)) =>
-              unknownTemplateIds.toSet shouldBe Set(
-                domain.TemplateId(None, "AAA", "BBB"),
-                domain.TemplateId(None, "XXX", "YYY"),
-              )
-            }
+          inside(response) {
+            case domain.ErrorResponse(errors, warnings, StatusCodes.BadRequest, _) =>
+              errors shouldBe List(ErrorMessages.cannotResolveAnyTemplateId)
+              inside(warnings) { case Some(domain.UnknownTemplateIds(unknownTemplateIds)) =>
+                unknownTemplateIds.toSet shouldBe Set(
+                  domain.TemplateId(None, "AAA", "BBB"),
+                  domain.TemplateId(None, "XXX", "YYY"),
+                )
+              }
           }
         }
       }
@@ -494,6 +496,7 @@ abstract class AbstractHttpServiceIntegrationTestTokenIndependent
             Seq(_),
             Some(domain.UnknownTemplateIds(Seq(TpId.IIou.IIou))),
             StatusCodes.BadRequest,
+            _,
           ) =>
         succeed
     }
@@ -704,6 +707,28 @@ abstract class AbstractHttpServiceIntegrationTestTokenIndependent
           expectedOneErrorMessage(output) should include(
             s"Contract could not be found with id $contractIdString"
           )
+          val ledgerApiError =
+            output.asJsObject.fields("ledgerApiError").convertTo[domain.LedgerApiError]
+          ledgerApiError.message should include("CONTRACT_NOT_FOUND")
+          ledgerApiError.message should include(
+            "Contract could not be found with id 000000000000000000000000000000000000000000000000000000000000000000"
+          )
+          import org.scalatest.Inspectors._
+          forExactly(1, ledgerApiError.details) {
+            case ErrorInfoDetail(errorCodeId, _) =>
+              errorCodeId shouldBe "CONTRACT_NOT_FOUND"
+            case _ => fail()
+          }
+          forExactly(1, ledgerApiError.details) {
+            case RequestInfoDetail(_) => succeed
+            case _ => fail()
+          }
+          forExactly(1, ledgerApiError.details) {
+            case ResourceInfoDetail(name, typ) =>
+              name shouldBe "000000000000000000000000000000000000000000000000000000000000000000"
+              typ shouldBe "CONTRACT_ID"
+            case _ => fail()
+          }
         }: Future[Assertion]
   }
 
@@ -951,7 +976,7 @@ abstract class AbstractHttpServiceIntegrationTestTokenIndependent
       ).flatMap { case (status, output) =>
         status shouldBe StatusCodes.BadRequest
         inside(decode1[domain.SyncResponse, List[domain.PartyDetails]](output)) {
-          case \/-(domain.ErrorResponse(List(error), None, StatusCodes.BadRequest)) =>
+          case \/-(domain.ErrorResponse(List(error), None, StatusCodes.BadRequest, _)) =>
             error should include("Daml-LF Party is empty")
         }
       }: Future[Assertion]

diff --git a/ledger-service/http-json/src/main/scala/com/digitalasset/http/CommandService.scala b/ledger-service/http-json/src/main/scala/com/digitalasset/http/CommandService.scala
@@ -313,7 +313,7 @@ object CommandService {
       id: Grpc.Category.PermissionDenied \/ Grpc.Category.InvalidArgument,
       message: String,
   ) extends Error
-  final case class GrpcError(status: io.grpc.Status) extends Error
+  final case class GrpcError(status: com.google.rpc.Status) extends Error
   final case class InternalError(id: Option[Symbol], error: Throwable) extends Error
   object InternalError {
     def apply(id: Option[Symbol], message: String): InternalError =

diff --git 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
@@ -522,7 +522,7 @@ object Endpoints {
           )
         )
       case CommandService.GrpcError(status) =>
-        ParticipantServerError(status.getCode, Option(status.getDescription))
+        ParticipantServerError(status)
       case CommandService.ClientError(-\/(Category.PermissionDenied), message) =>
         Unauthorized(message)
       case CommandService.ClientError(\/-(Category.InvalidArgument), message) =>

diff --git a/ledger-service/http-json/src/main/scala/com/digitalasset/http/EndpointsCompanion.scala b/ledger-service/http-json/src/main/scala/com/digitalasset/http/EndpointsCompanion.scala
@@ -7,7 +7,7 @@ import akka.http.scaladsl.model._
 import akka.http.scaladsl.server.RouteResult.Complete
 import akka.http.scaladsl.server.{RequestContext, Route}
 import akka.util.ByteString
-import com.daml.http.domain.{JwtPayload, JwtPayloadLedgerIdOnly, JwtWritePayload}
+import com.daml.http.domain.{JwtPayload, JwtPayloadLedgerIdOnly, JwtWritePayload, LedgerApiError}
 import com.daml.http.json.SprayJson
 import com.daml.http.util.Logging.{InstanceUUID, RequestID, extendWithRequestIdLogCtx}
 import util.GrpcHttpErrorCodes._
@@ -20,12 +20,15 @@ import com.daml.ledger.api.auth.{
 }
 import com.daml.ledger.api.domain.UserRight
 import UserRight.{CanActAs, CanReadAs}
+import com.daml.error.utils.ErrorDetails
+import com.daml.error.utils.ErrorDetails.ErrorDetail
 import com.daml.ledger.api.refinements.{ApiTypes => lar}
 import com.daml.ledger.client.services.admin.UserManagementClient
 import com.daml.ledger.client.services.identity.LedgerIdentityClient
 import com.daml.lf.data.Ref.UserId
 import com.daml.logging.{ContextualizedLogger, LoggingContextOf}
-import io.grpc.Status.{Code => GrpcCode}
+import com.google.rpc.{Code => GrpcCode}
+import com.google.rpc.Status
 import scalaz.syntax.std.option._
 import scalaz.{-\/, EitherT, Monad, NonEmptyList, Show, \/, \/-}
 import spray.json.JsValue
@@ -45,8 +48,20 @@ object EndpointsCompanion {
 
   final case class ServerError(message: Throwable) extends Error
 
-  final case class ParticipantServerError(grpcStatus: GrpcCode, description: Option[String])
-      extends Error
+  final case class ParticipantServerError(
+      grpcStatus: GrpcCode,
+      description: String,
+      details: Seq[ErrorDetail],
+  ) extends Error
+
+  object ParticipantServerError {
+    def apply(status: Status): ParticipantServerError =
+      ParticipantServerError(
+        com.google.rpc.Code.forNumber(status.getCode),
+        status.getMessage,
+        ErrorDetails.from(status),
+      )
+  }
 
   final case class NotFound(message: String) extends Error
 
@@ -58,16 +73,15 @@ object EndpointsCompanion {
   object Error {
     implicit val ShowInstance: Show[Error] = Show shows {
       case InvalidUserInput(e) => s"Endpoints.InvalidUserInput: ${e: String}"
-      case ParticipantServerError(s, d) =>
-        s"Endpoints.ParticipantServerError: ${s: GrpcCode}${d.cata((": " + _), "")}"
+      case ParticipantServerError(grpcStatus, description, _) =>
+        s"Endpoints.ParticipantServerError: $grpcStatus: $description"
       case ServerError(e) => s"Endpoints.ServerError: ${e.getMessage: String}"
       case Unauthorized(e) => s"Endpoints.Unauthorized: ${e: String}"
       case NotFound(e) => s"Endpoints.NotFound: ${e: String}"
     }
 
     def fromThrowable: Throwable PartialFunction Error = {
-      case LedgerClientJwt.Grpc.StatusEnvelope(status) =>
-        ParticipantServerError(status.getCode, Option(status.getDescription))
+      case LedgerClientJwt.Grpc.StatusEnvelope(status) => ParticipantServerError(status)
       case NonFatal(t) => ServerError(t)
     }
   }
@@ -245,17 +259,37 @@ object EndpointsCompanion {
   private[http] def errorResponse(
       error: Error
   )(implicit lc: LoggingContextOf[InstanceUUID with RequestID]): domain.ErrorResponse = {
-    val (status, errorMsg): (StatusCode, String) = error match {
-      case InvalidUserInput(e) => StatusCodes.BadRequest -> e
-      case ParticipantServerError(grpcStatus, d) =>
-        grpcStatus.asAkkaHttpForJsonApi -> s"$grpcStatus${d.cata((": " + _), "")}"
+    def mkErrorResponse(
+        status: StatusCode,
+        error: String,
+        ledgerApiError: Option[LedgerApiError] = None,
+    ) =
+      domain.ErrorResponse(
+        errors = List(error),
+        warnings = None,
+        status = status,
+        ledgerApiError = ledgerApiError,
+      )
+    error match {
+      case InvalidUserInput(e) => mkErrorResponse(StatusCodes.BadRequest, e)
+      case ParticipantServerError(grpcStatus, description, details) =>
+        val ledgerApiError =
+          domain.LedgerApiError(
+            code = grpcStatus.getNumber,
+            message = description,
+            details = details,
+          )
+        mkErrorResponse(
+          grpcStatus.asAkkaHttpForJsonApi,
+          s"$grpcStatus: $description",
+          Some(ledgerApiError),
+        )
       case ServerError(reason) =>
         logger.error(s"Internal server error occured", reason)
-        StatusCodes.InternalServerError -> "HTTP JSON API Server Error"
-      case Unauthorized(e) => StatusCodes.Unauthorized -> e
-      case NotFound(e) => StatusCodes.NotFound -> e
+        mkErrorResponse(StatusCodes.InternalServerError, "HTTP JSON API Server Error")
+      case Unauthorized(e) => mkErrorResponse(StatusCodes.Unauthorized, e)
+      case NotFound(e) => mkErrorResponse(StatusCodes.NotFound, e)
     }
-    domain.ErrorResponse(errors = List(errorMsg), warnings = None, status = status)
   }
 
   private[http] def httpResponse(status: StatusCode, data: JsValue): HttpResponse = {

diff --git a/ledger-service/http-json/src/main/scala/com/digitalasset/http/LedgerClientJwt.scala b/ledger-service/http-json/src/main/scala/com/digitalasset/http/LedgerClientJwt.scala
@@ -22,17 +22,18 @@ import com.daml.ledger.client.withoutledgerid.{LedgerClient => DamlLedgerClient}
 import com.daml.lf.data.Ref
 import com.daml.logging.{ContextualizedLogger, LoggingContextOf}
 import com.google.protobuf
-import io.grpc.Status, Status.Code, Code.{values => _, _}
-import scalaz.{OneAnd, \/, -\/}
+import scalaz.{-\/, OneAnd, \/}
 import scalaz.syntax.std.boolean._
 
-import scala.concurrent.{ExecutionContext => EC, Future}
+import scala.concurrent.{Future, ExecutionContext => EC}
 import scala.util.control.NonFatal
 import com.daml.ledger.api.{domain => LedgerApiDomain}
 import com.daml.ledger.api.v1.admin.metering_report_service.{
   GetMeteringReportRequest,
   GetMeteringReportResponse,
 }
+import com.google.rpc.{Code, Status}
+import io.grpc.protobuf.StatusProto
 
 object LedgerClientJwt {
   import Grpc.EFuture, Grpc.Category._
@@ -199,13 +200,13 @@ object LedgerClientJwt {
   def listKnownParties(client: DamlLedgerClient)(implicit ec: EC): ListKnownParties =
     jwt =>
       client.partyManagementClient.listKnownParties(bearer(jwt)).requireHandling {
-        case PERMISSION_DENIED => PermissionDenied
+        case Code.PERMISSION_DENIED => PermissionDenied
       }
 
   def getParties(client: DamlLedgerClient)(implicit ec: EC): GetParties =
     (jwt, partyIds) =>
       client.partyManagementClient.getParties(partyIds, bearer(jwt)).requireHandling {
-        case PERMISSION_DENIED => PermissionDenied
+        case Code.PERMISSION_DENIED => PermissionDenied
       }
 
   def allocateParty(client: DamlLedgerClient): AllocateParty =
@@ -253,9 +254,14 @@ object LedgerClientJwt {
     private[http] object StatusEnvelope {
       def unapply(t: Throwable): Option[Status] = t match {
         case NonFatal(t) =>
-          val s = Status fromThrowable t
-          // fromThrowable uses UNKNOWN if it didn't find one
-          (s.getCode != UNKNOWN) option s
+          val status = StatusProto fromThrowable t
+          if (status == null) None
+          else {
+            // fromThrowable uses UNKNOWN if it didn't find one
+            val code = com.google.rpc.Code.forNumber(status.getCode)
+            if (code == null) None
+            else (code != com.google.rpc.Code.UNKNOWN) option status
+          }
         case _ => None
       }
     }
@@ -276,8 +282,8 @@ object LedgerClientJwt {
       // think of it more like a Venn diagram
 
       private[LedgerClientJwt] val submitErrors: Code PartialFunction SubmitError = {
-        case PERMISSION_DENIED => PermissionDenied
-        case INVALID_ARGUMENT => InvalidArgument
+        case Code.PERMISSION_DENIED => PermissionDenied
+        case Code.INVALID_ARGUMENT => InvalidArgument
       }
 
       private[LedgerClientJwt] implicit final class `Future Status Category ops`[A](
@@ -286,7 +292,7 @@ object LedgerClientJwt {
         def requireHandling[E](c: Code PartialFunction E)(implicit ec: EC): EFuture[E, A] =
           fa map \/.right[Error[E], A] recover Function.unlift {
             case StatusEnvelope(status) =>
-              c.lift(status.getCode) map (e => -\/(Error(e, status.asRuntimeException.getMessage)))
+              c.lift(Code.forNumber(status.getCode)) map (e => -\/(Error(e, status.getMessage)))
             case _ => None
           }
       }

diff --git a/ledger-service/http-json/src/main/scala/com/digitalasset/http/domain.scala b/ledger-service/http-json/src/main/scala/com/digitalasset/http/domain.scala
@@ -49,6 +49,7 @@ package object domain extends com.daml.fetchcontracts.domain.Aliases {
 
 package domain {
 
+  import com.daml.error.utils.ErrorDetails.ErrorDetail
   import com.daml.fetchcontracts.domain.`fc domain ErrorOps`
 
   trait JwtPayloadTag
@@ -562,10 +563,17 @@ package domain {
       status: StatusCode = StatusCodes.OK,
   ) extends SyncResponse[R]
 
+  final case class LedgerApiError(
+      code: Int,
+      message: String,
+      details: Seq[ErrorDetail],
+  )
+
   final case class ErrorResponse(
       errors: List[String],
       warnings: Option[ServiceWarning],
       status: StatusCode,
+      ledgerApiError: Option[LedgerApiError] = None,
   ) extends SyncResponse[Nothing]
 
   object OkResponse {