From 60fd736958b3b38c6076f584cfb86cada1cbcc92 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Mon, 18 Oct 2021 19:45:24 +0200 Subject: [PATCH 1/9] [DPP-618] Error codes: Adapt error codes in ApiPackageService CHANGELOG_BEGIN CHANGELOG_END --- .../error/definitions/LedgerApiErrors.scala | 38 ++++++++++- .../server/api/ValidationLogger.scala | 4 +- .../api/validation/ErrorFactories.scala | 32 ++++++++- .../api/validation/ErrorFactoriesSpec.scala | 29 +++++++- .../platform/apiserver/ApiServices.scala | 3 +- .../services/ApiPackageService.scala | 67 +++++++++++-------- 6 files changed, 138 insertions(+), 35 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index 25fe24578d77..ac028de53e56 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -8,12 +8,12 @@ import com.daml.error.definitions.ErrorGroups.ParticipantErrorGroup.TransactionE import com.daml.lf.data.Ref import com.daml.lf.data.Ref.PackageId import com.daml.lf.engine.Error.Validation.ReplayMismatch +import com.daml.lf.engine.{Error => LfError} +import com.daml.lf.interpretation.{Error => LfInterpretationError} import com.daml.lf.language.{LanguageVersion, LookupError, Reference} import com.daml.lf.transaction.GlobalKey import com.daml.lf.value.Value import com.daml.lf.{VersionRange, language} -import com.daml.lf.engine.{Error => LfError} -import com.daml.lf.interpretation.{Error => LfInterpretationError} object LedgerApiErrors extends LedgerApiErrorGroup { @@ -33,6 +33,40 @@ object LedgerApiErrors extends LedgerApiErrorGroup { } object ReadErrors extends ErrorGroup() { + + @Explanation("This rejection is given when a package id is malformed.") + @Resolution( + """Make sure the package id provided in the request has correct form.""".stripMargin + ) + object MalformedPackageId + extends ErrorCode( + id = "MALFORMED_PACKAGE_ID", + ErrorCategory.InvalidIndependentOfSystemState, + ) { + case class Reject(message: String)(implicit + loggingContext: ContextualizedErrorLogger + ) extends LoggingTransactionErrorImpl( + cause = message + ) + } + + @Explanation( + "This rejection is given when a read request tries to access a package which does not exist." + ) + @Resolution("""Make sure the requested package is available on the ledger. + |It might have not been uploaded or the upload might have been rejected.""".stripMargin) + object CouldNotFindPackage + extends ErrorCode( + id = "COULD_NOT_FIND_PACKAGE", + ErrorCategory.InvalidGivenCurrentSystemStateResourceMissing, + ) { + case class Reject()(implicit + loggingContext: ContextualizedErrorLogger + ) extends LoggingTransactionErrorImpl( + cause = "Could not found package." + ) + } + @Explanation("This rejection is given when a read request tries to access pruned data.") @Resolution("Use an offset that is after the pruning offset.") object ParticipantPrunedDataAccessed diff --git a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/ValidationLogger.scala b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/ValidationLogger.scala index f62a53df42ee..ef8d9facdebb 100644 --- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/ValidationLogger.scala +++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/ValidationLogger.scala @@ -15,10 +15,10 @@ object ValidationLogger { t } - def logFailureWithContext[Request](request: Request, t: Throwable)(implicit + def logFailureWithContext[Request, T <: Throwable](request: Request, t: T)(implicit logger: ContextualizedLogger, loggingContext: LoggingContext, - ): Throwable = { + ): T = { logger.debug(s"Request validation failed for $request. Message: ${t.getMessage}") logger.info(t.getMessage) t diff --git a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala index 6d7ea01906b6..324f4bf4c821 100644 --- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala +++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala @@ -7,11 +7,12 @@ import com.daml.error.definitions.LedgerApiErrors import com.daml.error.{ContextualizedErrorLogger, ErrorCodesVersionSwitcher} import com.daml.ledger.api.domain.LedgerId import com.daml.ledger.grpc.GrpcStatuses -import com.daml.platform.server.api.ApiException +import com.daml.logging.{ContextualizedLogger, LoggingContext} import com.daml.platform.server.api.validation.ErrorFactories.{ addDefiniteAnswerDetails, definiteAnswers, } +import com.daml.platform.server.api.{ApiException, ValidationLogger} import com.google.protobuf.{Any => AnyProto} import com.google.rpc.{ErrorInfo, Status} import io.grpc.Status.Code @@ -21,6 +22,35 @@ import scalaz.syntax.tag._ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitcher) { + def malformedPackageId[Request](request: Request, message: String)(implicit + contextualizedErrorLogger: ContextualizedErrorLogger, + logger: ContextualizedLogger, + loggingContext: LoggingContext, + ): StatusRuntimeException = { + errorCodesVersionSwitcher.choose( + v1 = ValidationLogger.logFailureWithContext( + request, + io.grpc.Status.INVALID_ARGUMENT + .withDescription(message) + .asRuntimeException(), + ), + v2 = LedgerApiErrors.ReadErrors.MalformedPackageId + .Reject( + message = message + ) + .asGrpcError, + ) + } + + def couldNotFindPackage(implicit + contextualizedErrorLogger: ContextualizedErrorLogger + ): StatusRuntimeException = { + errorCodesVersionSwitcher.choose( + v1 = io.grpc.Status.NOT_FOUND.asRuntimeException(), + v2 = LedgerApiErrors.ReadErrors.CouldNotFindPackage.Reject().asGrpcError, + ) + } + def duplicateCommandException(implicit contextualizedErrorLogger: ContextualizedErrorLogger ): StatusRuntimeException = diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 2ba482d91820..8673b36484cb 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -4,8 +4,8 @@ package com.daml import com.daml.error.{ - DamlContextualizedErrorLogger, ContextualizedErrorLogger, + DamlContextualizedErrorLogger, ErrorCodesVersionSwitcher, } import com.daml.ledger.api.domain.LedgerId @@ -31,6 +31,33 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope new DamlContextualizedErrorLogger(logger, loggingContext, Some(correlationId)) "ErrorFactories" should { + + "return malformedPackageId" in { + assertVersionedError( + _.malformedPackageId(request = "request123", message = "message123")( + contextualizedErrorLogger = contextualizedErrorLogger, + logger = logger, + loggingContext = loggingContext, + ) + )( + v1_code = Code.INVALID_ARGUMENT, + v1_message = "message123", + v1_details = Seq.empty, + v2_code = Code.INVALID_ARGUMENT, + v2_message = s"MALFORMED_PACKAGE_ID(8,$correlationId): message123", + ) + } + + "return couldNotFindPackage" in { + assertVersionedError(_.couldNotFindPackage)( + v1_code = Code.NOT_FOUND, + v1_message = "", + v1_details = Seq.empty, + v2_code = Code.NOT_FOUND, + v2_message = s"COULD_NOT_FIND_PACKAGE(11,$correlationId): Could not found package.", + ) + } + "return the DuplicateCommandException" in { assertVersionedError(_.duplicateCommandException)( v1_code = Code.ALREADY_EXISTS, diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala index d3ccb7ce8593..017cf6f127f3 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/ApiServices.scala @@ -156,7 +156,8 @@ private[daml] object ApiServices { val apiVersionService = ApiVersionService.create() - val apiPackageService = ApiPackageService.create(ledgerId, packagesService) + val apiPackageService = + ApiPackageService.create(ledgerId, packagesService, errorsVersionsSwitcher) val apiConfigurationService = ApiLedgerConfigurationService.create(ledgerId, configurationService) diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala index 8d966427a76e..c71fda6b0ea9 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala @@ -4,11 +4,8 @@ package com.daml.platform.apiserver.services import com.daml.daml_lf_dev.DamlLf.{Archive, HashFunction} +import com.daml.error.{DamlContextualizedErrorLogger, ErrorCodesVersionSwitcher} import com.daml.ledger.api.domain.LedgerId -import com.daml.ledger.api.v1.package_service.HashFunction.{ - SHA256 => APISHA256, - Unrecognized => APIUnrecognized, -} import com.daml.ledger.api.v1.package_service.PackageServiceGrpc.PackageService import com.daml.ledger.api.v1.package_service.{HashFunction => APIHashFunction, _} import com.daml.ledger.participant.state.index.v2.IndexPackagesService @@ -16,20 +13,25 @@ import com.daml.lf.data.Ref import com.daml.logging.LoggingContext.withEnrichedLoggingContext import com.daml.logging.{ContextualizedLogger, LoggingContext} import com.daml.platform.api.grpc.GrpcApiService -import com.daml.platform.server.api.ValidationLogger -import com.daml.platform.server.api.validation.PackageServiceValidation -import io.grpc.{BindableService, ServerServiceDefinition, Status} +import com.daml.platform.server.api.validation.{ErrorFactories, PackageServiceValidation} +import io.grpc.{BindableService, ServerServiceDefinition} import scala.concurrent.{ExecutionContext, Future} private[apiserver] final class ApiPackageService private ( - backend: IndexPackagesService + backend: IndexPackagesService, + errorCodesVersionSwitcher: ErrorCodesVersionSwitcher, )(implicit executionContext: ExecutionContext, loggingContext: LoggingContext) extends PackageService with GrpcApiService { private implicit val logger: ContextualizedLogger = ContextualizedLogger.get(this.getClass) + private implicit val contextualizedErrorLogger: DamlContextualizedErrorLogger = + new DamlContextualizedErrorLogger(logger, loggingContext, None) + + private val errorFactories = ErrorFactories(errorCodesVersionSwitcher) + override def bindService(): ServerServiceDefinition = PackageServiceGrpc.bindService(this, executionContext) @@ -47,13 +49,14 @@ private[apiserver] final class ApiPackageService private ( withEnrichedLoggingContext("packageId" -> request.packageId) { implicit loggingContext => logger.info(s"Received request for a package: $request") withValidatedPackageId(request.packageId, request) { packageId => + def archiveOToResponse(archiveO: Option[Archive]): Future[GetPackageResponse] = + archiveO.fold( + Future.failed(errorFactories.couldNotFindPackage) + )(archive => Future.successful(toGetPackageResponse(archive))) + backend .getLfArchive(packageId) - .flatMap( - _.fold(Future.failed[GetPackageResponse](Status.NOT_FOUND.asRuntimeException()))( - archive => Future.successful(toGetPackageResponse(archive)) - ) - ) + .flatMap(archiveOToResponse) .andThen(logger.logErrorsOnCall[GetPackageResponse]) } } @@ -80,28 +83,27 @@ private[apiserver] final class ApiPackageService private ( private def withValidatedPackageId[T, R](packageId: String, request: R)( block: Ref.PackageId => Future[T] - ) = + ): Future[T] = Ref.PackageId .fromString(packageId) .fold( - error => + errorMessage => Future.failed[T]( - ValidationLogger.logFailureWithContext( - request, - Status.INVALID_ARGUMENT - .withDescription(error) - .asRuntimeException(), - ) + errorFactories.malformedPackageId(request = request, message = errorMessage) ), - pId => block(pId), + packageId => block(packageId), ) private def toGetPackageResponse(archive: Archive): GetPackageResponse = { - val hashF: APIHashFunction = archive.getHashFunction match { - case HashFunction.SHA256 => APISHA256 - case _ => APIUnrecognized(-1) + val hashFunction = archive.getHashFunction match { + case HashFunction.SHA256 => APIHashFunction.SHA256 + case _ => APIHashFunction.Unrecognized(-1) } - GetPackageResponse(hashF, archive.getPayload, archive.getHash) + GetPackageResponse( + hashFunction = hashFunction, + archivePayload = archive.getPayload, + hash = archive.getHash, + ) } } @@ -110,12 +112,21 @@ private[platform] object ApiPackageService { def create( ledgerId: LedgerId, backend: IndexPackagesService, + errorCodesVersionSwitcher: ErrorCodesVersionSwitcher, )(implicit executionContext: ExecutionContext, loggingContext: LoggingContext, - ): PackageService with GrpcApiService = - new PackageServiceValidation(new ApiPackageService(backend), ledgerId) with BindableService { + ): PackageService with GrpcApiService = { + val service = new ApiPackageService( + backend = backend, + errorCodesVersionSwitcher = errorCodesVersionSwitcher, + ) + new PackageServiceValidation( + service = service, + ledgerId = ledgerId, + ) with BindableService { override def bindService(): ServerServiceDefinition = PackageServiceGrpc.bindService(this, executionContext) } + } } From fbdb476d5111a4318b38414180cff3b07c73a063 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Tue, 19 Oct 2021 08:05:27 +0200 Subject: [PATCH 2/9] fix --- .../scala/platform/apiserver/services/ApiPackageService.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala index c71fda6b0ea9..3e4ca014a5e7 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala @@ -51,7 +51,7 @@ private[apiserver] final class ApiPackageService private ( withValidatedPackageId(request.packageId, request) { packageId => def archiveOToResponse(archiveO: Option[Archive]): Future[GetPackageResponse] = archiveO.fold( - Future.failed(errorFactories.couldNotFindPackage) + Future.failed[GetPackageResponse](errorFactories.couldNotFindPackage) )(archive => Future.successful(toGetPackageResponse(archive))) backend From 5858fe8cdc3f77507f37fe1a72274539248d72a3 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Tue, 19 Oct 2021 14:59:33 +0200 Subject: [PATCH 3/9] TV review --- .../scala/com/daml/error/ErrorResource.scala | 3 +++ .../error/definitions/LedgerApiErrors.scala | 19 ++++++++++++------- .../api/validation/ErrorFactories.scala | 4 ++-- .../api/validation/ErrorFactoriesSpec.scala | 8 ++++---- .../services/ApiPackageService.scala | 18 +++++++++++------- 5 files changed, 32 insertions(+), 20 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala b/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala index aa4dcc83444b..07219f03ee2f 100644 --- a/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala +++ b/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala @@ -35,4 +35,7 @@ object ErrorResource { object CommandId extends ErrorResource { def asString: String = "COMMAND_ID" } + object PackageId extends ErrorResource { + def asString: String = "PACKAGE_ID" + } } diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index ac028de53e56..fec12244d82e 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -51,20 +51,25 @@ object LedgerApiErrors extends LedgerApiErrorGroup { } @Explanation( - "This rejection is given when a read request tries to access a package which does not exist." + "This rejection is given when a read request tries to access a package which does not exist on the ledger." ) - @Resolution("""Make sure the requested package is available on the ledger. - |It might have not been uploaded or the upload might have been rejected.""".stripMargin) - object CouldNotFindPackage + @Resolution("Use a package id pertaining to a package existing on the ledger.") + // TODO error codes: Possible duplicate of `LedgerApiErrors.Package.MissingPackage` + object PackageNotFound extends ErrorCode( - id = "COULD_NOT_FIND_PACKAGE", + id = "PACKAGE_NOT_FOUND", ErrorCategory.InvalidGivenCurrentSystemStateResourceMissing, ) { - case class Reject()(implicit + case class Reject(packageId: String)(implicit loggingContext: ContextualizedErrorLogger ) extends LoggingTransactionErrorImpl( cause = "Could not found package." - ) + ) { + + override def resources: Seq[(ErrorResource, String)] = { + super.resources :+ ((ErrorResource.PackageId, packageId)) + } + } } @Explanation("This rejection is given when a read request tries to access pruned data.") diff --git a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala index 324f4bf4c821..0e48e5d341fa 100644 --- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala +++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala @@ -42,12 +42,12 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch ) } - def couldNotFindPackage(implicit + def packageNotFound(packageId: String)(implicit contextualizedErrorLogger: ContextualizedErrorLogger ): StatusRuntimeException = { errorCodesVersionSwitcher.choose( v1 = io.grpc.Status.NOT_FOUND.asRuntimeException(), - v2 = LedgerApiErrors.ReadErrors.CouldNotFindPackage.Reject().asGrpcError, + v2 = LedgerApiErrors.ReadErrors.PackageNotFound.Reject(packageId = packageId).asGrpcError, ) } diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 8673b36484cb..6c08b7ee5149 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -12,7 +12,7 @@ import com.daml.ledger.api.domain.LedgerId import com.daml.logging.{ContextualizedLogger, LoggingContext} import com.daml.platform.server.api.validation.ErrorFactories import com.daml.platform.server.api.validation.ErrorFactories._ -import com.google.rpc.Status +import com.google.rpc._ import io.grpc.Status.Code import io.grpc.StatusRuntimeException import io.grpc.protobuf.StatusProto @@ -48,13 +48,13 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope ) } - "return couldNotFindPackage" in { - assertVersionedError(_.couldNotFindPackage)( + "return packageNotFound" in { + assertVersionedError(_.packageNotFound("packageId123"))( v1_code = Code.NOT_FOUND, v1_message = "", v1_details = Seq.empty, v2_code = Code.NOT_FOUND, - v2_message = s"COULD_NOT_FIND_PACKAGE(11,$correlationId): Could not found package.", + v2_message = s"PACKAGE_NOT_FOUND(11,$correlationId): Could not found package.", ) } diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala index 3e4ca014a5e7..4ceb060e2d9f 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala @@ -27,7 +27,10 @@ private[apiserver] final class ApiPackageService private ( private implicit val logger: ContextualizedLogger = ContextualizedLogger.get(this.getClass) - private implicit val contextualizedErrorLogger: DamlContextualizedErrorLogger = + // NOTE: Using `def` to capture the most specific `loggingContext` at the call sites. + private implicit def contextualizedErrorLogger(implicit + loggingContext: LoggingContext + ): DamlContextualizedErrorLogger = new DamlContextualizedErrorLogger(logger, loggingContext, None) private val errorFactories = ErrorFactories(errorCodesVersionSwitcher) @@ -49,14 +52,15 @@ private[apiserver] final class ApiPackageService private ( withEnrichedLoggingContext("packageId" -> request.packageId) { implicit loggingContext => logger.info(s"Received request for a package: $request") withValidatedPackageId(request.packageId, request) { packageId => - def archiveOToResponse(archiveO: Option[Archive]): Future[GetPackageResponse] = - archiveO.fold( - Future.failed[GetPackageResponse](errorFactories.couldNotFindPackage) - )(archive => Future.successful(toGetPackageResponse(archive))) - backend .getLfArchive(packageId) - .flatMap(archiveOToResponse) + .flatMap { + case None => + Future.failed[GetPackageResponse]( + errorFactories.packageNotFound(packageId = packageId) + ) + case Some(archive) => Future.successful(toGetPackageResponse(archive)) + } .andThen(logger.logErrorsOnCall[GetPackageResponse]) } } From 12a8d169340c98d80ce2d79b8e56984bc7f6ed03 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Tue, 19 Oct 2021 15:03:26 +0200 Subject: [PATCH 4/9] 1 --- .../main/scala/com/daml/error/definitions/LedgerApiErrors.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index fec12244d82e..a32008deb4f5 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -38,6 +38,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup { @Resolution( """Make sure the package id provided in the request has correct form.""".stripMargin ) + // TODO error codes: Consider replacing with a variant `LedgerApiErrors.CommandValidation.InvalidArgument` that does not imply command validation object MalformedPackageId extends ErrorCode( id = "MALFORMED_PACKAGE_ID", From af3af3111b5d047c4dc7fe967330ce2eb4617b40 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Tue, 19 Oct 2021 15:04:31 +0200 Subject: [PATCH 5/9] 1 --- .../main/scala/com/daml/error/definitions/LedgerApiErrors.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index a32008deb4f5..77ed26d1b087 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -38,7 +38,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup { @Resolution( """Make sure the package id provided in the request has correct form.""".stripMargin ) - // TODO error codes: Consider replacing with a variant `LedgerApiErrors.CommandValidation.InvalidArgument` that does not imply command validation + // TODO error codes: Consider using `LedgerApiErrors.CommandValidation.InvalidArgument` object MalformedPackageId extends ErrorCode( id = "MALFORMED_PACKAGE_ID", From 93a86133a8e395993eba189c17377b69664dcde0 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Tue, 19 Oct 2021 15:38:03 +0200 Subject: [PATCH 6/9] 1 --- .../main/scala/com/daml/error/definitions/LedgerApiErrors.scala | 2 +- .../platform/server/api/validation/ErrorFactoriesSpec.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index 77ed26d1b087..84065c0528af 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -64,7 +64,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup { case class Reject(packageId: String)(implicit loggingContext: ContextualizedErrorLogger ) extends LoggingTransactionErrorImpl( - cause = "Could not found package." + cause = "Could not find package." ) { override def resources: Seq[(ErrorResource, String)] = { diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 6c08b7ee5149..a2bc4648687b 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -54,7 +54,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope v1_message = "", v1_details = Seq.empty, v2_code = Code.NOT_FOUND, - v2_message = s"PACKAGE_NOT_FOUND(11,$correlationId): Could not found package.", + v2_message = s"PACKAGE_NOT_FOUND(11,$correlationId): Could not find package.", ) } From 1670911937ae25fd057bab2b69371581f507be54 Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Wed, 20 Oct 2021 09:16:09 +0200 Subject: [PATCH 7/9] TV review --- .../services/ApiPackageService.scala | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala index 4ceb060e2d9f..617be74fb331 100644 --- a/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala +++ b/ledger/participant-integration-api/src/main/scala/platform/apiserver/services/ApiPackageService.scala @@ -27,12 +27,6 @@ private[apiserver] final class ApiPackageService private ( private implicit val logger: ContextualizedLogger = ContextualizedLogger.get(this.getClass) - // NOTE: Using `def` to capture the most specific `loggingContext` at the call sites. - private implicit def contextualizedErrorLogger(implicit - loggingContext: LoggingContext - ): DamlContextualizedErrorLogger = - new DamlContextualizedErrorLogger(logger, loggingContext, None) - private val errorFactories = ErrorFactories(errorCodesVersionSwitcher) override def bindService(): ServerServiceDefinition = @@ -57,7 +51,9 @@ private[apiserver] final class ApiPackageService private ( .flatMap { case None => Future.failed[GetPackageResponse]( - errorFactories.packageNotFound(packageId = packageId) + errorFactories.packageNotFound(packageId = packageId)( + createContextualizedErrorLogger + ) ) case Some(archive) => Future.successful(toGetPackageResponse(archive)) } @@ -93,7 +89,11 @@ private[apiserver] final class ApiPackageService private ( .fold( errorMessage => Future.failed[T]( - errorFactories.malformedPackageId(request = request, message = errorMessage) + errorFactories.malformedPackageId(request = request, message = errorMessage)( + createContextualizedErrorLogger, + logger, + loggingContext, + ) ), packageId => block(packageId), ) @@ -110,6 +110,11 @@ private[apiserver] final class ApiPackageService private ( ) } + private def createContextualizedErrorLogger(implicit + loggingContext: LoggingContext + ): DamlContextualizedErrorLogger = + new DamlContextualizedErrorLogger(logger, loggingContext, None) + } private[platform] object ApiPackageService { From 659ec2b1da323957a69a7107256d3966dc8e225a Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Wed, 20 Oct 2021 09:39:25 +0200 Subject: [PATCH 8/9] 1 --- .../server/api/validation/ErrorFactoriesSpec.scala | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 1794e1dfd209..6e1a8c0179bc 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -49,6 +49,10 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope v1_details = Seq.empty, v2_code = Code.INVALID_ARGUMENT, v2_message = s"MALFORMED_PACKAGE_ID(8,$correlationId): message123", + v2_details = Seq[ErrorDetails.ErrorDetail]( + ErrorDetails.ErrorInfoDetail("MALFORMED_PACKAGE_ID"), + DefaultTraceIdRequestInfo, + ), ) } @@ -59,6 +63,11 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope v1_details = Seq.empty, v2_code = Code.NOT_FOUND, v2_message = s"PACKAGE_NOT_FOUND(11,$correlationId): Could not find package.", + v2_details = Seq[ErrorDetails.ErrorDetail]( + ErrorDetails.ErrorInfoDetail("PACKAGE_NOT_FOUND"), + DefaultTraceIdRequestInfo, + ErrorDetails.ResourceInfoDetail("PACKAGE_ID", "packageId123"), + ), ) } From dd55beb6b52883922ce649fbc3aa5e627d0e00ba Mon Sep 17 00:00:00 2001 From: Pawel Batko Date: Wed, 20 Oct 2021 10:33:50 +0200 Subject: [PATCH 9/9] TV review --- .../error/src/main/scala/com/daml/error/ErrorResource.scala | 3 --- .../scala/com/daml/error/definitions/LedgerApiErrors.scala | 6 ++---- .../platform/server/api/validation/ErrorFactoriesSpec.scala | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala b/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala index 07219f03ee2f..aa4dcc83444b 100644 --- a/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala +++ b/ledger/error/src/main/scala/com/daml/error/ErrorResource.scala @@ -35,7 +35,4 @@ object ErrorResource { object CommandId extends ErrorResource { def asString: String = "COMMAND_ID" } - object PackageId extends ErrorResource { - def asString: String = "PACKAGE_ID" - } } diff --git a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala index 84065c0528af..1d8181bcd5b2 100644 --- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala +++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala @@ -35,9 +35,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup { object ReadErrors extends ErrorGroup() { @Explanation("This rejection is given when a package id is malformed.") - @Resolution( - """Make sure the package id provided in the request has correct form.""".stripMargin - ) + @Resolution("Make sure the package id provided in the request has correct form.") // TODO error codes: Consider using `LedgerApiErrors.CommandValidation.InvalidArgument` object MalformedPackageId extends ErrorCode( @@ -68,7 +66,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup { ) { override def resources: Seq[(ErrorResource, String)] = { - super.resources :+ ((ErrorResource.PackageId, packageId)) + super.resources :+ ((ErrorResource.DalfPackage, packageId)) } } } diff --git a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala index 6e1a8c0179bc..5335a23debe6 100644 --- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala +++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/platform/server/api/validation/ErrorFactoriesSpec.scala @@ -66,7 +66,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope v2_details = Seq[ErrorDetails.ErrorDetail]( ErrorDetails.ErrorInfoDetail("PACKAGE_NOT_FOUND"), DefaultTraceIdRequestInfo, - ErrorDetails.ResourceInfoDetail("PACKAGE_ID", "packageId123"), + ErrorDetails.ResourceInfoDetail("PACKAGE", "packageId123"), ), ) }