GNOME Bugzilla – Bug 487572
advanced portfolio breaks on "Most Recent to Report"
Last modified: 2018-06-29 21:52:22 UTC
Selecting the "Most Recent to Report" price source crashes the report a la: In /home/andrew/opt/gnucash/share/gnucash/guile-modules/gnucash/main.scm: 154: 15* [lazy-catch #t #<procedure #f ()> #<procedure dumper (key . args)>] In unknown file: ?: 16* [#<procedure #f ()>] In /home/andrew/opt/gnucash/share/gnucash/guile-modules/gnucash/main.scm: 155: 17* [apply #<procedure #f ()> ()] In unknown file: ?: 18 [#<procedure #f ()>] In /home/andrew/opt/gnucash/share/gnucash/scm/report.scm: ... 460: 19 (set! html (gnc:report-render-html report #t)) 460: 20* [gnc:report-render-html # #t] 426: 21 (if (and (not #) (gnc:report-ctext report)) (gnc:report-ctext report) ...) 434: 22 (let ((template #) (doc #f)) (set! doc (if template # #f)) doc) 437: 23* (set! doc (if template (let* # # # ...) #f)) 437: 24* (if template (let* # # # ...) #f) 438: 25 (let* (# # # ...) (gnc:html-document-set-style-sheet! doc stylesheet) ...) 440: 26* [advanced-portfolio-renderer #] In /home/andrew/opt/gnucash/share/gnucash/guile-modules/gnucash/report/advanced-portfolio.scm: 179: 27 (let (# # #) (letrec # # #)) ... 553: 28 (let (# # # ...) (gnc:html-document-set-title! document #) ...) 593: 29* (if (not (null? accounts)) (let* (# # # ...) (if show-symbol #) ...) ...) 595: 30 (let* (# # # # ...) (if show-symbol #) (if show-listing #) ...) 649: 31* [table-add-stock-rows # # # ...] 287: 32 (let ((share-print-info #)) (letrec (#) (set! work-to-do #) ...)) ... 294: 33 (let* (# # # # ...) (for-each # #) (set! use-txn #) ...) 322: 34* (exchange-fn (gnc:make-gnc-monetary commodity units) currency) /home/andrew/opt/gnucash/share/gnucash/guile-modules/gnucash/report/advanced-portfolio.scm:322:25: In expression (exchange-fn (gnc:make-gnc-monetary commodity units) currency): /home/andrew/opt/gnucash/share/gnucash/guile-modules/gnucash/report/advanced-portfolio.scm:322:25: Wrong type to apply: #<unspecified> I will attach a test file that causes this crash. Steps to reproduce: 1. open the attached file "Gnutest" 2. open an advanced portfolio report. 3. click the option button. 4. Select the general tab. 5. select a price source "Most Recent to Report" 6. click "OK" 7. ?? 8. profit^Wcrash report
Created attachment 97374 [details] test file to replicate this bug
Other "advanced portfolio" bugs (feel free to close this one as "duplicate" if they are in fact identical or a direct subset of this bug): bug#115267 bug#336240 bug#343245 bug#344566 bug#346062 bug#460232 Does your patch of bug#460232 help here?
oh, that might work. Good catch... testing... nope. at least not directly. It does look like there is a smob thing going on here, I'll look into it some more. The state of the variables before it crashes: (commodity; #<swig-pointer gnc_commodity * 8664bd0> units: #<<gnc-numeric> num: 2000000 denom: 10000> currency: #<swig-pointer gnc_commodity * 88792c8> make-monetary: #<<gnc-monetary> commodity: #<swig-pointer gnc_commodity * 8664bd0> amount: #<<gnc-numeric> num: 2000000 denom: 10000>>) In unknown file: ?: 7* [#<procedure #f ()>] ...crash report... thanks for the clue.
I'm trying to figure this out and it must be a problem in the exchange function itself, I guess. how is the exchange-fn different in this case than in the non-crasher?
I found it, I think. The problem is that an exchange-fn gets created using this bit from commodity-utilities.scm: (define (gnc:case-exchange-fn source-option report-currency to-date-tp) (case source-option ((weighted-average) (gnc:make-exchange-function (gnc:make-exchange-alist report-currency to-date-tp))) ((pricedb-latest) gnc:exchange-by-pricedb-latest) ((pricedb-nearest) (lambda (foreign domestic) (gnc:exchange-by-pricedb-nearest foreign domestic to-date-tp))) (else (gnc:warn "gnc:case-exchange-fn: bad price-source value: " source-option)))) but it is possible for source-option == pricedb-latest-before which is not defined . That should throw a warning, but also doesn't return any kind of function, so later, it crashes. I think I understand that, anyway. And swig doesn't handle it where gwrap probably ignored it? I don't know the specifics of how that works, but that's got to be the case. There should be more than just a warning here. There should be some kind of sane default to prevent crashes as well. Also, obviously, there should be a pricedb-latest-before option as well with its associated gnc:exchange-by-pricedb-latest-before function.
okay. I need some help on this one. Its a question of what to do with the report. The problem is that we have two different sets of price/exchanges going on here. One is the price of the commodities being reported on (stocks, bonds, whatever) and the other is the exchange rate of the currencies used to buy and sell the commodities. Currently, we use the *same* option for both of these -- price-source. I suppose that's fine except that they are different things. ISTM that technically, there should be two options -- one for the source of prices for the commodities and one for the exchange rate to use for different currencies. For those really savvy traders who are following prices of stocks in multiple currencies where exchange rates are changing at a different rate than the stock prices reflect ?? shrug. Anyway. I'm attaching a patch to prevent the report crashing by setting a sane default for the exchange-fn. I assume that people want their reports in *today's* yen or dollar or whatever regardless of where they are getting their stock prices from. What needs to happen though is some determination of how to implement exchange rates versus commodity price sources. If we decide to leave them as a homogenous unit as they currently are, then the gnc:exchange-by-pricedb-latest-before needs to be implemented because the current fix is broken in regards to that. Also, it looks like gnc:case-exchange-time-fn is also broken in the same way. Currently no report implements a price-source option that would trigger this bug, but it could happen. Should that be patched as well?
Created attachment 98597 [details] [review] patch to prevent crashing of adv. portfolio report. as described above, this patch only prevents the crash, and does not actually fix functionality of the report. That will require hacking up more exchange functions.
Comment on attachment 98597 [details] [review] patch to prevent crashing of adv. portfolio report. >Index: src/report/report-system/commodity-utilities.scm >=================================================================== >--- src/report/report-system/commodity-utilities.scm (revision 16580) >+++ src/report/report-system/commodity-utilities.scm (working copy) >@@ -828,8 +828,17 @@ > ((pricedb-nearest) (lambda (foreign domestic) > (gnc:exchange-by-pricedb-nearest > foreign domestic to-date-tp))) >- (else (gnc:warn "gnc:case-exchange-fn: bad price-source value: " >- source-option)))) >+ (else (begin >+ (gnc:warn "gnc:case-exchange-fn: bad price-source value: " >+ source-option) >+ ;; return a sane, though possibly incorrect function. This >+ ;; prevents report-crashing bugs in reports that implement >+ ;; additional source-options that have not been coded up >+ ;; here yet. >+ ;; >+ ;; Known to be missing: pricedb-latest-before. >+ gnc:exchange-by-pricedb-latest) >+ ))) > > ;; Return a ready-to-use function. Which one to use is determined by > ;; the value of 'source-option', whose possible values are set in >@@ -858,6 +867,9 @@ > ((pricedb-latest) (lambda (foreign domestic date) > (gnc:exchange-by-pricedb-latest foreign domestic))) > ((pricedb-nearest) gnc:exchange-by-pricedb-nearest) >+ ;; warning!! this (else ) block will not return a function and >+ ;; will cause crashes in reports that implement new source-options >+ ;; without adding them to this function!!! > (else (gnc:warn "gnc:case-exchange-time-fn: bad price-source value: " > source-option)))) >
Created attachment 98624 [details] [review] corrected version well, that didn't work the way I expected. here is a de-reprot-ified version.
patch above (or a similar one anyway) committed 16651.
branches/2.2@r16661. Thanks!
GnuCash bug tracking has moved to a new Bugzilla host. This bug has been copied to https://bugs.gnucash.org/show_bug.cgi?id=487572. Please update any external references or bookmarks.