Skip to content

Commit

Permalink
Don't duplicate log failed futures in the http json api (commandservi…
Browse files Browse the repository at this point in the history
…ce specific ones) (#9990)

* Don't duplicate log failed futures in the http json api (commandservice specific)

changelog_begin

- [JSON API] Errors which arise from the CommandService are not logged twice anymore (thus reducing noise)

changelog_end

* Fix duplicate logging of a failed future differently via adding another error case in CommandService.scala

* Restore old formatting

* Improve comment

* Remove the wrong changes I accidentally made, the functions are quite similar xD

* remove the additional type and just make the id optional & remove unecessary comment
  • Loading branch information
realvictorprm authored Jun 15, 2021
1 parent 0c32cf2 commit 3d2d4b9
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class CommandService(
val command = createCommand(input)
val request = submitAndWaitRequest(jwtPayload, input.meta, command, "create")
val et: ET[ActiveContract[lav1.value.Value]] = for {
response <- rightT(logResult(Symbol("create"), submitAndWaitForTransaction(jwt, request)))
response <- logResult(Symbol("create"), submitAndWaitForTransaction(jwt, request))
contract <- either(exactlyOneActiveContract(response))
} yield contract
et.run
Expand All @@ -69,9 +69,8 @@ class CommandService(
val request = submitAndWaitRequest(jwtPayload, input.meta, command, "exercise")

val et: ET[ExerciseResponse[lav1.value.Value]] = for {
response <- rightT(
response <-
logResult(Symbol("exercise"), submitAndWaitForTransactionTree(jwt, request))
)
exerciseResult <- either(exerciseResult(response))
contracts <- either(contracts(response))
} yield ExerciseResponse(exerciseResult, contracts)
Expand All @@ -90,8 +89,9 @@ class CommandService(
val command = createAndExerciseCommand(input)
val request = submitAndWaitRequest(jwtPayload, input.meta, command, "createAndExercise")
val et: ET[ExerciseResponse[lav1.value.Value]] = for {
response <- rightT(
logResult(Symbol("createAndExercise"), submitAndWaitForTransactionTree(jwt, request))
response <- logResult(
Symbol("createAndExercise"),
submitAndWaitForTransactionTree(jwt, request),
)
exerciseResult <- either(exerciseResult(response))
contracts <- either(contracts(response))
Expand All @@ -102,13 +102,18 @@ class CommandService(

private def logResult[A](op: Symbol, fa: Future[A])(implicit
lc: LoggingContextOf[InstanceUUID with RequestID]
): Future[A] = {
): ET[A] = {
val opName = op.name
fa.onComplete {
case Failure(e) => logger.error(s"$opName failure", e)
case Success(a) => logger.debug(s"$opName success: $a")
EitherT {
fa.transformWith {
case Failure(e) =>
logger.error(s"$opName failure", e)
Future.successful(-\/(Error(None, e.toString)))
case Success(a) =>
logger.debug(s"$opName success: $a")
Future.successful(\/-(a))
}
}
fa
}

private def createCommand(
Expand Down Expand Up @@ -184,7 +189,7 @@ class CommandService(
case xs @ _ =>
-\/(
Error(
Symbol("exactlyOneActiveContract"),
Some(Symbol("exactlyOneActiveContract")),
s"Expected exactly one active contract, got: $xs",
)
)
Expand All @@ -195,7 +200,7 @@ class CommandService(
): Error \/ ImmArraySeq[ActiveContract[lav1.value.Value]] =
response.transaction
.toRightDisjunction(
Error(Symbol("activeContracts"), s"Received response without transaction: $response")
Error(Some(Symbol("activeContracts")), s"Received response without transaction: $response")
)
.flatMap(activeContracts)

Expand All @@ -205,22 +210,25 @@ class CommandService(
Transactions
.allCreatedEvents(tx)
.traverse(ActiveContract.fromLedgerApi(_))
.leftMap(e => Error(Symbol("activeContracts"), e.shows))
.leftMap(e => Error(Some(Symbol("activeContracts")), e.shows))
}

private def contracts(
response: lav1.command_service.SubmitAndWaitForTransactionTreeResponse
): Error \/ List[Contract[lav1.value.Value]] =
response.transaction
.toRightDisjunction(
Error(Symbol("contracts"), s"Received response without transaction: $response")
Error(Some(Symbol("contracts")), s"Received response without transaction: $response")
)
.flatMap(contracts)

private def contracts(
tx: lav1.transaction.TransactionTree
): Error \/ List[Contract[lav1.value.Value]] =
Contract.fromTransactionTree(tx).leftMap(e => Error(Symbol("contracts"), e.shows)).map(_.toList)
Contract
.fromTransactionTree(tx)
.leftMap(e => Error(Some(Symbol("contracts")), e.shows))
.map(_.toList)

private def exerciseResult(
a: lav1.command_service.SubmitAndWaitForTransactionTreeResponse
Expand All @@ -233,7 +241,7 @@ class CommandService(

result.toRightDisjunction(
Error(
Symbol("choiceArgument"),
Some(Symbol("choiceArgument")),
s"Cannot get exerciseResult from the first ExercisedEvent of gRPC response: ${a.toString}",
)
)
Expand All @@ -249,11 +257,14 @@ class CommandService(
}

object CommandService {
final case class Error(id: Symbol, message: String)
final case class Error(id: Option[Symbol], message: String)

object Error {
implicit val errorShow: Show[Error] = Show shows { e =>
s"CommandService Error, ${e.id: Symbol}: ${e.message: String}"
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"
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,9 @@ class Endpoints(
resolvedCmd = cmd.copy(argument = apiArg, reference = resolvedRef)

resp <- eitherT(
handleFutureEitherFailure(commandService.exercise(jwt, jwtPayload, resolvedCmd))
handleFutureEitherFailure(
commandService.exercise(jwt, jwtPayload, resolvedCmd)
)
): ET[domain.ExerciseResponse[ApiValue]]

jsVal <- either(SprayJson.encode1(resp).liftErr(ServerError)): ET[JsValue]
Expand All @@ -179,7 +181,9 @@ class Endpoints(
): ET[domain.CreateAndExerciseCommand[ApiRecord, ApiValue, TemplateId.RequiredPkg]]

resp <- eitherT(
handleFutureEitherFailure(commandService.createAndExercise(jwt, jwtPayload, cmd))
handleFutureEitherFailure(
commandService.createAndExercise(jwt, jwtPayload, cmd)
)
): ET[domain.ExerciseResponse[ApiValue]]

jsVal <- either(SprayJson.encode1(resp).liftErr(ServerError)): ET[JsValue]
Expand Down

0 comments on commit 3d2d4b9

Please sign in to comment.