Open hurryabit opened 4 years ago
@hurryabit we do not have a way of getting the details of which field in particular was not properly formatted, however in case of invalid JSON, Spray gives us the line and position in the input JSON at which it failed (see 2nd example below).
We should also add some context to our errors. E.g. in case of empty party string, we can say something like this:
Party format error. JsonError: spray.json.DeserializationException: empty string
When submitting a create command we can do:
"CreateCommand format error. JsonError: JsonReaderError. Cannot read JSON: <{\n \"templateId\": \"Iou:Iou\"xxxx,\n \"payload\": {\n \"issuer\": \"Alice\",\n \"owner\": \"Alice\",\n \"currency\": \"USD\",\n \"amount\": \"999.99\",\n \"observers\": []\n }\n}>. Cause: spray.json.JsonParser$ParsingException: Unexpected character 'x' at input index 27 (line 2, position 26), expected '}':\n \"templateId\": \"Iou:Iou\"xxxx,\n ^\n"
The error message when a key is not found in the meantime changed to:
Command interpretation error in LF-DAMLe: dependency error: couldn't find key com.daml.lf.transaction.GlobalKey@1c2d82ec
because GlobalKey
is not a case class
anymore. So it's actually impossible to learn anything about the key.
I also agree with the scary looking error message around user abort
.
Both of these issues were recently reported by a customer trying to understand what's going on.
The error message when a key is not found in the meantime changed to:
Command interpretation error in LF-DAMLe: dependency error: couldn't find key com.daml.lf.transaction.GlobalKey@1c2d82ec
because
GlobalKey
is not acase class
anymore. So it's actually impossible to learn anything about the key.
That is very bad for debuggability. Do we know why GlobalKey
is not a case class anymore? And is there anything we can do to make the message better? cc @remyhaemmerle-da
GlobalKey
is not a case class. We just need to override the toString
method to make it nicer.
Needs scoping and design, will become more relevant after the Ledger API adopts the new error codes (scheduled for Nov 2021).
Currently, the only way to get nice error messages for users of apps written in DAML seems to be to replicate a fair chunk of business logic in the frontend. Catching exceptions coming from the JSON API or Ledger API and displaying them to the user will horrify them.
Here are a few examples:
JsonError: spray.json.DeserializationException: empty string
. This error message does not contain the slightest hint of what has gone wrong. When you try to use a forbidden character in a party identifier, you getJsonError: spray.json.DeserializationException: non expected character 0x2e in \"dr.hu\"
. This hints at least at the fact what has gone wrong but still looks terrifying.create-daml-app
, the choice to add new friends contains a few assertions with nice error messages that check that you don't add yourself or an existing friend. However, when these assertions fail, you get the nice error message hidden somewhere between a lot of other stuff:io.grpc.StatusRuntimeException: INVALID_ARGUMENT: Command interpretation error in LF-DAMLe: Interpretation error: Error: User abort: You cannot add yourself as a friend. Details: Last location: [DA.Internal.Assert:20], partial transaction: <empty transaction>.
orio.grpc.StatusRuntimeException: INVALID_ARGUMENT: Command interpretation error in LF-DAMLe: Interpretation error: Error: User abort: You cannot add a friend twice. Details: Last location: [DA.Internal.Assert:20], partial transaction: <empty transaction>.
These errors look scarier than they need to.io.grpc.StatusRuntimeException: INVALID_ARGUMENT: Command interpretation error in LF-DAMLe: dependency error: couldn't find key GlobalKey(Identifier(ca0008d825a30569a47d53663134058f89f4544240cbc6a607a87096c87fb048,User:User),ValueParty(Carl),Hash(0d6d9e8f4d6d9fa60322554bcf0c9ed6abb2ecf9377eb1625446f3bc029f0ef1)). Details: N/A.
I think there are a few things we could immediately improve:
assert*
should be easily accessible by the frontend.My general impression is that it will need a concentrated effort to make this better across the board.
Related to: #3247, #4521
cc @bame-da @gerolf-da @S11001001 @leo-da