Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Self-service error codes] No redundant logging in Ledger API [DPP-731] #11649

Merged
merged 1 commit into from
Nov 15, 2021
Merged

[Self-service error codes] No redundant logging in Ledger API [DPP-731] #11649

merged 1 commit into from
Nov 15, 2021

Conversation

tudor-da
Copy link
Contributor

CHANGELOG_BEGIN
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
  • 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 tudor-da force-pushed the tudor/dpp-731-ledger-api-no-redundant-logging branch 2 times, most recently from d9fbb2e to 96dcfff Compare November 11, 2021 10:20
tudor-da
tudor-da commented Nov 11, 2021
View reviewed changes
...ledger-api-common/src/main/scala/com/digitalasset/platform/server/api/ValidationLogger.scala
t
}

def logFailureWithContext[Request, T <: Throwable](request: Request, t: T)(implicit
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate with different name (compared to above)

tudor-da
tudor-da commented Nov 11, 2021
View reviewed changes
...i-common/src/main/scala/com/digitalasset/platform/server/api/validation/ErrorFactories.scala
),
v1 = io.grpc.Status.INVALID_ARGUMENT
.withDescription(message)
.asRuntimeException(),
v2 = LedgerApiErrors.ReadErrors.MalformedPackageId
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplification

@tudor-da tudor-da force-pushed the tudor/dpp-731-ledger-api-no-redundant-logging branch from 96dcfff to 9a423c7 Compare November 11, 2021 10:30
@tudor-da tudor-da force-pushed the tudor/dpp-731-ledger-api-no-redundant-logging branch from 9a423c7 to a63a511 Compare November 14, 2021 07:38
@tudor-da tudor-da marked this pull request as ready for review November 14, 2021 07:51
@tudor-da tudor-da requested review from a team as code owners November 14, 2021 07:51
@tudor-da tudor-da changed the title [Self-service error codes] No redundant logging in Ledger API [Self-service error codes] No redundant logging in Ledger API [DPP_731 Nov 14, 2021
@tudor-da tudor-da changed the title [Self-service error codes] No redundant logging in Ledger API [DPP_731 [Self-service error codes] No redundant logging in Ledger API [DPP-731] Nov 14, 2021
mziolekda
mziolekda approved these changes Nov 15, 2021
View reviewed changes
ledger/error/src/main/scala/com/daml/error/ErrorCode.scala
@@ -137,7 +138,15 @@ abstract class ErrorCode(val id: String, val category: ErrorCategory)(implicit
// Builder methods for metadata are not exposed, so going route via creating an exception
val ex = StatusProto.toStatusRuntimeException(status)
// Strip stack trace from exception
new ErrorCode.ApiException(ex.getStatus, ex.getTrailers)
// TODO error codes: Define a generic mechanism (trait or method) to check if errors are logged on creation,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@tudor-da tudor-da merged commit 8e08450 into main Nov 15, 2021
@tudor-da tudor-da deleted the tudor/dpp-731-ledger-api-no-redundant-logging branch November 15, 2021 08:13
azure-pipelines bot pushed a commit that referenced this pull request Nov 17, 2021
release 1.18.0-snapshot.20211116.8395.0.ccbf7140
539fd4e 
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.

@victormueller-da is in charge of this release.

Commit log:
```
ccbf714 expand the text about errors in the log [DPP-755] (#11723)
fbc436d [DPP-739][Self-service error codes] Adapt QueueBackedTracker error codes (#11719)
07cff7c [DPP-417][Self-service error-codes] Document error categories.  (#11727)
c866fa6 Optimize data migrations for performance [DPP-714] (#11714)
f801314 Drop exercise with actor from LF spec (#11721)
a63e091 kvutils: Use consistent argument name casing in KVErrors (#11609)
c1f4beb Ledger ID mismatch does not change error codes. (#11720)
b5b29fd test_suite for http-json, http-json-oracle integration tests (#11713)
a29fee9 [Self-service error-codes] Add introductory section to self-service error codes page. (#11712)
639c431 libs-scala: Move `SourceQueueResourceOwner` from the Integration Kit (#11708)
05f604d [Self-service error codes] Enrich migration guide [DPP-755] (#11711)
2d5b548 Use explicit party specifications in Daml Script over JSON API (#11678)
844e5d0 participant-integration-api: Propagate log context through the indexer. (#11710)
c969266 Formatting feedback from: Log execution context throwables as error (#11707)
ec6d7cc [Self-service error codes] Error code definitions revisiting [DPP-675] (#11686)
a1fc5c6 Simplify handling for EqualList builtin. (#11676)
90ad968 Workflow for the ledger-api-bench-tool  defined in YAML file [DPP-669] (#11682)
1791a52 [TS-Bindings] Changes to re-enable ws multiplexing (#11681)
fd94099 update compat versions for 1.18.0-snapshot.20211111.8349.0.d938a44c (#11667)
612a838 ledger-api-bench-tool: Allocate observers indexed from 0 until the total number (#11700)
41d2d95 Factorize Java codegen integration test build defs (#11704)
bf86ee4 Log execution context throwables as error (#11702)
69471d6 ledger-api-client: Fix a log message related to an empty submission ID (#11695)
775b41c LF: Reintroduce TransactionVersion.asVersionedTransaction (#11692)
8015f60 Add `cpu:2` tag to extractor tests (#11697)
8e08450 [Self-service error codes] No redundant logging in Ledger API (#11649)
18433eb fix arity of TArrow type constructor in Daml-LF spec (#11677)
fb840fa Introduce filter table [DPP-714] (#11687)
cb5a675 Activate template_id interning [DPP-713] (#11680)
c4da8e3 Limit gRPC wire transport size legacy error codes size (#11666)
72f3dd4 Activate party interning [DPP-712] (#11663)
57f1b86 Improve byInterfaceNodes based on suggestions (#11672)
d6ca68d [Self-service error codes] Adapt Sandbox classic rejection error codes (#11622)
15b925a LF: Compute transaction version as the maximum of the children (#11626)
ebdab31 rotate release duty after 1.18.0-snapshot.20211109.8328.0.92181161 (#11624)
7eb22dd LF: drop forgotten deprecate Node aliases. (#11673)
b0463aa Move script auth tests to postgresql (#11670)
8c46559 Remove unnecessary constructors: SEDamlException, SEImportValue, from SExpr{0,1} (#11668)
c109552 Disable ofInterestRule by default (#11637)
409864f Use by_interface field when fetching transaction dependencies. (#11662)
81e2450 [DDP-596][Self-service error codes] Add migration guide (#11632)
3192d5e Different types before after closure conversion (#11661)
2f4476c [Self-service error codes] Fill in missing places with submission id for correlation id (#11593)
d3bb036 Fix sandbox database config in script & trigger tests (#11655)
22d916b party-set arguments for JSON API (#11454)
3d109b6 Release snapshot 1.18.0-snapshot.20211111.8349.0.d938a44c (#11664)
```
Changelog:
```

- [Daml Script] When run over the JSON API, Daml Script can now use a
  subset of the claims in the token. E.g., `submit p` works even if you
  have a token with `actAs = [p, p2]`.

- [Integration Kit] - ledger-api-bench-tool - All stream configuration parameters can be defined through the YAML configuration file.
[TS-BINDINGS] Re-enable ws multiplexing for stream queries after resolving the reconnect connection close bug associated with ws state and liveness.
- [JSON API] ``actAs`` and ``readAs`` may be specified for create, exercise,
  create-and-exercise, non-WS fetch, and non-WS query.
  See `issue #11454 <https://github.com/digital-asset/daml/pull/11454>`__.
```

CHANGELOG_BEGIN
CHANGELOG_END
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mziolekda mziolekda mziolekda approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tudor-da @mziolekda