Fix More informative error message on import failure (#684)

* Fix More informative error message on import failure
Fixes #670

* Handle errors when celery is not configured

* Fix test

Author: DavidMStraub

gramps_webapi/api/resources/transactions.py

@@ -70,9 +70,12 @@ def post(self, args) -> ResponseReturnValue:
             if isinstance(task, AsyncResult):
                 return make_task_response(task)
             return task, 200
-        trans_dict = process_transactions(
-            tree=tree, user_id=user_id, payload=payload, force=args["force"]
-        )
+        try:
+            trans_dict = process_transactions(
+                tree=tree, user_id=user_id, payload=payload, force=args["force"]
+            )
+        except ValueError as exc:
+            abort_with_message(400, str(exc))
         res = Response(
             response=json.dumps(trans_dict),
             status=200,

gramps_webapi/api/tasks.py

@@ -47,6 +47,7 @@
 from .report import run_report
 from .resources.delete import delete_all_objects
 from .resources.util import (
+    abort_with_message,
     app_has_semantic_search,
     dry_run_import,
     run_import,
@@ -60,7 +61,6 @@
     update_telemetry_timestamp,
 )
 from .util import (
-    abort_with_message,
     check_quota_people,
     close_db,
     get_config,
@@ -76,7 +76,10 @@ def run_task(task: Task, **kwargs) -> Union[AsyncResult, Any]:
     """Send a task to the task queue or run immediately if no queue set up."""
     if not current_app.config["CELERY_CONFIG"]:
         with current_app.app_context():
-            return task(**kwargs)
+            try:
+                return task(**kwargs)
+            except Exception as exc:
+                abort_with_message(500, str(exc))
     return task.delay(**kwargs)
 
 
@@ -497,7 +500,7 @@ def process_transactions(
                     ):
                         if num_people_added or num_people_deleted:
                             update_usage_people(tree=tree, user_id=user_id)
-                        abort_with_message(409, "Object has changed")
+                        raise ValueError("Object has changed")
                     new_data = item["new"]
                     if new_data:
                         new_obj = gramps_object_from_dict(new_data)
@@ -515,11 +518,11 @@ def process_transactions(
                     else:
                         if num_people_added or num_people_deleted:
                             update_usage_people(tree=tree, user_id=user_id)
-                        abort_with_message(400, "Unexpected transaction type")
+                        raise ValueError("Unexpected transaction type")
                 except (KeyError, UnicodeDecodeError, json.JSONDecodeError, TypeError):
                     if num_people_added or num_people_deleted:
                         update_usage_people(tree=tree, user_id=user_id)
-                    abort_with_message(400, "Error while processing transaction")
+                    raise ValueError("Error while processing transaction")
             trans_dict = transaction_to_json(trans)
     finally:
         # close the *writeable* db handle regardless of errors
@@ -570,7 +573,7 @@ def handle_commit(trans: DbTxn, class_name: str, obj) -> None:
 def handle_add(trans: DbTxn, class_name: str, obj) -> None:
     """Handle an add action."""
     if class_name != "Tag" and not obj.gramps_id:
-        abort_with_message(400, "Gramps ID missing")
+        raise ValueError("Gramps ID missing")
     handle_commit(trans, class_name, obj)
 
 

tests/test_endpoints/test_importers.py

@@ -203,7 +203,7 @@ def test_importers_example_data_quota(self):
             data=file_obj,
             headers=headers,
         )
-        assert rv.status_code == 405
+        assert rv.status_code == 500
         rv = check_success(self, f"{BASE_URL}/people/")
         assert len(rv) == 0
         with self.test_app.app_context():

tests/test_endpoints/test_transactions.py

@@ -379,7 +379,7 @@ def test_missing_handle(self):
         rv = self.client.post(
             "/api/transactions/?background=1", json=trans, headers=headers
         )
-        self.assertEqual(rv.status_code, 400)
+        self.assertEqual(rv.status_code, 500)
 
     def test_missing_gramps_id(self):
         """Add with missing gramps ID."""
@@ -397,7 +397,7 @@ def test_missing_gramps_id(self):
         rv = self.client.post(
             "/api/transactions/?background=1", json=trans, headers=headers
         )
-        self.assertEqual(rv.status_code, 400)
+        self.assertEqual(rv.status_code, 500)
 
     def test_gramps60_issue(self):
         """Test for an issue that occurred after upgrading to Gramps 6.0"""