[ledger-api] - Return FAILED_PRECONDITION gRPC status code for invalid deduplication duration [KVL-1170] (#11483)

CHANGELOG_BEGIN [ledger-api] - Return FAILED_PRECONDITION gRPC status code, instead of INVALID_ARGUMENT, for an invalid command deduplication duration CHANGELOG_END
2024-09-20 09:17:43 +03:00 · 2021-11-03 06:34:31 -07:00 · 2021-11-03 06:34:31 -07:00 · 9bb1d6443c
commit 9bb1d6443c
parent c098d75621
6 changed files with 100 additions and 28 deletions
--- a/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala
+++ b/ledger/error/src/main/scala/com/daml/error/definitions/LedgerApiErrors.scala
@ -284,6 +284,24 @@ object LedgerApiErrors extends LedgerApiErrorGroup {
          )
    }

+    @Explanation(
+      "This error is emitted when a submitted ledger API command specifies an invalid deduplication period."
+    )
+    @Resolution(
+      "Inspect the error message, adjust the value of the deduplication period or ask the participant operator to increase the maximum."
+    )
+    object InvalidDeduplicationPeriodField
+        extends ErrorCode(
+          id = "INVALID_DEDUPLICATION_PERIOD",
+          ErrorCategory.InvalidGivenCurrentSystemStateOther,
+        ) {
+      case class Reject(_reason: String)(implicit
+          loggingContext: ContextualizedErrorLogger
+      ) extends LoggingTransactionErrorImpl(
+            cause = s"The submitted command had an invalid deduplication period: ${_reason}"
+          )
+    }
+
  }

  object CommandPreparation extends ErrorGroup {
--- 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
@ -3,6 +3,8 @@

 package com.daml.platform.server.api.validation

+import java.sql.{SQLNonTransientException, SQLTransientException}
+
 import com.daml.error.ErrorCode.ApiException
 import com.daml.error.definitions.{IndexErrors, LedgerApiErrors}
 import com.daml.error.{ContextualizedErrorLogger, ErrorCodesVersionSwitcher}
@ -22,8 +24,6 @@ import io.grpc.protobuf.StatusProto
 import io.grpc.{Metadata, StatusRuntimeException}
 import scalaz.syntax.tag._

-import java.sql.{SQLNonTransientException, SQLTransientException}
-
 class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitcher) {
  def sqlTransientException(exception: SQLTransientException)(implicit
      contextualizedErrorLogger: ContextualizedErrorLogger
@ -227,6 +227,18 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
        .asGrpcError,
    )

+  def invalidDeduplicationDuration(
+      fieldName: String,
+      message: String,
+      definiteAnswer: Option[Boolean],
+  )(implicit contextualizedErrorLogger: ContextualizedErrorLogger): StatusRuntimeException =
+    errorCodesVersionSwitcher.choose(
+      legacyInvalidField(fieldName, message, definiteAnswer),
+      LedgerApiErrors.CommandValidation.InvalidDeduplicationPeriodField
+        .Reject(message)
+        .asGrpcError,
+    )
+
  /** @param fieldName An invalid field's name.
    * @param message A status' message.
    * @param definiteAnswer A flag that says whether it is a definite answer. Provided only in the context of command deduplication.
@ -238,19 +250,30 @@ class ErrorFactories private (errorCodesVersionSwitcher: ErrorCodesVersionSwitch
      definiteAnswer: Option[Boolean],
  )(implicit contextualizedErrorLogger: ContextualizedErrorLogger): StatusRuntimeException =
    errorCodesVersionSwitcher.choose(
-      v1 = {
-        val statusBuilder = Status
-          .newBuilder()
-          .setCode(Code.INVALID_ARGUMENT.value())
-          .setMessage(s"Invalid field $fieldName: $message")
-        addDefiniteAnswerDetails(definiteAnswer, statusBuilder)
-        grpcError(statusBuilder.build())
-      },
-      v2 = LedgerApiErrors.CommandValidation.InvalidField
-        .Reject(s"Invalid field $fieldName: $message")
-        .asGrpcError,
+      v1 = legacyInvalidField(fieldName, message, definiteAnswer),
+      v2 = ledgerCommandValidationInvalidField(fieldName, message).asGrpcError,
    )

+  private def ledgerCommandValidationInvalidField(fieldName: String, message: String)(implicit
+      contextualizedErrorLogger: ContextualizedErrorLogger
+  ): LedgerApiErrors.CommandValidation.InvalidField.Reject = {
+    LedgerApiErrors.CommandValidation.InvalidField
+      .Reject(s"Invalid field $fieldName: $message")
+  }
+
+  private def legacyInvalidField(
+      fieldName: String,
+      message: String,
+      definiteAnswer: Option[Boolean],
+  ): StatusRuntimeException = {
+    val statusBuilder = Status
+      .newBuilder()
+      .setCode(Code.INVALID_ARGUMENT.value())
+      .setMessage(s"Invalid field $fieldName: $message")
+    addDefiniteAnswerDetails(definiteAnswer, statusBuilder)
+    grpcError(statusBuilder.build())
+  }
+
  def offsetOutOfRange(description: String)(implicit
      contextualizedErrorLogger: ContextualizedErrorLogger
  ): StatusRuntimeException =
--- a/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala
+++ b/ledger/ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/validation/FieldValidations.scala
@ -3,6 +3,9 @@

 package com.daml.platform.server.api.validation

+import java.time.Duration
+
+import com.daml.api.util.DurationConversion
 import com.daml.error.ContextualizedErrorLogger
 import com.daml.ledger.api.domain.LedgerId
 import com.daml.ledger.api.v1.commands.Commands.{DeduplicationPeriod => DeduplicationPeriodProto}
@ -14,8 +17,6 @@ import com.daml.lf.value.Value.ContractId
 import com.google.protobuf.duration.{Duration => DurationProto}
 import io.grpc.StatusRuntimeException

-import java.time.Duration
-
 // TODO error codes: Remove default usage of ErrorFactories
 class FieldValidations private (errorFactories: ErrorFactories) {
  import errorFactories._
@ -156,12 +157,18 @@ class FieldValidations private (errorFactories: ErrorFactories) {
        if (duration.isNegative)
          Left(invalidField(fieldName, "Duration must be positive", definiteAnswer = Some(false)))
        else if (duration.compareTo(maxDeduplicationTime) > 0)
-          Left(invalidField(fieldName, exceedsMaxDurationMessage, definiteAnswer = Some(false)))
+          Left(
+            invalidDeduplicationDuration(
+              fieldName,
+              exceedsMaxDurationMessage,
+              definiteAnswer = Some(false),
+            )
+          )
        else Right(duration)
      }

      def protoDurationToDurationPeriod(duration: DurationProto) = {
-        val result = Duration.ofSeconds(duration.seconds, duration.nanos.toLong)
+        val result = DurationConversion.fromProto(duration)
        validateDuration(
          result,
          s"The given deduplication duration of $result exceeds the maximum deduplication time of $maxDeduplicationTime",
--- a/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala
+++ b/ledger/ledger-api-common/src/test/suite/scala/com/digitalasset/ledger/api/validation/SubmitRequestValidatorTest.scala
@ -19,13 +19,13 @@ import com.daml.lf.value.{Value => Lf}
 import com.daml.platform.server.api.validation.{ErrorFactories, FieldValidations}
 import com.google.protobuf.duration.Duration
 import com.google.protobuf.empty.Empty
-import io.grpc.Status.Code.{INVALID_ARGUMENT, NOT_FOUND, UNAVAILABLE}
+import io.grpc.Status.Code.{FAILED_PRECONDITION, INVALID_ARGUMENT, NOT_FOUND, UNAVAILABLE}
 import org.mockito.MockitoSugar
 import org.scalatest.prop.TableDrivenPropertyChecks
 import org.scalatest.wordspec.AnyWordSpec
 import scalaz.syntax.tag._
-
 import java.time.{Instant, Duration => JDuration}
+
 import scala.annotation.nowarn

@nowarn("msg=deprecated")
@ -415,9 +415,9 @@ class SubmitRequestValidatorTest
            expectedDescriptionV1 =
              s"Invalid field deduplication_period: The given deduplication duration of ${java.time.Duration
                .ofSeconds(durationSecondsExceedingMax)} exceeds the maximum deduplication time of ${internal.maxDeduplicationDuration}",
-            expectedCodeV2 = INVALID_ARGUMENT,
-            expectedDescriptionV2 =
-              s"INVALID_FIELD(8,0): The submitted command has a field with invalid value: Invalid field deduplication_period: The given deduplication duration of PT24H1S exceeds the maximum deduplication time of ${internal.maxDeduplicationDuration}",
+            expectedCodeV2 = FAILED_PRECONDITION,
+            expectedDescriptionV2 = s"INVALID_DEDUPLICATION_PERIOD(9,0): The submitted command had an invalid deduplication period: The given deduplication duration of ${java.time.Duration
+              .ofSeconds(durationSecondsExceedingMax)} exceeds the maximum deduplication time of ${internal.maxDeduplicationDuration}",
          )
        }
      }
--- 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
@ -313,6 +313,23 @@ class ErrorFactoriesSpec
      }
    }

+    "return an invalid deduplication period error" in {
+      val errorDetailMessage = "message"
+      val field = "field"
+      assertVersionedError(_.invalidDeduplicationDuration(field, errorDetailMessage, None))(
+        v1_code = Code.INVALID_ARGUMENT,
+        v1_message = s"Invalid field $field: $errorDetailMessage",
+        v1_details = Seq.empty,
+        v2_code = Code.FAILED_PRECONDITION,
+        v2_message =
+          s"INVALID_DEDUPLICATION_PERIOD(9,trace-id): The submitted command had an invalid deduplication period: $errorDetailMessage",
+        v2_details = Seq[ErrorDetails.ErrorDetail](
+          ErrorDetails.ErrorInfoDetail("INVALID_DEDUPLICATION_PERIOD"),
+          DefaultTraceIdRequestInfo,
+        ),
+      )
+    }
+
    "return an invalidField error" in {
      val testCases = Table(
        ("definite answer", "expected details"),
@ -562,7 +579,7 @@ class ErrorFactoriesSpec
    status.getCode shouldBe expectedCode.value()
    status.getMessage shouldBe expectedMessage
    val details = status.getDetailsList.asScala.toSeq
-    val _ = ErrorDetails.from(details) should contain theSameElementsAs (expectedDetails)
+    val _ = ErrorDetails.from(details) should contain theSameElementsAs expectedDetails
    // TODO error codes: Assert logging
  }
 }
--- a/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/LedgerConfigurationServiceIT.scala
+++ b/ledger/ledger-api-test-tool/src/main/scala/com/daml/ledger/api/testtool/suites/LedgerConfigurationServiceIT.scala
@ -3,6 +3,8 @@

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

+import java.util.regex.Pattern
+
 import com.daml.error.definitions.LedgerApiErrors
 import com.daml.ledger.api.testtool.infrastructure.Allocation._
 import com.daml.ledger.api.testtool.infrastructure.Assertions._
@ -10,8 +12,6 @@ import com.daml.ledger.api.testtool.infrastructure.LedgerTestSuite
 import com.daml.ledger.test.model.Test.Dummy
 import io.grpc.Status

-import java.util.regex.Pattern
-
 class LedgerConfigurationServiceIT extends LedgerTestSuite {
  test("ConfigSucceeds", "Return a valid configuration for a valid request", allocate(NoParties))(
    implicit ec => { case Participants(Participant(ledger)) =>
@ -63,7 +63,7 @@ class LedgerConfigurationServiceIT extends LedgerTestSuite {

  test(
    "CSLSuccessIfMaxDeduplicationTimeExceeded",
-    "Submission returns INVALID_ARGUMENT if deduplication time is too big",
+    "Submission returns expected error codes if deduplication time is too big",
    allocate(SingleParty),
  )(implicit ec => { case Participants(Participant(ledger, party)) =>
    val request = ledger.submitRequest(party, Dummy(party).create.command)
@ -80,11 +80,18 @@ class LedgerConfigurationServiceIT extends LedgerTestSuite {
        )
        .mustFail("submitting a command with a deduplication time that is too big")
    } yield {
+      val expectedCode =
+        if (ledger.features.selfServiceErrorCodes) Status.Code.FAILED_PRECONDITION
+        else Status.Code.INVALID_ARGUMENT
+      val expectedError =
+        if (ledger.features.selfServiceErrorCodes)
+          LedgerApiErrors.CommandValidation.InvalidDeduplicationPeriodField
+        else LedgerApiErrors.CommandValidation.InvalidField
      assertGrpcErrorRegex(
        ledger,
        failure,
-        Status.Code.INVALID_ARGUMENT,
-        LedgerApiErrors.CommandValidation.InvalidField,
+        expectedCode,
+        expectedError,
        Some(
          Pattern.compile(
            "The given deduplication duration .+ exceeds the maximum deduplication time of .+"