Skip to content

Commit

Permalink
[DPP-686][Self-service error codes] Removing default error factories (#…
Browse files Browse the repository at this point in the history
…11403)

CHANGELOG_BEGIN
CHANGELOG_END
  • Loading branch information
pbatko-da authored Nov 3, 2021
1 parent bb37eef commit 8a9f15b
Show file tree
Hide file tree
Showing 48 changed files with 342 additions and 133 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@ object LedgerApiErrors extends LedgerApiErrorGroup {
cause = message
)
}

@Explanation(
"This rejection is given when a request might not been processed and a time-out was reached."
)
@Resolution(
"Retry for transient problems. If non-transient contact the operator as the time-out limit might be to short."
)
object RequestTimeOut
extends ErrorCode(
id = "REQUEST_TIME_OUT",
ErrorCategory.DeadlineExceededRequestStateUnknown,
) {
case class Reject(message: String, override val definiteAnswer: Boolean)(implicit
loggingContext: ContextualizedErrorLogger
) extends LoggingTransactionErrorImpl(
cause = message
)
}
}

object ReadErrors extends ErrorGroup() {
Expand Down Expand Up @@ -121,7 +139,7 @@ object LedgerApiErrors extends LedgerApiErrorGroup {
"This rejection is given when a read request uses an offset beyond the current ledger end."
)
@Resolution("Use an offset that is before the ledger end.")
object RequestedOffsetAfterLedgerEnd
object RequestedOffsetOutOfRange
extends ErrorCode(
id = "REQUESTED_OFFSET_OUT_OF_RANGE",
ErrorCategory.InvalidGivenCurrentSystemStateSeekAfterEnd,
Expand Down Expand Up @@ -579,6 +597,13 @@ object LedgerApiErrors extends LedgerApiErrorGroup {
ErrorCategory.SystemInternalAssumptionViolated,
) {

// TODO error codes: This is an internal error not related to the interpreter
case class CommandTrackerInternalError(
message: String
)(implicit
loggingContext: ContextualizedErrorLogger
) extends LoggingTransactionErrorImpl(cause = message)

case class PackageSelfConsistency(
err: LfError.Package.SelfConsistency
)(implicit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,16 @@

package com.daml.error.definitions

import com.daml.error.{BaseError, ErrorCause, ErrorCode, ContextualizedErrorLogger}
import com.daml.error.{BaseError, ContextualizedErrorLogger, ErrorCause, ErrorCode}
import com.daml.ledger.participant.state
import com.daml.lf.engine.Error.{Interpretation, Package, Preprocessing, Validation}
import com.daml.lf.engine.{Error => LfError}
import com.daml.lf.interpretation.{Error => LfInterpretationError}
import io.grpc.Status.Code
import io.grpc.StatusRuntimeException
import io.grpc.protobuf.StatusProto

import scala.util.{Failure, Success, Try}
import com.daml.lf.engine.{Error => LfError}
import com.daml.lf.interpretation.{Error => LfInterpretationError}

class RejectionGenerators(conformanceMode: Boolean) {
private val adjustErrors = Map(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class LedgerOffsetValidator(errorFactories: ErrorFactories) {

private val fieldValidations = FieldValidations(errorFactories)

import errorFactories.{invalidArgument, missingField, offsetAfterLedgerEnd}
import errorFactories.{invalidArgument, missingField, offsetOutOfRange}
import fieldValidations.requireLedgerString

def validateOptional(
Expand Down Expand Up @@ -59,7 +59,7 @@ class LedgerOffsetValidator(errorFactories: ErrorFactories) {
ledgerOffset match {
case abs: domain.LedgerOffset.Absolute if abs > ledgerEnd =>
Left(
offsetAfterLedgerEnd(
offsetOutOfRange(
s"$offsetType offset ${abs.value} is after ledger end ${ledgerEnd.value}"
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,8 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
.asGrpcError,
)

// TODO error codes: Reconcile with com.daml.platform.server.api.validation.ErrorFactories.offsetAfterLedgerEnd
def readingOffsetAfterLedgerEnd_was_invalidArgument(
// TODO error codes: Reconcile with com.daml.platform.server.api.validation.ErrorFactories.offsetOutOfRange
def offsetOutOfRange_was_invalidArgument(
definiteAnswer: Option[Boolean]
)(message: String)(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
Expand All @@ -206,7 +206,7 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
v1 = {
invalidArgumentV1(definiteAnswer, message)
},
v2 = LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd
v2 = LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange
.Reject(message)
.asGrpcError,
)
Expand Down Expand Up @@ -251,7 +251,7 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
.asGrpcError,
)

def offsetAfterLedgerEnd(description: String)(implicit
def offsetOutOfRange(description: String)(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
): StatusRuntimeException =
// TODO error codes: Pass the offsets as arguments to this method and build the description here
Expand All @@ -263,7 +263,7 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
.setMessage(description)
.build()
),
v2 = LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd.Reject(description).asGrpcError,
v2 = LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange.Reject(description).asGrpcError,
)

/** @param message A status' message.
Expand All @@ -281,6 +281,21 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
grpcError(statusBuilder.build())
}

def isTimeoutUnknown_wasAborted(message: String, definiteAnswer: Option[Boolean])(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
): StatusRuntimeException = {
errorCodesVersionSwitcher.choose(
v1 = aborted(message, definiteAnswer),
v2 = LedgerApiErrors.WriteErrors.RequestTimeOut
.Reject(
message,
// TODO error codes: How to handle None definiteAnswer?
definiteAnswer.getOrElse(false),
)
.asGrpcError,
)
}

def packageUploadRejected(message: String, definiteAnswer: Option[Boolean])(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
): StatusRuntimeException = {
Expand Down Expand Up @@ -414,6 +429,20 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
v2 = LedgerApiErrors.ServiceNotRunning.Reject().asGrpcError,
)

def trackerFailure(msg: String)(implicit
contextualizedErrorLogger: ContextualizedErrorLogger
): StatusRuntimeException =
errorCodesVersionSwitcher.choose(
v1 = {
val builder = Status
.newBuilder()
.setCode(Code.INTERNAL.value())
.setMessage(msg)
grpcError(builder.build())
},
v2 = LedgerApiErrors.InternalError.CommandTrackerInternalError(msg).asGrpcError,
)

/** Transforms Protobuf [[Status]] objects, possibly including metadata packed as [[ErrorInfo]] objects,
* into exceptions with metadata in the trailers.
*
Expand All @@ -439,11 +468,7 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
}
}

/** Object exposing the legacy error factories.
* TODO error codes: Remove default implementation once all Ledger API services
* output versioned error codes.
*/
object ErrorFactories extends ErrorFactories(new ErrorCodesVersionSwitcher(false)) {
object ErrorFactories {
val SelfServiceErrorCodeFactories: ErrorFactories = ErrorFactories(
new ErrorCodesVersionSwitcher(enableSelfServiceErrorCodes = true)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,19 @@ import com.google.rpc._
import io.grpc.Status.Code
import io.grpc.StatusRuntimeException
import io.grpc.protobuf.StatusProto
import org.mockito.MockitoSugar
import org.scalatest.matchers.should.Matchers
import org.scalatest.prop.TableDrivenPropertyChecks
import org.scalatest.wordspec.AnyWordSpec

import java.sql.{SQLNonTransientException, SQLTransientException}
import scala.jdk.CollectionConverters._

class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPropertyChecks {
class ErrorFactoriesSpec
extends AnyWordSpec
with Matchers
with TableDrivenPropertyChecks
with MockitoSugar {
private val correlationId = "trace-id"
private val logger = ContextualizedLogger.get(getClass)
private val loggingContext = LoggingContext.ForTesting
Expand All @@ -36,6 +41,8 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
private val DefaultTraceIdRequestInfo: ErrorDetails.RequestInfoDetail =
ErrorDetails.RequestInfoDetail("trace-id")

val tested = ErrorFactories(mock[ErrorCodesVersionSwitcher])

"ErrorFactories" should {
"return sqlTransientException" in {
val failureReason = "some db transient failure"
Expand Down Expand Up @@ -181,6 +188,23 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
)
}

"return a isTimeoutUnknown_wasAborted error" in {
assertVersionedError(
_.isTimeoutUnknown_wasAborted("message123", definiteAnswer = Some(false))
)(
v1_code = Code.ABORTED,
v1_message = "message123",
v1_details = Seq(definiteAnswers(false)),
v2_code = Code.DEADLINE_EXCEEDED,
v2_message = s"REQUEST_TIME_OUT(3,trace-id): message123",
v2_details = Seq[ErrorDetails.ErrorDetail](
ErrorDetails.ErrorInfoDetail("REQUEST_TIME_OUT"),
DefaultTraceIdRequestInfo,
ErrorDetails.RetryInfoDetail(1),
),
)
}

"return a nonHexOffset error" in {
assertVersionedError(
_.nonHexOffset(None)(
Expand All @@ -202,8 +226,8 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
)
}

"return a readingOffsetAfterLedgerEnd error" in {
assertVersionedError(_.readingOffsetAfterLedgerEnd_was_invalidArgument(None)("message123"))(
"return a offsetOutOfRange_was_invalidArgument error" in {
assertVersionedError(_.offsetOutOfRange_was_invalidArgument(None)("message123"))(
v1_code = Code.INVALID_ARGUMENT,
v1_message = "Invalid argument: message123",
v1_details = Seq.empty,
Expand Down Expand Up @@ -281,7 +305,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
)

forEvery(testCases) { (definiteAnswer, expectedDetails) =>
val exception = aborted("my message", definiteAnswer)
val exception = tested.aborted("my message", definiteAnswer)
val status = StatusProto.fromThrowable(exception)
status.getCode shouldBe Code.ABORTED.value()
status.getMessage shouldBe "my message"
Expand Down Expand Up @@ -338,7 +362,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
}

"fail on creating a ledgerIdMismatch error due to a wrong definite answer" in {
an[IllegalArgumentException] should be thrownBy ledgerIdMismatch(
an[IllegalArgumentException] should be thrownBy tested.ledgerIdMismatch(
LedgerId("expected"),
LedgerId("received"),
definiteAnswer = Some(true),
Expand All @@ -359,8 +383,23 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope
)
}

"return an offsetAfterLedgerEnd error" in {
assertVersionedError(_.offsetAfterLedgerEnd("my message"))(
"return a trackerFailure error" in {
assertVersionedError(_.trackerFailure("message123"))(
v1_code = Code.INTERNAL,
v1_message = "message123",
v1_details = Seq.empty,
v2_code = Code.INTERNAL,
v2_message =
s"An error occurred. Please contact the operator and inquire about the request trace-id",
v2_details = Seq[ErrorDetails.ErrorDetail](
ErrorDetails.ErrorInfoDetail("LEDGER_API_INTERNAL_ERROR"),
DefaultTraceIdRequestInfo,
),
)
}

"return an offsetOutOfRange error" in {
assertVersionedError(_.offsetOutOfRange("my message"))(
v1_code = Code.OUT_OF_RANGE,
v1_message = "my message",
v1_details = Seq.empty,
Expand Down Expand Up @@ -482,7 +521,7 @@ class ErrorFactoriesSpec extends AnyWordSpec with Matchers with TableDrivenPrope

"should create an ApiException without the stack trace" in {
val status = Status.newBuilder().setCode(Code.INTERNAL.value()).build()
val exception = grpcError(status)
val exception = tested.grpcError(status)
exception.getStackTrace shouldBe Array.empty
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ final class CommandSubmissionCompletionIT extends LedgerTestSuite {
ledger,
failure,
Status.Code.OUT_OF_RANGE,
LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd,
LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange,
Some("is after ledger end"),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ class ParticipantPruningIT extends LedgerTestSuite {
participant,
cannotPruneOffsetBeyondEnd,
Status.Code.INVALID_ARGUMENT,
LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd,
LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange,
Some("prune_up_to needs to be before ledger end"),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class TransactionServiceStreamsIT extends LedgerTestSuite {
ledger,
failure,
Status.Code.OUT_OF_RANGE,
LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd,
LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange,
Some("is after ledger end"),
)
}
Expand All @@ -106,7 +106,7 @@ class TransactionServiceStreamsIT extends LedgerTestSuite {
ledger,
failure,
Status.Code.OUT_OF_RANGE,
LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd,
LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange,
Some("is after ledger end"),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class TransactionServiceValidationIT extends LedgerTestSuite {
ledger,
failure,
Status.Code.INVALID_ARGUMENT,
LedgerApiErrors.ReadErrors.RequestedOffsetAfterLedgerEnd,
LedgerApiErrors.ReadErrors.RequestedOffsetOutOfRange,
Some("is before Begin offset"),
)
}
Expand Down
4 changes: 3 additions & 1 deletion ledger/participant-integration-api/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ compile_deps = [
"//libs-scala/scala-utils",
"//libs-scala/timer-utils",
"//libs-scala/nameof",
"@maven//:com_google_api_grpc_proto_google_common_protos",
"@maven//:com_google_guava_guava",
"@maven//:com_zaxxer_HikariCP",
"@maven//:io_dropwizard_metrics_metrics_core",
Expand Down Expand Up @@ -162,6 +161,7 @@ da_scala_library(
"@maven//:org_scalatest_scalatest_flatspec",
"@maven//:org_scalatest_scalatest_matchers_core",
"@maven//:org_scalatest_scalatest_shouldmatchers",
"@maven//:org_mockito_mockito_scala",
],
scala_runtime_deps = [
"@maven//:com_typesafe_akka_akka_slf4j",
Expand Down Expand Up @@ -211,6 +211,7 @@ da_scala_library(
"@maven//:io_netty_netty_common",
"@maven//:io_netty_netty_handler",
"@maven//:io_netty_netty_transport",
"@maven//:org_mockito_mockito_core",
"@maven//:org_scalatest_scalatest_compatible",
],
)
Expand Down Expand Up @@ -353,6 +354,7 @@ da_scala_test_suite(
"@maven//:org_scalatest_scalatest_matchers_core",
"@maven//:org_scalatest_scalatest_shouldmatchers",
"@maven//:org_scalatest_scalatest_wordspec",
"@maven//:org_mockito_mockito_scala",
],
tags = [] if oracle_testing else ["manual"],
runtime_deps = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ final class StandaloneApiServer(
enableMutableContractStateCache = config.enableMutableContractStateCache,
maxTransactionsInMemoryFanOutBufferSize = config.maxTransactionsInMemoryFanOutBufferSize,
enableInMemoryFanOutForLedgerApi = config.enableInMemoryFanOutForLedgerApi,
enableSelfServiceErrorCodes = config.enableSelfServiceErrorCodes,
)
.map(index => new SpannedIndexService(new TimedIndexService(index, metrics)))
errorCodesVersionSwitcher = new ErrorCodesVersionSwitcher(
Expand Down
Loading

0 comments on commit 8a9f15b

Please sign in to comment.