Parseable, consistent error messages

[DamienCassou]

Right now, hledger check error messages are hard to parse which makes writing tools around hledger hard. Here is an example of an error output when an account is undeclared:

$ hledger check --strict
hledger: 
strict mode: undeclared account "debt"
in transaction at: "/home/cassou/Documents/configuration/ledger/2021.ledger" (lines 11-13)

and here is an error output when a transaction doesn't balance:

$ hledger check --strict
hledger: "/home/cassou/Documents/configuration/ledger/2021.ledger" (lines 16-18)
could not balance this transaction:
real postings' sum should be 0 but is: -1.00€
2021-01-01 Misc
    asset:account         -11.00€
    expense:food           10.00€

As you can see, the structures of the error messages are different, requiring different error parsers.

[simonmichael]

I also have the feeling that, for maximum compatibility with tools, the first line should contain the word "error" and the error's position. Does that sound correct ?

We also want error messages to be readable on a standard 80-char terminal, so we usually have a challenge with long lines.

Perhaps it's not necessary to mention the program name.

And of course eventually we'd like to show a pretty, coloured human-friendly rendering of the error region.

Some mockups:

$ hledger check payees
Error at "/Users/simon/src/hledger/examples/sample.journal" (line 27):
Undeclared payee "income"

> 2018-01-01 income
      assets:bank:checking              $1
      income:salary                    $-1

$ hledger check --strict
Error at "/home/cassou/Documents/configuration/ledger/2021.ledger" (line 12):
Undeclared account "debt"

  2020-01-01
>   debt    1
    assets

$ hledger check
Error in transaction at "/home/cassou/Documents/configuration/ledger/2021.ledger" (lines 16-18):
Could not balance this transaction:
real postings' sum should be 0 but is: -1.00€

> 2021-01-01 Misc
>     asset:account         -11.00€
>     expense:food           10.00€

We also have an cosmetic issue where some errors include text like "*** Exception:" or "ExitFailure 1" and others don't, depending on where they are handled.

[simonmichael]

More notes:

• best to include a brief error description on the first line as well, even if it makes lines wide ?
• and/or include a unique searchable short error code ? Maybe our errors are simple enough this is overkill.
• also be compatible with LSP servers, eg for hledger-vscode

[DamienCassou]

Emacs defines a bunch of regular expressions in compilation-error-regexp-alist-alist to match many types of error messages. This could be a starting point.

Another way to report errors is to use several-line long messages as in clang. Your mockups above are a good step in that direction.

My main requirement would be to use the same error structure for all messages and to document it.

If you want to go fancy, I suggest using a parseable structured format such as JSON (e.g., JSON-RPC error format).

I also have the feeling that, for maximum compatibility with tools, the first line should contain the word "error" and the error's position. Does that sound correct ?

It doesn't matter to me much because all tools are different and require different error-handling support anyway.

We also want error messages to be readable on a standard 80-char terminal, so we usually have a challenge with long lines.

Not really important to me either.

Perhaps it's not necessary to mention the program name.

indeed

And of course eventually we'd like to show a pretty, coloured human-friendly rendering of the error region.

That would be nice to have. For my use-case, it's not important though because flycheck will highlight the problematic region of the buggy file: I only care about the position (start and end lines as well as the start and end columns) and the error message.

To me there are clearly 2 use cases:

1. The CLI user who wants to see a nice error message with details, some excerpt of his/her file and colors.
2. The software developer who wants the error to be parseable so it can be integrated into another tool (e.g., flycheck). The developer doesn't want colors or parts of the problematic file in the error message.

Which brings me to my idea that there is something wrong with the way that hledger's checking functions are implemented. First, the check engine stops at the first error it encounters even though it is not blocking (e.g., an undeclared payee shouldn't prevent a transaction balance check on the next transaction). Another problem is that the code is harder to understand: instead of collecting validation results, the code throws exceptions which has an impact on the calling code. Finally, because each checking function is responsible for formatting error messages, the 2 use cases above can't be implemented at the same time.

I suggest reading replaceThrowWithNotification refactoring and applying it if possible. This will result in each checking function being responsible for returning (and not throwing) validation results. Then, the calling code is responsible for taking the decision regarding the validation results: e.g.,

• hledger balance could decide that the validation results shouldn't prevent the balance report from being calculated and displayed. It might even decide to show the validation results as warnings or don't show them at all;
• hledger check --output-format json may format all validation results in a JSON object (e.g., with the JSON-RPC error format);
• hledger check may format all validation results as a human-readable string.

There is also the case that a user might want some validation results (e.g., undeclared accounts) to be considered errors (and thus prevent hledger balance from computing any report) whereas other users might consider them warning or just ignore them.

We also have an cosmetic issue where some errors include text like "*** Exception:" or "ExitFailure 1" and others don't, depending on where they are handled.

My suggestion above would fix that by stopping the checking functions from being responsible for quitting the application.

best to include a brief error description on the first line as well, even if it makes lines wide ?

I'm not sure that is important and I would leave them on the next lines.

and/or include a unique searchable short error code ? Maybe our errors are simple enough this is overkill.

That would be a nice bonus. flycheck includes support for error IDs and links to error descriptions on the web as is used in shellcheck errors for example.

I don't see that as very important right now though.

also be compatible with LSP servers, eg for hledger-vscode

I don't know much about LSP but I guess that a compatible syntax can't hurt (either LSP or JSON-RPC or something else).

[simonmichael]

Thanks for the input. I agree that reporting all errors (where possible) rather than just one would be better. The check command is brand new, but we'll get there.

[simonmichael]

Probably the next action for this issue is to start gathering examples of all our current errors for review.
[Starting points:

• https://hledger.org/hledger.html#check
• https://github.com/simonmichael/hledger/tree/master/hledger/test -> check-*.test

]

[DamienCassou]

Here is a list of matchers I implemented for all errors I found:

(;; Used for an unbalanced transaction:
 (error line-start "hledger: \"" (file-name) "\" (lines " line "-" end-line ")\n"
        (message (zero-or-more line-start (zero-or-more not-newline) "\n")) "\n")
  ;; Used for invalid balance assertion:
  (error line-start "hledger: balance assertion: \"" (file-name) "\" (line " line ", column " column ")\n"
          "transaction:\n"
          (message (zero-or-more line-start (zero-or-more not-newline) "\n")) "\n")
  ;; Used for invalid regular expression:
  (error line-start "hledger: " (message "this regular expression" (zero-or-more not-newline)) "\n")
  ;; Used for an undeclared payee:
  (error line-start "Error: " (message) "\n"
        "at: \"" (file-name) "\" (lines " line "-" end-line ")\n")
  ;; Used for unordered dates:
  (error line-start "Error: " (message) "\n"
        "at \"" (file-name) "\" (lines " line "-" end-line "):\n")
  ;; Used for duplicate leaf names:
  (error line-start "Error: " (message) "\n")
  ;; Used for an undeclared account:
  (error line-start "hledger: " (message) "\n"
        "in transaction at: \"" (file-name) "\" (lines " line "-" end-line ")\n")
  ;; Used for parse errors and invalid dates:
  (error line-start "hledger: " (file-name) ":" line ":" column ":\n"
        (message (zero-or-more line-start (zero-or-more not-newline) "\n")) "\n"))

Some explanation:

1. Each matcher starts with the word error to indicate the level of the problem.
2. After the word error starts the regular expression (expressed in the readable but unfamiliar syntax of rx).
3. Some parts of the regular expression contains error-matching keywords:
   • (message) means "match all text to the end of the line and consider that the error message";
   • (message REGEXP) means "match REGEXP and consider that the error message";
   • (file-name) means "match all text to the end of the line and consider that the filename where the error appears";
   • line means "match a number and consider that the line where the error appears";
   • end-line means "match a number and consider that the last line where the error appears";
   • column means "match a number and consider that the column where the error appears".

[DamienCassou]

I added 2 more errors in my message above:

• Used for invalid balance assertion
• Used for invalid regular expression

Please note that some error messages have no filename or line number.