converting server errors to proper client errors

docs/source/json-api/index.rst

@@ -209,23 +209,27 @@ HTTP Status Codes
 The **JSON API** reports errors using standard HTTP status codes. It divides HTTP status codes into 3 groups indicating:
 
 1. success (200)
-2. failure due to a client-side problem (400, 401, 404)
-3. failure due to a server-side problem (500)
+2. failure due to a client-side problem (400, 401, 403, 404, 409, 429)
+3. failure due to a server-side problem (500, 503)
 
 The **JSON API** can return one of the following HTTP status codes:
 
 - 200 - OK
 - 400 - Bad Request (Client Error)
 - 401 - Unauthorized, authentication required
+- 403 - Forbidden, insufficient permissions
 - 404 - Not Found
+- 409 - Conflict, contract ID or key missing or duplicated
+- 429 - Too Many Requests, ledger server has hit configured limit of in-flight commands
 - 500 - Internal Server Error
+- 503 - Service Unavailable, ledger server is not running yet or has been shut down
 
 If a client's HTTP GET or POST request reaches an API endpoint, the corresponding response will always contain a JSON object with a ``status`` field, either an ``errors`` or ``result`` field and an optional ``warnings``:
 
 .. code-block:: none
 
     {
-        "status": <400 | 401 | 404 | 500>,
+        "status": <400 | 401 | 403 | 404 | 409 | 429 | 500 | 503>,
         "errors": <JSON array of strings>, | "result": <JSON object or array>,
         ["warnings": <JSON object> ]
     }

ledger-service/http-json/src/itlib/scala/http/AbstractHttpServiceIntegrationTest.scala

@@ -1138,8 +1138,8 @@ abstract class AbstractHttpServiceIntegrationTest
       val exerciseJson: JsValue = encodeExercise(encoder)(iouExerciseTransferCommand(contractId))
       postJsonRequest(uri.withPath(Uri.Path("/v1/exercise")), exerciseJson)
         .flatMap { case (status, output) =>
-          status shouldBe StatusCodes.InternalServerError
-          assertStatus(output, StatusCodes.InternalServerError)
+          status shouldBe StatusCodes.Conflict
+          assertStatus(output, StatusCodes.Conflict)
           expectedOneErrorMessage(output) should include(
             s"Contract could not be found with id ContractId($contractIdString)"
           )

ledger-service/http-json/src/main/scala/com/digitalasset/http/CommandService.scala

@@ -21,6 +21,7 @@ import com.daml.http.util.FutureUtil._
 import com.daml.http.util.IdentifierConverters.refApiIdentifier
 import com.daml.http.util.Logging.{InstanceUUID, RequestID}
 import com.daml.http.util.{Commands, Transactions}
+import LedgerClientJwt.Grpc
 import com.daml.jwt.domain.Jwt
 import com.daml.ledger.api.refinements.{ApiTypes => lar}
 import com.daml.ledger.api.{v1 => lav1}
@@ -30,7 +31,7 @@ import scalaz.std.scalaFuture._
 import scalaz.syntax.show._
 import scalaz.syntax.std.option._
 import scalaz.syntax.traverse._
-import scalaz.{-\/, EitherT, Show, \/, \/-}
+import scalaz.{-\/, EitherT, \/, \/-}
 
 import scala.concurrent.{ExecutionContext, Future}
 import scala.util.{Failure, Success}
@@ -130,16 +131,30 @@ class CommandService(
       et.run
     }
 
-  private def logResult[A](op: Symbol, fa: Future[A])(implicit
+  private def logResult[A](
+      op: Symbol,
+      fa: Grpc.EFuture[Grpc.Category.SubmitError, A],
+  )(implicit
       lc: LoggingContextOf[InstanceUUID with RequestID]
   ): ET[A] = {
     val opName = op.name
     EitherT {
       fa.transformWith {
         case Failure(e) =>
           logger.error(s"$opName failure", e)
-          Future.successful(-\/(Error(None, e.toString)))
-        case Success(a) =>
+          Future.successful(-\/(e match {
+            case Grpc.StatusEnvelope(status) => GrpcError(status)
+            case _ => InternalError(None, e.toString)
+          }))
+        case Success(-\/(e)) =>
+          logger.error(s"$opName failure: ${e.e}: ${e.message}")
+          import Grpc.Category._
+          val tagged = e.e match {
+            case PermissionDenied => -\/(PermissionDenied)
+            case InvalidArgument => \/-(InvalidArgument)
+          }
+          Future.successful(-\/(ClientError(tagged, e.message)))
+        case Success(\/-(a)) =>
           logger.debug(s"$opName success: $a")
           Future.successful(\/-(a))
       }
@@ -218,7 +233,7 @@ class CommandService(
       case Seq(x) => \/-(x)
       case xs @ _ =>
         -\/(
-          Error(
+          InternalError(
             Some(Symbol("exactlyOneActiveContract")),
             s"Expected exactly one active contract, got: $xs",
           )
@@ -230,7 +245,10 @@ class CommandService(
   ): Error \/ ImmArraySeq[ActiveContract[lav1.value.Value]] =
     response.transaction
       .toRightDisjunction(
-        Error(Some(Symbol("activeContracts")), s"Received response without transaction: $response")
+        InternalError(
+          Some(Symbol("activeContracts")),
+          s"Received response without transaction: $response",
+        )
       )
       .flatMap(activeContracts)
 
@@ -240,15 +258,18 @@ class CommandService(
     Transactions
       .allCreatedEvents(tx)
       .traverse(ActiveContract.fromLedgerApi(_))
-      .leftMap(e => Error(Some(Symbol("activeContracts")), e.shows))
+      .leftMap(e => InternalError(Some(Symbol("activeContracts")), e.shows))
   }
 
   private def contracts(
       response: lav1.command_service.SubmitAndWaitForTransactionTreeResponse
   ): Error \/ List[Contract[lav1.value.Value]] =
     response.transaction
       .toRightDisjunction(
-        Error(Some(Symbol("contracts")), s"Received response without transaction: $response")
+        InternalError(
+          Some(Symbol("contracts")),
+          s"Received response without transaction: $response",
+        )
       )
       .flatMap(contracts)
 
@@ -257,7 +278,7 @@ class CommandService(
   ): Error \/ List[Contract[lav1.value.Value]] =
     Contract
       .fromTransactionTree(tx)
-      .leftMap(e => Error(Some(Symbol("contracts")), e.shows))
+      .leftMap(e => InternalError(Some(Symbol("contracts")), e.shows))
       .map(_.toList)
 
   private def exerciseResult(
@@ -270,7 +291,7 @@ class CommandService(
     } yield exResult
 
     result.toRightDisjunction(
-      Error(
+      InternalError(
         Some(Symbol("choiceArgument")),
         s"Cannot get exerciseResult from the first ExercisedEvent of gRPC response: ${a.toString}",
       )
@@ -287,16 +308,13 @@ class CommandService(
 }
 
 object CommandService {
-  final case class Error(id: Option[Symbol], message: String)
-
-  object Error {
-    implicit val errorShow: Show[Error] = Show shows {
-      case Error(None, message) =>
-        s"CommandService Error, $message"
-      case Error(Some(id), message) =>
-        s"CommandService Error, $id: $message"
-    }
-  }
+  sealed abstract class Error extends Product with Serializable
+  final case class ClientError(
+      id: Grpc.Category.PermissionDenied \/ Grpc.Category.InvalidArgument,
+      message: String,
+  ) extends Error
+  final case class GrpcError(status: io.grpc.Status) extends Error
+  final case class InternalError(id: Option[Symbol], message: String) extends Error
 
   private type ET[A] = EitherT[Future, Error, A]
 

ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsService.scala

@@ -222,9 +222,8 @@ class ContractsService(
 
   private def lookupResult(
       errorOrAc: Option[Error \/ domain.ActiveContract[LfValue]]
-  ): Future[Option[domain.ActiveContract[LfValue]]] = {
-    errorOrAc.cata(x => toFuture(x).map(Some(_)), Future.successful(None))
-  }
+  ): Future[Option[domain.ActiveContract[LfValue]]] =
+    errorOrAc traverse (toFuture(_))
 
   private def isContractId(k: domain.ContractId)(a: domain.ActiveContract[LfValue]): Boolean =
     (a.contractId: domain.ContractId) == k
@@ -405,7 +404,7 @@ class ContractsService(
       queryParams: InMemoryQuery,
   )(implicit
       lc: LoggingContextOf[InstanceUUID]
-  ): Source[Error \/ domain.ActiveContract[LfValue], NotUsed] = {
+  ): Source[InternalError \/ domain.ActiveContract[LfValue], NotUsed] = {
 
     logger.debug(
       s"Searching in memory, parties: $parties, templateIds: $templateIds, queryParms: $queryParams"
@@ -423,7 +422,7 @@ class ContractsService(
         val (errors, converted) = step.toInsertDelete.partitionMapPreservingIds { apiEvent =>
           domain.ActiveContract
             .fromLedgerApi(apiEvent)
-            .leftMap(e => Error(Symbol("searchInMemory"), e.shows))
+            .leftMap(e => InternalError(Symbol("searchInMemory"), e.shows))
             .flatMap(apiAcToLfAc): Error \/ Ac
         }
         val convertedInserts = converted.inserts filter { ac =>
@@ -529,7 +528,7 @@ class ContractsService(
       ac: domain.ActiveContract[ApiValue]
   ): Error \/ domain.ActiveContract[LfValue] =
     ac.traverse(ApiValueToLfValueConverter.apiValueToLfValue)
-      .leftMap(e => Error(Symbol("apiAcToLfAc"), e.shows))
+      .leftMap(e => InternalError(Symbol("apiAcToLfAc"), e.shows))
 
   private[http] def valuePredicate(
       templateId: domain.TemplateId.RequiredPkg,
@@ -539,7 +538,7 @@ class ContractsService(
 
   private def lfValueToJsValue(a: LfValue): Error \/ JsValue =
     \/.attempt(LfValueCodec.apiValueToJsValue(a))(e =>
-      Error(Symbol("lfValueToJsValue"), e.description)
+      InternalError(Symbol("lfValueToJsValue"), e.description)
     )
 
   private[http] def resolveTemplateIds[Tid <: domain.TemplateId.OptionalPkg](
@@ -644,7 +643,9 @@ object ContractsService {
     ): Source[Error \/ domain.ActiveContract[LfV], NotUsed]
   }
 
-  case class Error(id: Symbol, message: String)
+  final case class Error(id: Symbol, message: String)
+  private type InternalError = Error
+  private[http] val InternalError: Error.type = Error
 
   object Error {
     implicit val errorShow: Show[Error] = Show shows { e =>

ledger-service/http-json/src/main/scala/com/digitalasset/http/Endpoints.scala

@@ -41,12 +41,10 @@ import com.daml.http.util.{ProtobufByteStrings, toLedgerId}
 import com.daml.jwt.domain.Jwt
 import com.daml.ledger.api.{v1 => lav1}
 import com.daml.logging.LoggingContextOf.withEnrichedLoggingContext
-import com.daml.scalautil.ExceptionOps._
 import scalaz.std.scalaFuture._
 import scalaz.syntax.std.option._
-import scalaz.syntax.show._
 import scalaz.syntax.traverse._
-import scalaz.{-\/, EitherT, NonEmptyList, Show, Traverse, \/, \/-}
+import scalaz.{-\/, EitherT, NonEmptyList, Traverse, \/, \/-}
 import spray.json._
 
 import scala.concurrent.duration.FiniteDuration
@@ -454,7 +452,8 @@ class Endpoints(
   def allParties(req: HttpRequest)(implicit
       lc: LoggingContextOf[InstanceUUID with RequestID]
   ): ET[domain.SyncResponse[List[domain.PartyDetails]]] =
-    proxyWithoutCommand((jwt, _) => partiesService.allParties(jwt))(req).map(domain.OkResponse(_))
+    proxyWithoutCommand((jwt, _) => partiesService.allParties(jwt))(req)
+      .flatMap(pd => either(pd map (domain.OkResponse(_))))
 
   def parties(req: HttpRequest)(implicit
       lc: LoggingContextOf[InstanceUUID with RequestID]
@@ -522,31 +521,24 @@ class Endpoints(
     } yield domain.OkResponse(())
 
   private def handleFutureEitherFailure[A, B](fa: Future[A \/ B])(implicit
-      A: IntoServerError[A],
+      A: IntoEndpointsError[A],
       lc: LoggingContextOf[InstanceUUID with RequestID],
   ): Future[Error \/ B] =
-    fa.map(_ leftMap A.run).recover { case NonFatal(e) =>
-      logger.error("Future failed", e)
-      -\/(ServerError(e.description))
-    }
+    fa.map(_ leftMap A.run)
+      .recover(logException("Future") andThen Error.fromThrowable andThen (-\/(_)))
 
-  private def handleFutureFailure[E >: ServerError, A](fa: Future[A])(implicit
+  private def handleFutureFailure[A](fa: Future[A])(implicit
       lc: LoggingContextOf[InstanceUUID with RequestID]
-  ): Future[E \/ A] =
-    fa.map(a => \/-(a)).recover { case NonFatal(e) =>
-      logger.error("Future failed", e)
-      -\/(ServerError(e.description))
-    }
+  ): Future[Error \/ A] =
+    fa.map(a => \/-(a)).recover(logException("Future") andThen Error.fromThrowable andThen (-\/(_)))
 
-  private def handleSourceFailure[E: Show, A](implicit
-      lc: LoggingContextOf[InstanceUUID with RequestID]
+  private def handleSourceFailure[E, A](implicit
+      E: IntoEndpointsError[E],
+      lc: LoggingContextOf[InstanceUUID with RequestID],
   ): Flow[E \/ A, Error \/ A, NotUsed] =
     Flow
-      .fromFunction((_: E \/ A).liftErr[Error](ServerError))
-      .recover { case NonFatal(e) =>
-        logger.error("Source failed", e)
-        -\/(ServerError(e.description))
-      }
+      .fromFunction((_: E \/ A).leftMap(E.run))
+      .recover(logException("Source") andThen Error.fromThrowable andThen (-\/(_)))
 
   private def httpResponse(
       output: Future[Error \/ SearchResult[Error \/ JsValue]]
@@ -556,17 +548,22 @@ class Endpoints(
         case -\/(e) => httpResponseError(e)
         case \/-(searchResult) => httpResponse(searchResult)
       }
-      .recover { case NonFatal(e) =>
-        httpResponseError(ServerError(e.description))
-      }
+      .recover(Error.fromThrowable andThen (httpResponseError(_)))
+
+  private[this] def logException(fromWhat: String)(implicit
+      lc: LoggingContextOf[InstanceUUID with RequestID]
+  ): Throwable PartialFunction Throwable = { case NonFatal(e) =>
+    logger.error(s"$fromWhat failed", e)
+    e
+  }
 
   private def httpResponse(searchResult: SearchResult[Error \/ JsValue]): HttpResponse = {
     import json.JsonProtocol._
 
     val response: Source[ByteString, NotUsed] = searchResult match {
       case domain.OkResponse(result, warnings, _) =>
         val warningsJsVal: Option[JsValue] = warnings.map(SprayJson.encodeUnsafe(_))
-        ResponseFormats.resultJsObject(result, warningsJsVal)
+        ResponseFormats.resultJsObject(result via filterStreamErrors, warningsJsVal)
       case error: domain.ErrorResponse =>
         val jsVal: JsValue = SprayJson.encodeUnsafe(error)
         Source.single(ByteString(jsVal.compactPrint))
@@ -579,6 +576,12 @@ class Endpoints(
     )
   }
 
+  private[this] def filterStreamErrors[E, A]: Flow[Error \/ A, Error \/ A, NotUsed] =
+    Flow[Error \/ A].map {
+      case -\/(ServerError(_)) => -\/(ServerError("internal server error"))
+      case o => o
+    }
+
   private def httpResponse[A: JsonWriter](
       result: ET[domain.SyncResponse[A]]
   )(implicit metrics: Metrics): Future[HttpResponse] = {
@@ -598,9 +601,7 @@ class Endpoints(
               status = status,
             )
         }
-        .recover { case NonFatal(e) =>
-          httpResponseError(ServerError(e.description))
-        },
+        .recover(Error.fromThrowable andThen (httpResponseError(_))),
     )
   }
 
@@ -747,12 +748,27 @@ object Endpoints {
 
   private type LfValue = lf.value.Value
 
-  private final class IntoServerError[-A](val run: A => Error) extends AnyVal
-  private object IntoServerError extends IntoServerErrorLow {
-    implicit val id: IntoServerError[Error] = new IntoServerError(identity)
-  }
-  private sealed abstract class IntoServerErrorLow {
-    implicit def shown[A: Show]: IntoServerError[A] = new IntoServerError(a => ServerError(a.shows))
+  private final class IntoEndpointsError[-A](val run: A => Error) extends AnyVal
+  private object IntoEndpointsError {
+    import LedgerClientJwt.Grpc.Category
+
+    implicit val id: IntoEndpointsError[Error] = new IntoEndpointsError(identity)
+
+    implicit val fromCommands: IntoEndpointsError[CommandService.Error] = new IntoEndpointsError({
+      case CommandService.InternalError(id, message) =>
+        ServerError(s"command service error, ${id.cata(sym => s"${sym.name}: ", "")}$message")
+      case CommandService.GrpcError(status) =>
+        ParticipantServerError(status.getCode, Option(status.getDescription))
+      case CommandService.ClientError(-\/(Category.PermissionDenied), message) =>
+        Unauthorized(message)
+      case CommandService.ClientError(\/-(Category.InvalidArgument), message) =>
+        InvalidUserInput(message)
+    })
+
+    implicit val fromContracts: IntoEndpointsError[ContractsService.Error] =
+      new IntoEndpointsError({ case ContractsService.InternalError(id, msg) =>
+        ServerError(s"contracts service error, ${id.name}: $msg")
+      })
   }
 
   private def lfValueToJsValue(a: LfValue): Error \/ JsValue =