Skip to content

Commit

Permalink
[Self-service error codes] Use error validators in ApiPackageManageme…
Browse files Browse the repository at this point in the history
…ntService (#11575)

* Use error validators in ApiPackageManagementService

CHANGELOG_BEGIN
CHANGELOG_END

* Exclude PackageManagementServiceIT from compatibility tests after 1.17

CHANGELOG_BEGIN
CHANGELOG_END
  • Loading branch information
tudor-da authored Nov 9, 2021
1 parent 6372d41 commit 92dfcde
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 60 deletions.
12 changes: 12 additions & 0 deletions compatibility/bazel_tools/testing.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,18 @@ excluded_test_tool_tests = [
},
],
},
{
# Self-service error code assertions adapted
"end": "1.18.0-snapshot.20211102.8257.1",
"platform_ranges": [
{
"start": "1.18.0-snapshot.20211102.8257.1",
"exclusions": [
"PackageManagementServiceIT",
],
},
],
},
]

def in_range(version, range):
Expand Down
2 changes: 1 addition & 1 deletion language-support/hs/bindings/test/DA/Ledger/Tests.hs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ tUploadDarFileBad withSandbox = testCase "tUploadDarFileBad" $ run withSandbox $
lid <- getLedgerIdentity
let bytes = BS.fromString "not-the-bytes-for-a-darfile"
Left err <- uploadDarFileGetPid lid bytes
liftIO $ assertTextContains err "Invalid DAR: package-upload"
liftIO $ assertTextContains err "Dar file is corrupt"

tUploadDarFileGood :: SandboxTest
tUploadDarFileGood withSandbox = testCase "tUploadDarFileGood" $ run withSandbox $ \_darMetadata testId -> do
Expand Down
2 changes: 1 addition & 1 deletion ledger/error/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ da_scala_library(
name = "error",
srcs = glob(["src/main/scala/**/*.scala"]),
scala_deps = [
"@maven//:org_typelevel_cats_core",
"@maven//:io_spray_spray_json",
],
tags = ["maven_coordinates=com.daml:error:__VERSION__"],
Expand All @@ -21,6 +20,7 @@ da_scala_library(
},
visibility = ["//visibility:public"],
deps = [
"//daml-lf/archive:daml_lf_archive_reader",
"//daml-lf/data",
"//daml-lf/engine",
"//daml-lf/language",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,22 @@

package com.daml.error.definitions

import cats.data.EitherT
import com.daml.error._
import com.daml.error.{
BaseError,
ContextualizedErrorLogger,
ErrorCategory,
ErrorCode,
ErrorGroup,
Explanation,
Resolution,
}
import com.daml.error.definitions.ErrorGroups.ParticipantErrorGroup.PackageServiceErrorGroup
import com.daml.lf.archive.{Error => LfArchiveError}
import com.daml.lf.data.Ref
import com.daml.lf.data.Ref.PackageId
import com.daml.lf.engine.Error
import com.daml.lf.{VersionRange, language, validation}

import scala.concurrent.{ExecutionContext, Future}

trait PackageServiceError extends BaseError
object PackageServiceError extends PackageServiceErrorGroup {

Expand Down Expand Up @@ -140,24 +146,45 @@ object PackageServiceError extends PackageServiceErrorGroup {
}

object Validation {

def fromDamlLfEnginePackageError(err: Either[Error.Package.Error, Unit])(implicit
executionContext: ExecutionContext,
loggingContext: ContextualizedErrorLogger,
): EitherT[Future, PackageServiceError, Unit] =
EitherT.fromEither[Future](err.left.map {
case Error.Package.Internal(nameOfFunc, msg) =>
PackageServiceError.InternalError.Validation(nameOfFunc, msg)
case Error.Package.Validation(validationError) =>
ValidationError.Error(validationError)
case Error.Package.MissingPackage(packageId, _) =>
PackageServiceError.InternalError.Error(Set(packageId))
case Error.Package
.AllowedLanguageVersion(packageId, languageVersion, allowedLanguageVersions) =>
AllowedLanguageMismatchError(packageId, languageVersion, allowedLanguageVersions)
case Error.Package.SelfConsistency(packageIds, missingDependencies) =>
SelfConsistency.Error(packageIds, missingDependencies)
})
def handleLfArchiveError(
lfArchiveError: LfArchiveError
)(implicit contextualizedErrorLogger: ContextualizedErrorLogger): BaseError.Impl =
lfArchiveError match {
case LfArchiveError.InvalidDar(entries, cause) =>
PackageServiceError.Reading.InvalidDar
.Error(entries.entries.keys.toSeq, cause)
case LfArchiveError.InvalidZipEntry(name, entries) =>
PackageServiceError.Reading.InvalidZipEntry
.Error(name, entries.entries.keys.toSeq)
case LfArchiveError.InvalidLegacyDar(entries) =>
PackageServiceError.Reading.InvalidLegacyDar.Error(entries.entries.keys.toSeq)
case LfArchiveError.ZipBomb =>
PackageServiceError.Reading.ZipBomb.Error(LfArchiveError.ZipBomb.getMessage)
case e: LfArchiveError =>
PackageServiceError.Reading.ParseError.Error(e.msg)
case e =>
PackageServiceError.InternalError.Unhandled(e)
}

def handleLfEnginePackageError(err: Error.Package.Error)(implicit
loggingContext: ContextualizedErrorLogger
): BaseError.Impl = err match {
case Error.Package.Internal(nameOfFunc, msg) =>
PackageServiceError.InternalError.Validation(nameOfFunc, msg)
case Error.Package.Validation(validationError) =>
ValidationError.Error(validationError)
case Error.Package.MissingPackage(packageId, _) =>
PackageServiceError.InternalError.Error(Set(packageId))
case Error.Package
.AllowedLanguageVersion(packageId, languageVersion, allowedLanguageVersions) =>
AllowedLanguageMismatchError(
packageId,
languageVersion,
allowedLanguageVersions,
)
case Error.Package.SelfConsistency(packageIds, missingDependencies) =>
SelfConsistency.Error(packageIds, missingDependencies)
}

@Explanation("""This error indicates that the validation of the uploaded dar failed.""")
@Resolution("Inspect the error message and contact support.")
Expand Down Expand Up @@ -206,7 +233,5 @@ object PackageServiceError extends PackageServiceErrorGroup {
)
with PackageServiceError
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

package com.daml.ledger.api.testtool.suites

import com.daml.error.definitions.LedgerApiErrors
import com.daml.error.definitions.PackageServiceError
import com.daml.ledger.api.testtool.infrastructure.Allocation._
import com.daml.ledger.api.testtool.infrastructure.Assertions._
import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite
Expand All @@ -13,6 +13,7 @@ import com.daml.ledger.test.package_management.PackageManagementTest.PackageMana
import com.google.protobuf.ByteString
import io.grpc.Status

import java.util.regex.Pattern
import scala.collection.compat._
import scala.concurrent.{ExecutionContext, Future}

Expand All @@ -38,12 +39,12 @@ final class PackageManagementServiceIT extends LedgerTestSuite {
for {
failure <- ledger.uploadDarFile(ByteString.EMPTY).mustFail("uploading an empty package")
} yield {
assertGrpcError(
assertGrpcErrorRegex(
ledger,
failure,
Status.Code.INVALID_ARGUMENT,
LedgerApiErrors.CommandValidation.InvalidArgument,
Some("Invalid DAR: package-upload"),
PackageServiceError.Reading.InvalidDar,
Some(Pattern.compile("Invalid DAR: package-upload|Dar file is corrupt")),
)
}
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,13 @@ import akka.stream.Materializer
import akka.stream.scaladsl.Source
import com.daml.api.util.TimestampConversion
import com.daml.daml_lf_dev.DamlLf.Archive
import com.daml.error.ErrorCodesVersionSwitcher
import com.daml.error.DamlContextualizedErrorLogger
import com.daml.error.definitions.PackageServiceError.Validation
import com.daml.error.{
BaseError,
ContextualizedErrorLogger,
DamlContextualizedErrorLogger,
ErrorCodesVersionSwitcher,
}
import com.daml.ledger.api.domain.{LedgerOffset, PackageEntry}
import com.daml.ledger.api.v1.admin.package_management_service.PackageManagementServiceGrpc.PackageManagementService
import com.daml.ledger.api.v1.admin.package_management_service._
Expand All @@ -28,7 +33,6 @@ import com.daml.logging.{ContextualizedLogger, LoggingContext}
import com.daml.platform.api.grpc.GrpcApiService
import com.daml.platform.apiserver.services.admin.ApiPackageManagementService._
import com.daml.platform.apiserver.services.logging
import com.daml.platform.server.api.ValidationLogger
import com.daml.platform.server.api.validation.ErrorFactories
import com.daml.telemetry.{DefaultTelemetry, TelemetryContext}
import io.grpc.{ServerServiceDefinition, StatusRuntimeException}
Expand Down Expand Up @@ -94,56 +98,60 @@ private[apiserver] final class ApiPackageManagementService private (
.andThen(logger.logErrorsOnCall[ListKnownPackagesResponse])
}

private def decodeAndValidate(stream: ZipInputStream): Try[Dar[Archive]] =
private def decodeAndValidate(
stream: ZipInputStream
)(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
): Try[Dar[Archive]] =
for {
dar <- darReader.readArchive("package-upload", stream).toTry
packages <- dar.all.traverse(Decode.decodeArchive(_)).toTry
dar <- darReader
.readArchive("package-upload", stream)
.loggingValidation(Validation.handleLfArchiveError)
packages <- dar.all
.traverse(Decode.decodeArchive(_))
.loggingValidation(Validation.handleLfArchiveError)
_ <- engine
.validatePackages(packages.toMap)
.left
.map(e => new IllegalArgumentException(e.message))
.toTry
.loggingValidation(Validation.handleLfEnginePackageError)
} yield dar

override def uploadDarFile(request: UploadDarFileRequest): Future[UploadDarFileResponse] =
withEnrichedLoggingContext(logging.submissionId(request.submissionId)) {
implicit loggingContext =>
logger.info("Uploading DAR file")
val submissionId = submissionIdGenerator(request.submissionId)
val darInputStream = new ZipInputStream(request.darFile.newInput())

implicit val telemetryContext: TelemetryContext =
DefaultTelemetry.contextFromGrpcThreadLocalContext()

val submissionId = submissionIdGenerator(request.submissionId)
val darInputStream = new ZipInputStream(request.darFile.newInput())
implicit val contextualizedErrorLogger: ContextualizedErrorLogger =
new DamlContextualizedErrorLogger(
logger,
loggingContext,
Some(submissionId),
)

val response = for {
dar <- decodeAndValidate(darInputStream).fold(
err =>
Future.failed(
ValidationLogger
.logFailureWithContext(
request,
errorFactories.invalidArgument(None)(err.getMessage)(
new DamlContextualizedErrorLogger(
logger,
loggingContext,
Some(submissionId),
)
),
)
),
Future.successful,
)
dar <- Future.fromTry(decodeAndValidate(darInputStream))
_ <- synchronousResponse.submitAndWait(submissionId, dar)
} yield {
for (archive <- dar.all) {
logger.info(s"Package ${archive.getHash} successfully uploaded")
}
UploadDarFileResponse()
}

response.andThen(logger.logErrorsOnCall[UploadDarFileResponse])
}

private implicit class ErrorValidations[E, R](result: Either[E, R]) {
def loggingValidation(validate: E => BaseError.Impl): Try[R] =
result.left.map { err =>
val baseError = validate(err)
baseError.log()
baseError.asGrpcError
}.toTry
}
}

private[apiserver] object ApiPackageManagementService {
Expand Down Expand Up @@ -212,5 +220,4 @@ private[apiserver] object ApiPackageManagementService {
)
}
}

}

0 comments on commit 92dfcde

Please sign in to comment.