converting server errors to proper client errors #11184

S11001001 · 2021-10-08T23:08:30Z

CHANGELOG_BEGIN
- [JSON API] Several kinds of gRPC server errors are now reported with
  associated HTTP statuses; for example, a Daml-LF interpreter error now
  returns a 400 instead of a 500, and an exercise on an archived contract
  returns a 409 Conflict instead of a 500.  Errors internal to JSON API
  (e.g. internal assertion failures) are no longer detailed in the HTTP
  response; their details are only logged.
  See `issue #11184 <https://github.com/digital-asset/daml/pull/11184>`__.
CHANGELOG_END

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

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.

…ng correctly

…ercises

…wer-servererrors

…ipant-server error

…e ServerErrors

…wer-servererrors

CHANGELOG_BEGIN - [JSON API] Several kinds of gRPC server errors are now reported with associated HTTP statuses; for example, a Daml-LF interpreter error now returns a 400 instead of a 500, and an exercise on an archived contract returns a 409 Conflict instead of a 500. Errors internal to JSON API (e.g. internal assertion failures) are no longer detailed in the HTTP response; their details are only logged. See `issue #11184 <https://github.com/digital-asset/daml/pull/11184>`__. CHANGELOG_END

realvictorprm · 2021-10-19T11:48:18Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/util/GrpcHttpErrorCodes.scala

+  import akka.http.scaladsl.model.{StatusCode, StatusCodes => A}
+
+  implicit final class `gRPC status as akka http`(private val self: G) extends AnyVal {
+    // some version of this mapping _should_ already exist somewhere, right? -SC


Sadly I don't have a clue either ._.

realvictorprm

Looks good to me, nice work 😃 !

ledger-service/http-json/src/main/scala/com/digitalasset/http/util/GrpcHttpErrorCodes.scala

…wer-servererrors

@sofiafaro-da

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @sofiafaro-da is in charge of this release. Commit log: ``` 8ff347d Expand type synonyms lazily (#11282) 2fd200f KV: support contextual logging in KeyValueConsumption [KVL-1143] (#11288) f4df1cc converting server errors to proper client errors (#11184) 525e4ce Add GrpcStatus.toProto(JavaStatus) -> ScalaStatus conversion [KVL-1143] (#11292) 9b00a1a Rotate release rotation (#11291) dd09c38 Upgrade rules-nodejs (#11290) 87f1418 [Short] Remove unnecessary traits from ApiSubmissionServiceSpec (#11254) 6b65a72 Merge npm install steps in create-daml-app tests (#11287) a9f6afb kvutils: Rename `VersionedOffset` to `KVOffset`. (#11286) 98cf8d8 KV: introduce v2 error codes behind a CLI switch [KVL-1140] (#11224) 46f6877 Increase time model skew limits (#11273) 8f94cff kvutils: Use `VersionedOffsetBuilder` where possible, and introduce `VersionedOffsetMutator`. [KVL-1154] (#11277) 81fde97 Bazel: Call `_wrap_rule` directly when building the Scala REPL rule. (#11279) edb2b04 Document scaladoc usage in Bazel (#11278) 6255837 Data dependencies support for reexports (#11147) 2fc7490 [Self-service error codes] Adapt error factories [DPP-656] (#11270) b1a6b11 ledger-api-test-tool: Add static time awareness [KVL-1156] (#11266) 243c120 Add an LF typechecking benchmark (#11276) 2db8c13 Test authorization within Exercise choice. (#11246) ec58ed6 Typecheck nested LF type applications more efficiently (#11253) a940016 Add debugRaw corresponding to traceM in Haskell (#11259) a988579 Fix Scala repl targets (#11272) e4808f6 Extract UtilLF module from daml-lf-conversion to its own library (#11263) 38712c0 sandbox-next - Disable participant command deduplication [KVL-1156] (#11264) 82fa229 Add daml-util-ghc lib (#11260) 68a93cf kvutils - Use the ledger configured time for command dedup [KVL-1149] (#11239) c8cede0 Bump startup time in create-daml-app tests (#11261) 4fac87f Bump the schema version for the JSON API (#11252) 5507670 update NOTICES file (#11256) 0063b10 Retry faster during startup (#11255) 04feb40 Improve reset service tests (#11240) b3375fd update compat versions for 1.18.0-snapshot.20211013.8071.0.514e8b50 (#11237) 9ed1eb3 Address security notice on `ansi-regex` (#11243) 387d0e8 Make compatibility migrations `eternal` (#11242) be899b3 Extract rejection_reason.proto from daml_kvutils.proto [KVL-1090] (#11235) a1d94e1 LF: Create special case class for VersionedContractInstance (#11127) b738988 Release 1.18 snapshot (#11234) 3c26852 LF: Use template Id in exercise node of fixed choice. (#11229) 139b6f3 CI: Set `PROJ_DIR` inside the bash lib, not outside. (#11236) ce64cb2 interfaces: Do some TODOs (#11231) 7a88c7d trigger-service: dev-mode-unsafe flag (#11233) 909a1bf [DPP-417][DDP-612] Adapt ApiSubmissionService to support V2 error codes (#11052) ``` Changelog: ``` - [JSON API] Several kinds of gRPC server errors are now reported with associated HTTP statuses; for example, a Daml-LF interpreter error now returns a 400 instead of a 500, and an exercise on an archived contract returns a 409 Conflict instead of a 500. Errors internal to JSON API (e.g. internal assertion failures) are no longer detailed in the HTTP response; their details are only logged. See `issue #11184 <https://github.com/digital-asset/daml/pull/11184>`__. - [Daml Stdlib] Add `debugRaw` as a convenience wrapper around `traceRaw` when used inside a do-block. `debugRaw` compares to `debug` like `traceRaw` compares to `trace` meaning it expects a `Text` instead of calling `show` on an expression. sandbox-next - Disable participant command deduplication kvutils - Command deduplication uses the configured time model (static/wall) and not running always using wall-clock [JSON API] Solving a bug that could cause the JSON API to return correct result if a contract with the same key is observed twice required a schema change. The JSON API data needs to be dropped and the query store needs to reset. If you are migrating from a previous version, either reset your database manually or start the HTTP JSON API with one of the options that regenerate the schema (`create-only`, `create-if-needed-and-start`, `create-and-start`). ``` CHANGELOG_BEGIN CHANGELOG_END

akshayshirahatti-da · 2021-10-20T09:14:30Z

ledger-service/http-json/src/main/scala/com/digitalasset/http/ContractsService.scala

@@ -644,7 +643,9 @@ object ContractsService {
    ): Source[Error \/ domain.ActiveContract[LfV], NotUsed]
  }

-  case class Error(id: Symbol, message: String)
+  final case class Error(id: Symbol, message: String)


can we just pick oneError/ InternalError given any issues here ( ContractsService ) should always be mapped as server error, can you explain the notion for the additional type ? 🤔

Right now we only have one error constructor so I would rather it not be a sum type. However, that isn't guaranteed, i.e.

any issues here ( ContractsService ) should always be mapped as server error

is not an invariant, it is a coincidence of the current implementation, so since I went to the trouble of determining that particular expressions were constructing internal errors, I should go ahead and document that fact (with actual typed expressions, not comments, as is my wont).

So should we need to split the type and add a client error data ctor, the existing invocations already self-document that they don't use the new client error ctor.

@sofiafaro-da

This PR has been created by a script, which is not very smart and does not have all the context. Please do double-check that the version prefix is correct before merging. @sofiafaro-da is in charge of this release. Commit log: ``` 8ff347d Expand type synonyms lazily (#11282) 2fd200f KV: support contextual logging in KeyValueConsumption [KVL-1143] (#11288) f4df1cc converting server errors to proper client errors (#11184) 525e4ce Add GrpcStatus.toProto(JavaStatus) -> ScalaStatus conversion [KVL-1143] (#11292) 9b00a1a Rotate release rotation (#11291) dd09c38 Upgrade rules-nodejs (#11290) 87f1418 [Short] Remove unnecessary traits from ApiSubmissionServiceSpec (#11254) 6b65a72 Merge npm install steps in create-daml-app tests (#11287) a9f6afb kvutils: Rename `VersionedOffset` to `KVOffset`. (#11286) 98cf8d8 KV: introduce v2 error codes behind a CLI switch [KVL-1140] (#11224) 46f6877 Increase time model skew limits (#11273) 8f94cff kvutils: Use `VersionedOffsetBuilder` where possible, and introduce `VersionedOffsetMutator`. [KVL-1154] (#11277) 81fde97 Bazel: Call `_wrap_rule` directly when building the Scala REPL rule. (#11279) edb2b04 Document scaladoc usage in Bazel (#11278) 6255837 Data dependencies support for reexports (#11147) 2fc7490 [Self-service error codes] Adapt error factories [DPP-656] (#11270) b1a6b11 ledger-api-test-tool: Add static time awareness [KVL-1156] (#11266) 243c120 Add an LF typechecking benchmark (#11276) 2db8c13 Test authorization within Exercise choice. (#11246) ec58ed6 Typecheck nested LF type applications more efficiently (#11253) a940016 Add debugRaw corresponding to traceM in Haskell (#11259) a988579 Fix Scala repl targets (#11272) e4808f6 Extract UtilLF module from daml-lf-conversion to its own library (#11263) 38712c0 sandbox-next - Disable participant command deduplication [KVL-1156] (#11264) 82fa229 Add daml-util-ghc lib (#11260) 68a93cf kvutils - Use the ledger configured time for command dedup [KVL-1149] (#11239) c8cede0 Bump startup time in create-daml-app tests (#11261) 4fac87f Bump the schema version for the JSON API (#11252) 5507670 update NOTICES file (#11256) 0063b10 Retry faster during startup (#11255) 04feb40 Improve reset service tests (#11240) b3375fd update compat versions for 1.18.0-snapshot.20211013.8071.0.514e8b50 (#11237) 9ed1eb3 Address security notice on `ansi-regex` (#11243) 387d0e8 Make compatibility migrations `eternal` (#11242) be899b3 Extract rejection_reason.proto from daml_kvutils.proto [KVL-1090] (#11235) a1d94e1 LF: Create special case class for VersionedContractInstance (#11127) b738988 Release 1.18 snapshot (#11234) 3c26852 LF: Use template Id in exercise node of fixed choice. (#11229) 139b6f3 CI: Set `PROJ_DIR` inside the bash lib, not outside. (#11236) ce64cb2 interfaces: Do some TODOs (#11231) 7a88c7d trigger-service: dev-mode-unsafe flag (#11233) 909a1bf [DPP-417][DDP-612] Adapt ApiSubmissionService to support V2 error codes (#11052) ``` Changelog: ``` - [JSON API] Several kinds of gRPC server errors are now reported with associated HTTP statuses; for example, a Daml-LF interpreter error now returns a 400 instead of a 500, and an exercise on an archived contract returns a 409 Conflict instead of a 500. Errors internal to JSON API (e.g. internal assertion failures) are no longer detailed in the HTTP response; their details are only logged. See `issue #11184 <https://github.com/digital-asset/daml/pull/11184>`__. - [Daml Stdlib] Add `debugRaw` as a convenience wrapper around `traceRaw` when used inside a do-block. `debugRaw` compares to `debug` like `traceRaw` compares to `trace` meaning it expects a `Text` instead of calling `show` on an expression. sandbox-next - Disable participant command deduplication kvutils - Command deduplication uses the configured time model (static/wall) and not running always using wall-clock [JSON API] Solving a bug that could cause the JSON API to return correct result if a contract with the same key is observed twice required a schema change. The JSON API data needs to be dropped and the query store needs to reset. If you are migrating from a previous version, either reset your database manually or start the HTTP JSON API with one of the options that regenerate the schema (`create-only`, `create-if-needed-and-start`, `create-and-start`). ``` CHANGELOG_BEGIN CHANGELOG_END Co-authored-by: Azure Pipelines Daml Build <[email protected]>

a model for trapping client errors in Scala bindings shim and reporti…

314026b

…ng correctly

S11001001 added team/ledger-clients Related to the Ledger Clients team's components. component/json-api HTTP JSON API labels Oct 8, 2021

S11001001 added this to the HTTP JSON API Maintenance milestone Oct 8, 2021

S11001001 added 4 commits October 8, 2021 19:16

clean up some nesting with an alias

a9c4e00

filter out client-side command service errors

f9062b7

fix flattening error propagation of CommandService errors in endpoints

dfbdcb0

remove todo

b115e4a

S11001001 mentioned this pull request Oct 13, 2021

track & report "client" errors and don't report other errors in responses #11154

Closed

S11001001 added 20 commits October 13, 2021 18:04

Daml evaluation triggers INVALID_ARGUMENT; handle this for creates/ex…

f10f0a3

…ercises

clean up lookupResult

ec16d19

remove stripLeft utility; it is unused

02bb076

proper error propagation for /parties endpoint

afbbe64

Merge commit 'b7389885e8ae2e97c34b8cdbc7dc7b11d9330c64' into 11154-fe…

ddd199a

…wer-servererrors

map grpc status codes to HTTP error codes

24388a6

add a case to pass-through gRPC errors in Endpoints errors

4291e39

handle gRPC status in all explicit top-level catches

828ffe8

pass through gRPC errors in CommandService as well

0589aff

treat a gRPC status anywhere in the causal chain as indicating partic…

9f89ef3

…ipant-server error

propagate ContractsService errors without assuming they will always b…

c6978ff

…e ServerErrors

filter ServerErrors' contents when rendering errorful streams

b5f7f4b

log errors from websocket output instead of rendering full messages

845a948

hide message in ServerError case

4f7a9d6

remove Aborted

2715a4f

Merge commit '46f6877ee5688ec9bd3ff3229d7827af2e865eff' into 11154-fe…

3d1a543

…wer-servererrors

transfer with bad contract ID now returns 409

facbb44

mention new error codes

68ad766

remove unused Show and liftErr utility

ac5c643

S11001001 self-assigned this Oct 19, 2021

S11001001 requested review from realvictorprm and akshayshirahatti-da October 19, 2021 03:38

S11001001 marked this pull request as ready for review October 19, 2021 03:38

S11001001 requested review from bame-da, nemanja-da and stefanobaghino-da as code owners October 19, 2021 03:38

realvictorprm reviewed Oct 19, 2021

View reviewed changes

realvictorprm approved these changes Oct 19, 2021

View reviewed changes

ledger-service/http-json/src/main/scala/com/digitalasset/http/util/GrpcHttpErrorCodes.scala Show resolved Hide resolved

S11001001 added 4 commits October 19, 2021 11:57

adapt daml-script to new error codes

e5b0e0e

adapt typescript tests to new error codes

7115456

adapt json-api failure tests to new error codes

56aba1c

Merge commit '9b00a1aab5ac1f4b9058739a91ca02597d84dda5' into 11154-fe…

eb6387c

…wer-servererrors

S11001001 requested review from aherrmann-da, cocreature and garyverhaegen-da as code owners October 19, 2021 15:58

S11001001 requested a review from a user October 19, 2021 15:58

S11001001 merged commit f4df1cc into main Oct 19, 2021

S11001001 deleted the 11154-fewer-servererrors branch October 19, 2021 17:54

akshayshirahatti-da reviewed Oct 20, 2021

View reviewed changes

S11001001 mentioned this pull request Nov 17, 2021

link to documentation detailing gRPC-to-HTTP code mapping #11742

Merged

7 tasks

This was referenced Feb 11, 2022

[JSON-API] Fix logging of internal server errors #12822

Merged

[BUG] JSON API doesn't log all kind of internal server errors #12887

Closed

S11001001 mentioned this pull request Apr 6, 2022

What should a multi-query WS stream do when one subquery fails due to pruning? #13506

Closed

S11001001 mentioned this pull request Feb 7, 2023

log suppressed errors out-of-band #16261

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

converting server errors to proper client errors #11184

converting server errors to proper client errors #11184

S11001001 commented Oct 8, 2021 •

edited

Loading

realvictorprm Oct 19, 2021 •

edited

Loading

realvictorprm left a comment

akshayshirahatti-da Oct 20, 2021

S11001001 Oct 20, 2021

converting server errors to proper client errors #11184

converting server errors to proper client errors #11184

Conversation

S11001001 commented Oct 8, 2021 • edited Loading

Pull Request Checklist

realvictorprm Oct 19, 2021 • edited Loading

Choose a reason for hiding this comment

realvictorprm left a comment

Choose a reason for hiding this comment

akshayshirahatti-da Oct 20, 2021

Choose a reason for hiding this comment

S11001001 Oct 20, 2021

Choose a reason for hiding this comment

S11001001 commented Oct 8, 2021 •

edited

Loading

realvictorprm Oct 19, 2021 •

edited

Loading