[DPP-701] Maybe missed error code conversions #11510

pbatko-da · 2021-11-02T14:41:28Z

Pull Request Checklist

Read and understand the contribution guidelines
Include appropriate tests
Set a descriptive title and thorough description
Add a reference to the issue this PR will solve, if appropriate
Include changelog additions in one or more commit message bodies between the CHANGELOG_BEGIN and CHANGELOG_END tags
Normal production system change, include purpose of change in description
If you mean to change the status of a component, please make sure you keep the Component Status page up to date.

NOTE: CI is not automatically run on non-members pull-requests for security
reasons. The reviewer will have to comment with /AzurePipelines run to
trigger the build.

tudor-da · 2021-11-03T08:41:02Z

...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/ScenarioServiceMain.scala

@@ -72,6 +72,7 @@ object ScenarioServiceMain extends App {

 object ScenarioService {
  private def notFoundContextError(id: Long): StatusRuntimeException =
+  // TODO error codes: Adapt V2 ?


Language team

(Here and other places helpfully annotated with "Language team" by Tudor)

@remyhaemmerle-da We spotted these places that could need adapting to self service error codes (aka V2). Are you aware of these in the Language team (if there're legitimate)?

scenarios will die completely in Daml 2.0, safe to ignore.

tudor-da · 2021-11-03T08:41:07Z

...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/ScenarioServiceMain.scala

@@ -292,8 +293,10 @@ class ScenarioService(implicit

        } catch {
          case e: archive.Error =>
+            // TODO error codes: Adapt V2 ?


Language team

tudor-da · 2021-11-03T08:41:11Z

...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/ScenarioServiceMain.scala

            respObs.onError(Status.INVALID_ARGUMENT.withDescription(e.msg).asRuntimeException())
          case NonFatal(e) =>
+            // TODO error codes: Adapt V2 ?


Language team

tudor-da · 2021-11-03T08:41:31Z

...c/main/scala/com/digitalasset/daml/lf/engine/script/ledgerinteraction/JsonLedgerClient.scala

@@ -439,6 +439,7 @@ class JsonLedgerClient(
        // TODO (MK) Using a grpc exception here doesn’t make that much sense.
        // We should refactor this to provide something more general.
        Future.successful(
+          // TODO error codes: Adapt V2 ?


Language or runtime team?

@stefanobaghino-da @remyhaemmerle-da We spotted this place that could need adapting to self service error codes (aka V2). Are you aware of this in the Language or Runtime team (if it's legitimate)?

Language or runtime team?

Language is responsible from this part of the script. @stefanobaghino-da you can ignore this one.

I think you can ignore this one.
cc @cocreature

that’s fine, just keep it

...ings-rxjava/src/test/scala/com/daml/ledger/rxjava/grpc/helpers/TransactionsServiceImpl.scala

tudor-da · 2021-11-03T08:42:03Z

ledger/error/src/main/scala/com/daml/error/definitions/RejectionGenerators.scala

@@ -191,6 +191,7 @@ class RejectionGenerators(conformanceMode: Boolean) {
  // TODO error codes: This converter is deprecated and should be removed
  //                   Instead of using this, construct proper validation errors in callers of this method
  //                   and only convert to StatusRuntimeExceptions when dispatched (e.g. in ApiSubmissionService)
+  @deprecated("not used in daml sdk")


tudor-da · 2021-11-03T09:29:20Z

...tate/kvutils/src/main/scala/com/daml/ledger/validator/batch/BatchedValidatingCommitter.scala

@@ -78,6 +78,7 @@ class BatchedValidatingCommitter[LogResult](
          Future.successful(SubmissionResult.Acknowledged)
        case Failure(exception) =>
          Future.successful(
+            // TODO error codes: Adapt V2 ?


KV Committer team

tudor-da · 2021-11-03T09:31:34Z

...ate/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/api/BatchingQueue.scala

@@ -121,13 +121,15 @@ case class DefaultBatchingQueue(
                )
              )
            case f: QueueOfferResult.Failure =>
+              // TODO error codes: Adapt V2 ?


KV committer team

(Here and other places helpfully annotated with "KV committer team" by Tudor)

@fabiotudone-da We spotted these places that could need adapting to self service error codes (aka V2). Are you aware of these in the Committer team (if there're legitimate)?

tudor-da · 2021-11-03T09:31:41Z

...ate/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/api/BatchingQueue.scala

              SubmissionResult.SynchronousError(
                Status(
                  Code.INTERNAL.value,
                  f.toString,
                )
              )
            case QueueOfferResult.QueueClosed =>
+              // TODO error codes: Adapt V2 ?


KV committer team

tudor-da · 2021-11-03T09:31:45Z

...participant-state/kvutils/src/main/scala/com/daml/ledger/validator/ValidatingCommitter.scala

@@ -65,13 +65,15 @@ class ValidatingCommitter[LogResult](
            postCommit(value)
            SubmissionResult.Acknowledged
          case Left(MissingInputState(keys)) =>
+            // TODO error codes: Adapt V2 ?


KV committer team

tudor-da · 2021-11-03T09:31:49Z

...participant-state/kvutils/src/main/scala/com/daml/ledger/validator/ValidatingCommitter.scala

            SubmissionResult.SynchronousError(
              Status(
                Code.INTERNAL.value,
                s"Missing input state: ${keys.map(_.bytes.asScala.map("%02x".format(_)).mkString).mkString(", ")}",
              )
            )
          case Left(ValidationError(reason)) =>
+            // TODO error codes: Adapt V2 ?


KV committer team

.../sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/inmemory/InMemoryLedger.scala

tudor-da · 2021-11-03T09:32:43Z

ledger/sandbox-common/src/main/scala/platform/sandbox/services/SandboxResetService.scala

@@ -44,6 +44,7 @@ class SandboxResetService(
      serverCallHandler: ServerCallHandler[ReqT, RespT],
  ): Listener[ReqT] = {
    if (resetInitialized.get) {
+      // TODO error codes: Adapt V2 ?


Participant team

tudor-da · 2021-11-03T09:33:03Z

ledger/sandbox-common/src/main/scala/platform/sandbox/services/SandboxResetService.scala

@@ -71,6 +72,7 @@ class SandboxResetService(
    logger.info("Initiating server reset.")

    if (!resetInitialized.compareAndSet(false, true))
+    // TODO error codes: Adapt V2 ?


Participant team

tudor-da · 2021-11-03T09:33:27Z

ledger/sandbox-on-x/src/main/scala/com/daml/ledger/sandbox/ReadWriteServiceBridge.scala

@@ -261,6 +261,7 @@ object ReadWriteServiceBridge {
          )
        case QueueOfferResult.Failure(throwable) =>
          logger.error("Error enqueueing new submission.", throwable)
+          // TODO error codes: Adapt V2 ?


Participant team

tudor-da · 2021-11-03T09:33:35Z

ledger/sandbox-on-x/src/main/scala/com/daml/ledger/sandbox/ReadWriteServiceBridge.scala

@@ -269,6 +270,7 @@ object ReadWriteServiceBridge {
          )
        case QueueOfferResult.QueueClosed =>
          logger.error("Error enqueueing new submission: queue is closed.")
+          // TODO error codes: Adapt V2 ?


Participant team

tudor-da · 2021-11-03T09:41:27Z

ledger/participant-integration-api/src/main/scala/platform/store/Conversions.scala

@@ -332,6 +332,7 @@ private[platform] object Conversions {
    import RejectionReasonOps._

    def toParticipantStateRejectionReason: state.Update.CommandRejected.RejectionReasonTemplate =
+    // TODO error codes: Adapt V2 ?


Participant team: used in sandbox-classic

tudor-da · 2021-11-03T09:41:38Z

...integration-api/src/main/scala/platform/apiserver/services/tracking/QueueBackedTracker.scala

@@ -49,6 +49,7 @@ private[services] final class QueueBackedTracker(
            _.left.map(completionFailure => QueueCompletionFailure(completionFailure))
          )
        case QueueOfferResult.Failure(t) =>
+          // TODO error codes: Adapt V2 ?


Participant team.

tudor-da · 2021-11-03T09:41:47Z

...main/scala/com/digitalasset/ledger/client/services/commands/tracker/CompletionResponse.scala

@@ -85,6 +85,7 @@ object CompletionResponse {
    }

  private[daml] def toException(response: TrackedCompletionFailure): StatusException =
+  // TODO error codes: Adapt V2 ?


Participant team

Just like all command-tracker stuff, it is incorporated both into the ledger-api-server and the client code. As such we need to keep the runtime team in the loop.
@stefanobaghino-da FYI

tudor-da · 2021-11-03T09:41:53Z

...main/scala/com/digitalasset/ledger/client/services/commands/tracker/CompletionResponse.scala

@@ -115,6 +116,7 @@ object CompletionResponse {

  private def buildException(metadata: Map[String, String], status: StatusJavaProto.Builder) = {
    val details = mergeDetails(metadata, status)
+    // TODO error codes: Adapt V2 ?


Participant team

tudor-da · 2021-11-03T09:56:56Z

...participant-state/src/main/scala/com/daml/ledger/participant/state/v2/SubmissionResult.scala

@@ -28,6 +28,7 @@ object SubmissionResult {
    override val description: String = s"Submission failed with error ${status.message}"

    def exception: StatusRuntimeException =
+      // TODO error codes: Adapt V2 ?


Here we are already getting a Status, expecting that the WriteService is returning self-service error code formatted statuses. I'll look into it, thanks 👍

tudor-da · 2021-11-03T09:59:17Z

ledger/sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/Rejection.scala

@@ -28,6 +28,7 @@ object Rejection {
      domain.RejectionReason.InvalidLedgerTime(description)

    override lazy val toStateRejectionReason: state.Update.CommandRejected.RejectionReasonTemplate =
+      // TODO error codes: Adapt V2 ?


Participant team

pbatko-da

@fabiotudone-da, @remyhaemmerle-da, @stefanobaghino-da - Could you have a look? I mentioned you in the relevant comments.

pbatko-da · 2021-11-04T09:55:23Z

...ate/kvutils/src/main/scala/com/daml/ledger/participant/state/kvutils/api/BatchingQueue.scala

@@ -121,13 +121,15 @@ case class DefaultBatchingQueue(
                )
              )
            case f: QueueOfferResult.Failure =>
+              // TODO error codes: Adapt V2 ?


(Here and other places helpfully annotated with "KV committer team" by Tudor)

@fabiotudone-da We spotted these places that could need adapting to self service error codes (aka V2). Are you aware of these in the Committer team (if there're legitimate)?

pbatko-da · 2021-11-04T09:56:49Z

...er/scenario-service/server/src/main/scala/com/digitalasset/daml/lf/ScenarioServiceMain.scala

@@ -72,6 +72,7 @@ object ScenarioServiceMain extends App {

 object ScenarioService {
  private def notFoundContextError(id: Long): StatusRuntimeException =
+  // TODO error codes: Adapt V2 ?


(Here and other places helpfully annotated with "Language team" by Tudor)

@remyhaemmerle-da We spotted these places that could need adapting to self service error codes (aka V2). Are you aware of these in the Language team (if there're legitimate)?

pbatko-da · 2021-11-04T09:57:55Z

...c/main/scala/com/digitalasset/daml/lf/engine/script/ledgerinteraction/JsonLedgerClient.scala

@@ -439,6 +439,7 @@ class JsonLedgerClient(
        // TODO (MK) Using a grpc exception here doesn’t make that much sense.
        // We should refactor this to provide something more general.
        Future.successful(
+          // TODO error codes: Adapt V2 ?


@stefanobaghino-da @remyhaemmerle-da We spotted this place that could need adapting to self service error codes (aka V2). Are you aware of this in the Language or Runtime team (if it's legitimate)?

fabiotudone-da · 2021-11-04T11:04:22Z

@pbatko-da We've decided not to port batching-related errors to V2 as now we only use pre-execution. CC @miklos-da

pbatko-da · 2021-11-05T14:21:38Z

Thanks for the comments. To sum up:

Language sites are good to ignore (~going away in Daml 2.0)
Committer sites are good to ignore (as they batching-related which is not used)
Runtime site: [DPP-701] Maybe missed error code conversions #11510 (comment) waiting for @stefanobaghino-da
Other sites are Participant

(FYI @remyhaemmerle-da @stefanobaghino-da @fabiotudone-da)

[DPP-701] Maybe missed error code conversions

3d7ba64

pbatko-da self-assigned this Nov 2, 2021

tudor-da reviewed Nov 3, 2021

View reviewed changes

...ings-rxjava/src/test/scala/com/daml/ledger/rxjava/grpc/helpers/TransactionsServiceImpl.scala Show resolved Hide resolved

tudor-da reviewed Nov 3, 2021

View reviewed changes

.../sandbox-classic/src/main/scala/platform/sandbox/stores/ledger/inmemory/InMemoryLedger.scala Show resolved Hide resolved

tudor-da reviewed Nov 3, 2021

View reviewed changes

pbatko-da commented Nov 4, 2021

View reviewed changes

stefanobaghino-da mentioned this pull request Nov 23, 2021

Adopt rxjava bindings to V2 error codes #11587

Closed

pbatko-da closed this Nov 26, 2021

pbatko-da deleted the pbatko/error-codes/dpp-701-potentially-missed-error-code-conversions branch November 26, 2021 09:37

[DPP-701] Maybe missed error code conversions #11510

[DPP-701] Maybe missed error code conversions #11510

Conversation

pbatko-da commented Nov 2, 2021

Pull Request Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyhaemmerle-da Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyhaemmerle-da Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

remyhaemmerle-da Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tudor-da Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tudor-da Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pbatko-da left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fabiotudone-da commented Nov 4, 2021

pbatko-da commented Nov 5, 2021

remyhaemmerle-da Nov 4, 2021 •

edited

Loading

remyhaemmerle-da Nov 4, 2021 •

edited

Loading

remyhaemmerle-da Nov 4, 2021 •

edited

Loading

tudor-da Nov 3, 2021 •

edited

Loading

tudor-da Nov 3, 2021 •

edited

Loading