redstreet / fava_investor

Comprehensive set of reports, analyses, and tools for investments, for Beancount and Fava (plain text, double entry accounting software). Includes Fava plugins, command line tools, and libraries for each module.
GNU General Public License v3.0
136 stars 20 forks source link

[Feature] Write errors to GUI callout instead of exiting #74

Open putzwasser opened 2 years ago

putzwasser commented 2 years ago

TLDR If fava_investor hits some errors it writes to stderr, which leads fava to crash/exit with error code 1.

It would be nicer to display a warning callout instead—just like the

This page adds charts using a deprecated method which will be removed soon.

notification.


Two examples:

No asset allocation account found

2000-01-01 custom "fava-extension" "fava_investor" "{
  'asset_alloc_by_class' : {
    'accounts_patterns'    : [
            'Assets:Investing',
            'Assets:DoesNotExist'],
  },
}"

This will print error specificed in https://github.com/redstreet/fava_investor/blob/48641b9d4c239370ee48217d27a2751ebe88d588/fava_investor/modules/assetalloc_class/libassetalloc.py#L162

Related: #76 One could argue that it doesn't make sense to include a non-existing account. But:

2000-01-01 custom "fava-extension" "fava_investor" "{
  'asset_alloc_by_class' : {
    'accounts_patterns'    : [
            'Assets:Investing',
            'Assets:Cash'],
  },
}"

With this config I can compare my investments and the amount of cash I'm holding. If I wanted to know the ratios of my investments I need to exclude the cash accounts. I'd do it in fava via the GUI, but it "breaks" fava:

image

Nov 10 15:44:27: No included accounts found. (Your --accounts <regex> failed to match any account)
Nov 10 15:44:27: fava.service: Main process exited, code=exited, status=1/FAILURE
Nov 10 15:44:27: fava.service: Failed with result 'exit-code'.

(I'm running fava as a linux service unit)

The only way to exclude accounts is by changing the config and restarting fava. It would be way nicer to leverage the powerful webUI for excluding specific accounts temporarily. Why is this useful? Well, besides the cash example, if there's real estate in your assets it normally makes 70-90% of your asset allocation. This makes it hard to see if you're overexposed to another investment.


Error: unable to convert PRICE to base currency XY If you didn't explicitly add a price directive for an exit, fava_investor prints this error message

Error: unable to convert NEWASSET to base currency USD. (Missing price directive?)

https://github.com/redstreet/fava_investor/blob/48641b9d4c239370ee48217d27a2751ebe88d588/fava_investor/modules/assetalloc_class/libassetalloc.py#L112

So, a bean containing this:

plugin "beancount.plugins.implicit_prices"
2000-01-01 * "" "Buy new Asset"
  Assets:Investments:MyNewAsset                                  1 NEWASSET {1000 USD} @ 1000 USD
  Assets:Cash

Would "break" fava because of the stderr message. You need to explicitly add a price directive for NEWASSET:

plugin "beancount.plugins.implicit_prices"
2000-01-01 * "" "Buy new Asset"
  Assets:Investments:MyNewAsset                                  1 NEWASSET {1000 USD} @ 1000 USD
  Assets:Cash

2000-01-01 price NEWASSET 1000 USD

(Actually, this is also a feature request: #77 import/use the implicit prices that are added to postings)

redstreet commented 2 years ago

From what I can tell, there are a few separate asks here:

1) For errors, don't crash fava, but use some kind of an existing notification mechanism

2) Enable the GUI filters

3) import/use the implicit prices that are added to postings

Anything else?

Could you please file separate issues for each issue? This makes it easier to discuss each of them. Thanks!

putzwasser commented 2 years ago

Number one is the main one, for which I created the issue.

The other two are side issues, of this. I guess, allowing to filter accounts would be a very nice feature, though.

Created

redstreet commented 2 years ago

To clarify: Investor doesn't crash, but catches the error and exits (important distinction).

Agreed, if there is a persistent GUI callout, I'd be okay using that instead. Contributions welcome. I'm unlikely to get to this soon. Note that a solution would have to work for the command line and library as well.