Add error messages (#389)

* Start adding error messages

* Fix circular import

* More messages

* Make abort_with_message work well outside request

* Add more error msgs

* More descriptive error on user creation

* Fix missing import

* Revert "More descriptive error on user creation"

This reverts commit 6f45690.

* Fix users

* Change error message

Author: DavidMStraub

gramps_webapi/api/auth.py

@@ -22,7 +22,7 @@
 from functools import wraps
 from typing import Iterable
 
-from flask import abort, current_app
+from flask import abort
 from flask_jwt_extended import get_jwt, verify_jwt_in_request
 from flask_jwt_extended.exceptions import NoAuthorizationError
 

gramps_webapi/api/export.py

@@ -43,7 +43,7 @@
 from gramps.gen.utils.resourcepath import ResourcePath
 
 from ..const import DISABLED_EXPORTERS
-from .util import get_locale_for_language
+from .util import abort_with_message, get_locale_for_language
 
 _ = glocale.translation.gettext
 
@@ -229,35 +229,35 @@ def prepare_options(db_handle: DbReadBase, args: Dict):
         if args["gramps_id"] is not None:
             gramps_id = args["gramps_id"]
             if db_handle.get_person_from_gramps_id(gramps_id) is None:
-                abort(422)
+                abort_with_message(422, "Person with this Gramps ID not found")
         else:
             try:
                 person = db_handle.get_person_from_handle(args["handle"])
             except HandleError:
-                abort(422)
+                abort_with_message(422, "Person with this handle not found")
             gramps_id = person.gramps_id
         try:
             options.set_person_filter(args["person"], gramps_id)
-        except ValueError:
-            abort(422)
+        except ValueError as exc:
+            abort_with_message(422, str(exc))
     if args["event"] is not None:
         try:
             options.set_event_filter(args["event"])
-        except ValueError:
-            abort(422)
+        except ValueError as exc:
+            abort_with_message(422, str(exc))
     if args["note"] is not None:
         try:
             options.set_note_filter(args["note"])
-        except ValueError:
-            abort(422)
+        except ValueError as exc:
+            abort_with_message(422, str(exc))
     try:
         options.set_proxy_order(args["sequence"])
-    except ValueError:
-        abort(422)
+    except ValueError as exc:
+        abort_with_message(422, str(exc))
     if args["locale"] is not None:
         options.locale = get_locale_for_language(args["locale"])
         if options.locale is None:
-            abort(422)
+            abort_with_message(422, "Locale not found")
     return options
 
 
@@ -276,6 +276,6 @@ def run_export(db_handle: DbReadBase, extension: str, options):
             export_function = plugin.get_export_function()
             result = export_function(db_handle, file_path, User(), options)
             if not result:
-                abort(500)
+                abort_with_message(500, "Export function failed")
             return file_name, "." + extension
-    abort(404)  # exporter not found
+    abort_with_message(404, "Exporter not found")  # exporter not found

gramps_webapi/api/file.py

@@ -35,6 +35,7 @@
 
 from ..types import FilenameOrPath
 from .image import LocalFileThumbnailHandler, detect_faces
+from .util import abort_with_message
 
 
 class FileHandler:
@@ -94,7 +95,7 @@ def get_face_regions(self, etag: Optional[str] = None):
                 regions = detect_faces(fobj)
             except ImportError:
                 # numpy or opencv missing
-                abort(501)
+                abort_with_message(501, "OpenCV is not installed")
         else:
             regions = []
         res = make_response(jsonify(regions))
@@ -139,7 +140,7 @@ def get_file_object(self) -> BinaryIO:
         try:
             self._check_path()
         except ValueError:
-            abort(403)
+            abort_with_message(403, "File access not allowed")
         with open(self.path_abs, "rb") as f:
             stream = BytesIO(f.read())
         return stream
@@ -152,7 +153,7 @@ def get_file_size(self) -> int:
         try:
             self._check_path()
         except ValueError:
-            abort(403)
+            abort_with_message(403, "File access not allowed")
         return os.path.getsize(self.path_abs)
 
     def send_file(
@@ -177,7 +178,7 @@ def send_cropped(self, x1: int, y1: int, x2: int, y2: int, square: bool = False)
         try:
             self._check_path()
         except ValueError:
-            abort(403)
+            abort_with_message(403, "File access not allowed")
         thumb = LocalFileThumbnailHandler(self.path_abs, self.mime)
         buffer = thumb.get_cropped(x1=x1, y1=y1, x2=x2, y2=y2, square=square)
         return send_file(buffer, mimetype=MIME_JPEG)
@@ -187,7 +188,7 @@ def send_thumbnail(self, size: int, square: bool = False):
         try:
             self._check_path()
         except ValueError:
-            abort(403)
+            abort_with_message(403, "File access not allowed")
         thumb = LocalFileThumbnailHandler(self.path_abs, self.mime)
         buffer = thumb.get_thumbnail(size=size, square=square)
         return send_file(buffer, mimetype=MIME_JPEG)
@@ -199,7 +200,7 @@ def send_thumbnail_cropped(
         try:
             self._check_path()
         except ValueError:
-            abort(403)
+            abort_with_message(403, "File access not allowed")
         thumb = LocalFileThumbnailHandler(self.path_abs, self.mime)
         buffer = thumb.get_thumbnail_cropped(
             size=size, x1=x1, y1=y1, x2=x2, y2=y2, square=square

gramps_webapi/api/report.py

@@ -43,6 +43,7 @@
 from gramps.gen.utils.resourcepath import ResourcePath
 
 from ..const import MIME_TYPES, REPORT_DEFAULTS, REPORT_FILTERS
+from .util import abort_with_message
 
 _EXTENSION_MAP = {".gvpdf": ".pdf", ".gspdf": ".pdf"}
 
@@ -191,7 +192,7 @@ def run_report(
             file_type = _EXTENSION_MAP.get(file_type) or file_type
             if file_type not in MIME_TYPES:
                 current_app.logger.error(f"Cannot find {file_type} in MIME_TYPES")
-                abort(500)
+                abort_with_message(500, f"MIME type {file_type} not found")
             report_path = current_app.config.get("REPORT_DIR")
             os.makedirs(report_path, exist_ok=True)
             file_name = f"{uuid.uuid4()}{file_type}"
@@ -241,14 +242,14 @@ def validate_options(report: Dict, report_options: Dict, allow_file: bool = Fals
                 abort(422)
             continue
         if not isinstance(report_options[option], str):
-            abort(422)
+            abort_with_message(422, "Report options must be provided as strings")
         if "A number" in report["options_help"][option][2]:
             try:
                 float(report_options[option])
             except ValueError:
-                abort(422)
+                abort_with_message(422, "Cannot convert option string to number")
         if "Size in cm" in report["options_help"][option][2]:
             try:
                 float(report_options[option])
             except ValueError:
-                abort(422)
+                abort_with_message(422, "Cannot convert option string to number")

gramps_webapi/api/resources/base.py

@@ -53,6 +53,7 @@
 from .match import match_dates
 from .sort import sort_objects
 from .util import (
+    abort_with_message,
     add_object,
     fix_object_dict,
     get_backlinks,
@@ -83,7 +84,9 @@ def full_object(
             obj.backlinks = get_backlinks(self.db_handle, obj.handle)
         if args.get("soundex"):
             if self.gramps_class_name not in ["Person", "Family"]:
-                abort(422)
+                abort_with_message(
+                    422, f"Option soundex is not allowed for {self.gramps_class_name}"
+                )
             obj.soundex = get_soundex(self.db_handle, obj, self.gramps_class_name)
         obj = self.object_extend(obj, args, locale=locale)
         if args.get("profile") and (
@@ -147,17 +150,17 @@ def _parse_object(self) -> GrampsObject:
         """Parse the object."""
         obj_dict = request.json
         if obj_dict is None:
-            abort(400)
+            abort_with_message(400, "Empty object")
         if "_class" not in obj_dict:
             obj_dict["_class"] = self.gramps_class_name
         elif obj_dict["_class"] != self.gramps_class_name:
-            abort(400)
+            abort_with_message(400, "Wrong object type")
         try:
             obj_dict = fix_object_dict(obj_dict)
-        except ValueError:
-            abort(400)
+        except ValueError as exc:
+            abort_with_message(400, "Error while processing object")
         if not validate_object_dict(obj_dict):
-            abort(400)
+            abort_with_message(400, "Schema validation failed")
         return from_json(json.dumps(obj_dict))
 
     def has_handle(self, handle: str) -> bool:
@@ -249,7 +252,7 @@ def delete(self, handle: str) -> Response:
         get_etag = hash_object(obj)
         for etag in request.if_match:
             if etag != get_etag:
-                abort(412)
+                abort_with_message(412, "Resource does not match provided ETag")
         trans_dict = delete_object(
             self.db_handle_writable, handle, self.gramps_class_name
         )
@@ -273,16 +276,16 @@ def put(self, handle: str) -> Response:
         get_etag = hash_object(obj_old)
         for etag in request.if_match:
             if etag != get_etag:
-                abort(412)
+                abort_with_message(412, "Resource does not match provided ETag")
         obj = self._parse_object()
         if not obj:
-            abort(400)
+            abort_with_message(400, "Empty object")
         db_handle = self.db_handle_writable
         with DbTxn("Edit object", db_handle) as trans:
             try:
                 update_object(db_handle, obj, trans)
-            except ValueError:
-                abort(400)
+            except ValueError as exc:
+                abort_with_message(400, "Error while updating object")
             trans_dict = transaction_to_json(trans)
         # update search index
         tree = get_tree_from_jwt()
@@ -431,13 +434,13 @@ def post(self) -> Response:
             check_quota_people(to_add=1)
         obj = self._parse_object()
         if not obj:
-            abort(400)
+            abort_with_message(400, "Empty object")
         db_handle = self.db_handle_writable
         with DbTxn("Add objects", db_handle) as trans:
             try:
                 add_object(db_handle, obj, trans, fail_if_exists=True)
-            except ValueError:
-                abort(400)
+            except ValueError as exc:
+                abort_with_message(400, "Error while adding object")
             trans_dict = transaction_to_json(trans)
         # update usage
         if self.gramps_class_name == "Person":

gramps_webapi/api/resources/events.py

@@ -31,7 +31,7 @@
 from webargs import fields, validate
 
 from ...types import Handle
-from ..util import get_db_handle, get_locale_for_language, use_args
+from ..util import abort_with_message, get_db_handle, get_locale_for_language, use_args
 from . import ProtectedResource
 from .base import (
     GrampsObjectProtectedResource,
@@ -62,7 +62,7 @@ def object_extend(
                 obj.extended["place"] = get_place_by_handle(db_handle, obj.place)
         if "profile" in args:
             if "families" in args["profile"] or "events" in args["profile"]:
-                abort(422)
+                abort_with_message(422, "profile contains invalid keys")
             obj.profile = get_event_profile_for_object(
                 db_handle, obj, args["profile"], locale=locale
             )

gramps_webapi/api/resources/export_media.py

@@ -29,7 +29,7 @@
 from ..auth import has_permissions
 from ..ratelimiter import limiter_per_user
 from ..tasks import AsyncResult, export_media, make_task_response, run_task
-from ..util import get_buffer_for_file, get_tree_from_jwt
+from ..util import abort_with_message, get_buffer_for_file, get_tree_from_jwt
 from . import ProtectedResource
 
 
@@ -67,7 +67,7 @@ def get(self, filename: str) -> Response:
         )
         match = regex.match(filename)
         if not match:
-            abort(422)
+            abort_with_message(422, "Invalid filename")
 
         file_path = os.path.join(export_path, filename)
         if not os.path.isfile(file_path):

gramps_webapi/api/resources/exporters.py

@@ -37,7 +37,13 @@
 from ..auth import has_permissions
 from ..export import get_exporters, prepare_options, run_export
 from ..tasks import AsyncResult, export_db, make_task_response, run_task
-from ..util import get_buffer_for_file, get_db_handle, get_tree_from_jwt, use_args
+from ..util import (
+    abort_with_message,
+    get_buffer_for_file,
+    get_db_handle,
+    get_tree_from_jwt,
+    use_args,
+)
 from . import ProtectedResource
 from .emit import GrampsJSONEncoder
 
@@ -196,7 +202,7 @@ def get(self, extension: str, filename: str) -> Response:
         )
         match = regex.match(filename)
         if not match:
-            abort(422)
+            abort_with_message(422, "Invalid filename")
 
         file_type = match.group(2)
         file_path = os.path.join(export_path, filename)

gramps_webapi/api/resources/facts.py

@@ -32,7 +32,7 @@
 from gramps.plugins.lib.librecords import find_records
 from webargs import fields, validate
 
-from ..util import get_db_handle, get_locale_for_language, use_args
+from ..util import abort_with_message, get_db_handle, get_locale_for_language, use_args
 from . import ProtectedResource
 from .emit import GrampsJSONEncoder
 
@@ -57,12 +57,12 @@ def get_person_filter(db_handle: DbReadBase, args: Dict) -> Union[GenericFilter,
     if args["gramps_id"]:
         gramps_id = args["gramps_id"]
         if db_handle.get_person_from_gramps_id(gramps_id) is None:
-            abort(422)
+            abort_with_message(422, "Person with this Gramps ID not found")
     else:
         try:
             person = db_handle.get_person_from_handle(args["handle"])
         except HandleError:
-            abort(422)
+            abort_with_message(422, "Person with this handle not found")
         gramps_id = person.gramps_id
 
     person_filter = filters.GenericFilter()