Bug 795272 - QIF importer causes application crash if action is invalid.

Fixes the crash and also pauses after loading if there are load errors.
The load "protection" catches exceptions other than bad-date so that it's
real protection.
Check for unbalanced transactions (i.e. with only one account) and don't
try to match if there is; report the error in the error log space in the
assistant.
Don't proceed to finding duplicates if the new account tree hasn't been
created, there aren't any.
This commit is contained in:
John Ralls
2018-06-15 14:22:43 -07:00
parent 1f8f681732
commit d6de324b32
2 changed files with 98 additions and 78 deletions

View File

@@ -59,6 +59,7 @@
#include "gnc-prefs.h"
#include "gnc-ui.h"
#include "guile-mappings.h"
#include <gfec.h>
#include "swig-runtime.h"
@@ -1067,6 +1068,11 @@ gnc_ui_qif_import_commodity_update(QIFImportWindow * wind)
}
}
static void
_gfec_error_handler(const char *message)
{
PERR("qif-import:qif-to-gnc-undo encountered an error: %s", message);
}
/****************************************************************
* gnc_ui_qif_import_convert_undo
@@ -1082,7 +1088,7 @@ gnc_ui_qif_import_convert_undo(QIFImportWindow * wind)
gnc_set_busy_cursor(NULL, TRUE);
/* Undo the conversion. */
scm_call_1(undo, wind->imported_account_tree);
gfec_apply(undo, wind->imported_account_tree, _gfec_error_handler);
/* There's no imported account tree any more. */
scm_gc_unprotect_object(wind->imported_account_tree);
@@ -1916,6 +1922,7 @@ gnc_ui_qif_import_load_progress_start_cb(GtkButton * button,
wind->ask_date_format = TRUE;
}
wind->load_stop = TRUE;
}
else
{
@@ -1937,22 +1944,27 @@ gnc_ui_qif_import_load_progress_start_cb(GtkButton * button,
gtk_widget_set_sensitive(wind->load_pause, FALSE);
gtk_widget_set_sensitive(wind->load_start, FALSE);
/* The file was loaded successfully. */
gnc_progress_dialog_set_sub(wind->load_progress, _("Loading completed"));
gnc_progress_dialog_set_value(wind->load_progress, 1);
scm_gc_unprotect_object(wind->imported_files);
wind->imported_files = imported_files;
scm_gc_protect_object(wind->imported_files);
gtk_widget_set_sensitive(wind->load_pause, FALSE);
wind->busy = FALSE;
if (wind->load_stop == FALSE)
{
/* The file was loaded successfully. */
gnc_progress_dialog_set_sub(wind->load_progress, _("Loading completed"));
gnc_progress_dialog_set_value(wind->load_progress, 1);
scm_gc_unprotect_object(wind->imported_files);
wind->imported_files = imported_files;
scm_gc_protect_object(wind->imported_files);
gtk_widget_set_sensitive(wind->load_pause, FALSE);
wind->busy = FALSE;
/* Auto step to next page */
gtk_assistant_set_current_page (assistant, num + 1);
}
else
{
wind->load_stop = FALSE;
}
}
@@ -2934,58 +2946,59 @@ gnc_ui_qif_import_convert_progress_start_cb(GtkButton * button,
wind->busy = FALSE;
wind->load_stop = TRUE;
}
/* Save the imported account tree. */
scm_gc_unprotect_object(wind->imported_account_tree);
wind->imported_account_tree = retval;
scm_gc_protect_object(wind->imported_account_tree);
/*
* Detect potentially duplicated transactions.
*/
/* This step will fill the remainder of the bar. */
gnc_progress_dialog_push(wind->convert_progress, 1);
retval = scm_call_3(find_duplicates,
scm_c_eval_string("(gnc-get-current-root-account)"),
wind->imported_account_tree, progress);
gnc_progress_dialog_pop(wind->convert_progress);
/* Save the results. */
scm_gc_unprotect_object(wind->match_transactions);
wind->match_transactions = retval;
scm_gc_protect_object(wind->match_transactions);
if (retval == SCM_BOOL_T)
if (wind->load_stop == FALSE)
{
/* Canceled by the user. */
gtk_widget_set_sensitive(wind->convert_pause, FALSE);
gnc_progress_dialog_set_sub(wind->convert_progress, _("Canceling"));
wind->busy = FALSE;
wind->load_stop = TRUE;
/* Save the imported account tree. */
scm_gc_unprotect_object(wind->imported_account_tree);
wind->imported_account_tree = retval;
scm_gc_protect_object(wind->imported_account_tree);
/*
* Detect potentially duplicated transactions.
*/
/* This step will fill the remainder of the bar. */
gnc_progress_dialog_push(wind->convert_progress, 1);
retval = scm_call_3(find_duplicates,
scm_c_eval_string("(gnc-get-current-root-account)"),
wind->imported_account_tree, progress);
gnc_progress_dialog_pop(wind->convert_progress);
/* Save the results. */
scm_gc_unprotect_object(wind->match_transactions);
wind->match_transactions = retval;
scm_gc_protect_object(wind->match_transactions);
if (retval == SCM_BOOL_T)
{
/* Canceled by the user. */
gtk_widget_set_sensitive(wind->convert_pause, FALSE);
gnc_progress_dialog_set_sub(wind->convert_progress, _("Canceling"));
wind->busy = FALSE;
wind->load_stop = TRUE;
}
else if (retval == SCM_BOOL_F)
{
/* An error occurred during duplicate checking. */
/* Remove any converted data. */
gnc_progress_dialog_set_sub(wind->convert_progress, _("Cleaning up"));
gnc_ui_qif_import_convert_undo(wind);
/* Inform the user. */
gnc_progress_dialog_append_log(wind->convert_progress,
_( "A bug was detected while detecting duplicates."));
gnc_progress_dialog_set_sub(wind->convert_progress, _("Failed"));
gnc_progress_dialog_reset_value(wind->convert_progress);
gnc_error_dialog (GTK_WINDOW (assistant), "%s",
_( "A bug was detected while detecting duplicates."));
/* FIXME: How should we request that the user report this problem? */
gtk_widget_set_sensitive(wind->convert_pause, FALSE);
wind->busy = FALSE;
wind->load_stop = TRUE;
}
}
else if (retval == SCM_BOOL_F)
{
/* An error occurred during duplicate checking. */
/* Remove any converted data. */
gnc_progress_dialog_set_sub(wind->convert_progress, _("Cleaning up"));
gnc_ui_qif_import_convert_undo(wind);
/* Inform the user. */
gnc_progress_dialog_append_log(wind->convert_progress,
_( "A bug was detected while detecting duplicates."));
gnc_progress_dialog_set_sub(wind->convert_progress, _("Failed"));
gnc_progress_dialog_reset_value(wind->convert_progress);
gnc_error_dialog (GTK_WINDOW (assistant), "%s",
_( "A bug was detected while detecting duplicates."));
/* FIXME: How should we request that the user report this problem? */
gtk_widget_set_sensitive(wind->convert_pause, FALSE);
wind->busy = FALSE;
wind->load_stop = TRUE;
}
/* Enable the Assistant Forward Button */
gtk_assistant_set_page_complete (assistant, page, TRUE);

View File

@@ -255,7 +255,11 @@
(length (qif-file:xtns b))))))
(work-to-do 0)
(work-done 0))
;; Log any errors
(define (errorproc message)
(if (string? message)
(qif-import:log progress-dialog "qif-import:qif-to-gnc"
message)))
;; This procedure handles progress reporting, pause, and cancel.
(define (update-progress)
(set! work-done (+ 1 work-done))
@@ -379,7 +383,7 @@
(update-progress)
(if (not (qif-xtn:mark xtn))
(qif-import:mark-matching-xtns xtn rest))
(qif-import:mark-matching-xtns xtn rest errorproc))
(if (not (null? (cdr rest)))
(xloop (car rest) (cdr rest)))))
@@ -431,7 +435,7 @@
(lambda ()
(catch 'cancel
(lambda ()
(catch 'bad-date private-convert (lambda (key . args) key)))
(catch #t private-convert (lambda (key . args) key)))
(lambda (key . args) #t)))))
@@ -810,7 +814,7 @@
;; mark them so they won't be imported.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(define (qif-import:mark-matching-xtns xtn candidate-xtns)
(define (qif-import:mark-matching-xtns xtn candidate-xtns errorproc)
(let splitloop ((splits-left (qif-xtn:splits xtn)))
;; splits-left starts out as all the splits of this transaction.
@@ -822,7 +826,7 @@
(qif-split:category-is-account? (car splits-left)))
(set! splits-left
(qif-import:mark-some-splits
splits-left xtn candidate-xtns))
splits-left xtn candidate-xtns errorproc))
(set! splits-left (cdr splits-left))))
(if (not (null? splits-left))
@@ -835,7 +839,7 @@
;; don't get imported.
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(define (qif-import:mark-some-splits splits xtn candidate-xtns)
(define (qif-import:mark-some-splits splits xtn candidate-xtns errorproc)
(let* ((n- (lambda (n) (gnc-numeric-neg n)))
(nsub (lambda (a b) (gnc-numeric-sub a b 0 GNC-DENOM-LCD)))
(n+ (lambda (a b) (gnc-numeric-add a b 0 GNC-DENOM-LCD)))
@@ -910,17 +914,20 @@
;; this is the grind loop. Go over every unmarked transaction in
;; the candidate-xtns list.
(let xtn-loop ((xtns candidate-xtns))
(if (and (not (qif-xtn:mark (car xtns)))
(string=? (qif-xtn:from-acct (car xtns)) far-acct-name))
(begin
(set! how
(qif-import:xtn-has-matches? (car xtns) near-acct-name
date amount group-amount))
(if how
(begin
(qif-import:merge-and-mark-xtns xtn same-acct-splits
(car xtns) how)
(set! done #t)))))
(if (not (and far-acct-name near-acct-name))
(if errorproc
(errorproc "Transaction with no or only one associated account."))
(if (and (not (qif-xtn:mark (car xtns))))
(string=? (qif-xtn:from-acct (car xtns)) far-acct-name)
(begin
(set! how
(qif-import:xtn-has-matches? (car xtns) near-acct-name
date amount group-amount))
(if how
(begin
(qif-import:merge-and-mark-xtns xtn same-acct-splits
(car xtns) how)
(set! done #t))))))
;; iterate with the next transaction
(if (and (not done)
(not (null? (cdr xtns))))