Merge pull request #263 from SFTtech/milo/improve-error-handling

fix: improve general error handling and error messages

.gitignore

@@ -97,6 +97,9 @@ venv.bak/
 .dmypy.json
 dmypy.json
 
+# Ruff
+.ruff_cache
+
 # Pyre type checker
 .pyre/
 

.vscode/launch.json

@@ -0,0 +1,13 @@
+{
+    "version": "0.2.0",
+    "configurations": [
+        {
+            "name": "API",
+            "type": "debugpy",
+            "request": "launch",
+            "args": ["-c", "config.yaml", "-vvv", "api"],
+            "cwd": "${workspaceFolder}",
+            "module": "abrechnung"
+        }
+    ]
+}

.vscode/settings.json

@@ -1,3 +1,8 @@
 {
-    "python.formatting.provider": "black"
+  "[python]": {
+    "editor.defaultFormatter": "charliermarsh.ruff",
+  },
+  "python.testing.pytestArgs": ["tests"],
+  "python.testing.unittestEnabled": false,
+  "python.testing.pytestEnabled": true
 }

.vscode/tasks.json

@@ -1,12 +1,10 @@
 {
-  // See https://go.microsoft.com/fwlink/?LinkId=733558
-  // for the documentation about the tasks.json format
   "version": "2.0.0",
   "tasks": [
     {
       "label": "Abrechnung API",
       "type": "shell",
-      "command": ".venv/bin/abrechnung -c config.yaml -vvv api",
+      "command": "abrechnung -c config.yaml -vvv api",
       "problemMatcher": []
     }
   ]

abrechnung/application/accounts.py

@@ -2,7 +2,7 @@
 
 import asyncpg
 from sftkit.database import Connection
-from sftkit.error import InvalidArgument
+from sftkit.error import AccessDenied, InvalidArgument
 from sftkit.service import Service, with_db_connection, with_db_transaction
 
 from abrechnung.config import Config
@@ -60,7 +60,7 @@ async def _check_account_permissions(
             raise InvalidArgument("account not found")
 
         if can_write and not (result["can_write"] or result["is_owner"]):
-            raise PermissionError("user does not have write permissions")
+            raise AccessDenied("user does not have write permissions")
 
         if account_type:
             type_check = [account_type] if isinstance(account_type, str) else account_type
@@ -217,7 +217,7 @@ async def create_account(
 
         if account.owning_user_id is not None:
             if not group_membership.is_owner and account.owning_user_id != user.id:
-                raise PermissionError("only group owners can associate others with accounts")
+                raise AccessDenied("only group owners can associate others with accounts")
 
         account_id = await conn.fetchval(
             "insert into account (group_id, type) values ($1, $2) returning id",
@@ -291,13 +291,13 @@ async def update_account(
 
         if account.owning_user_id is not None:
             if not membership.is_owner and account.owning_user_id != user.id:
-                raise PermissionError("only group owners can associate others with accounts")
+                raise AccessDenied("only group owners can associate others with accounts")
         elif (
             committed_account["owning_user_id"] is not None
             and committed_account["owning_user_id"] != user.id
             and not membership.is_owner
         ):
-            raise PermissionError("only group owners can remove other users as account owners")
+            raise AccessDenied("only group owners can remove other users as account owners")
 
         await conn.execute(
             "insert into account_history (id, revision_id, name, description, owning_user_id, date_info) "

abrechnung/application/groups.py

@@ -2,7 +2,7 @@
 
 import asyncpg
 from sftkit.database import Connection
-from sftkit.error import InvalidArgument
+from sftkit.error import AccessDenied, InvalidArgument
 from sftkit.service import Service, with_db_connection, with_db_transaction
 
 from abrechnung.config import Config
@@ -37,7 +37,7 @@ async def create_group(
         terms: str,
     ) -> int:
         if user.is_guest_user:
-            raise PermissionError("guest users are not allowed to create group new groups")
+            raise AccessDenied("guest users are not allowed to create group new groups")
 
         group_id = await conn.fetchval(
             "insert into grp (name, description, currency_symbol, terms, add_user_account_on_join, created_by) "
@@ -87,7 +87,7 @@ async def create_invite(
         valid_until: datetime,
     ) -> int:
         if user.is_guest_user:
-            raise PermissionError("guest users are not allowed to create group invites")
+            raise AccessDenied("guest users are not allowed to create group invites")
 
         await create_group_log(conn=conn, group_id=group_id, user=user, type="invite-created")
         return await conn.fetchval(
@@ -151,14 +151,14 @@ async def join_group(self, *, conn: Connection, user: User, invite_token: str) -
             invite_token,
         )
         if not invite:
-            raise PermissionError("Invalid invite token")
+            raise AccessDenied("Invalid invite token")
 
         group = await conn.fetchrow(
             "select id, add_user_account_on_join from grp where grp.id = $1",
             invite["group_id"],
         )
         if not group:
-            raise PermissionError("Invalid invite token")
+            raise AccessDenied("Invalid invite token")
 
         user_is_already_member = await conn.fetchval(
             "select exists (select user_id from group_membership where user_id = $1 and group_id = $2)",
@@ -270,10 +270,10 @@ async def update_member_permissions(
             return
 
         if is_owner and not group_membership.is_owner:
-            raise PermissionError("group members cannot promote others to owner without being an owner")
+            raise AccessDenied("group members cannot promote others to owner without being an owner")
 
         if membership["is_owner"]:
-            raise PermissionError("group owners cannot be demoted by other group members")
+            raise AccessDenied("group owners cannot be demoted by other group members")
 
         if is_owner:
             await create_group_log(
@@ -367,7 +367,7 @@ async def preview_group(self, *, conn: Connection, invite_token: str) -> GroupPr
             invite_token,
         )
         if not group:
-            raise PermissionError("invalid invite token to preview group")
+            raise AccessDenied("invalid invite token to preview group")
 
         return group
 

abrechnung/application/transactions.py

@@ -4,7 +4,7 @@
 
 import asyncpg
 from sftkit.database import Connection
-from sftkit.error import InvalidArgument
+from sftkit.error import AccessDenied, InvalidArgument
 from sftkit.service import Service, with_db_connection, with_db_transaction
 
 from abrechnung.application.common import _get_or_create_tag_ids
@@ -49,7 +49,7 @@ async def _check_transaction_permissions(
             raise InvalidArgument("user is not a member of this group")
 
         if can_write and not (result["can_write"] or result["is_owner"]):
-            raise PermissionError("user does not have write permissions")
+            raise AccessDenied("user does not have write permissions")
 
         if transaction_type:
             type_check = [transaction_type] if isinstance(transaction_type, TransactionType) else transaction_type

abrechnung/application/users.py

@@ -7,7 +7,7 @@
 from passlib.context import CryptContext
 from pydantic import BaseModel
 from sftkit.database import Connection
-from sftkit.error import InvalidArgument
+from sftkit.error import AccessDenied, InvalidArgument, Unauthorized
 from sftkit.service import Service, with_db_transaction
 
 from abrechnung.config import Config
@@ -79,9 +79,9 @@ def decode_jwt_payload(self, token: str) -> TokenMetadata:
             try:
                 return TokenMetadata.model_validate(payload)
             except:
-                raise PermissionError("invalid access token token")
+                raise Unauthorized("invalid access token")
         except JWTError:
-            raise PermissionError
+            raise Unauthorized("invalid access token")
 
     @with_db_transaction
     async def get_user_from_token(self, *, conn: Connection, token: str) -> User:
@@ -93,7 +93,7 @@ async def get_user_from_token(self, *, conn: Connection, token: str) -> User:
             token_metadata.user_id,
         )
         if not sess:
-            raise PermissionError
+            raise Unauthorized("invalid access token")
         return await self._get_user(conn=conn, user_id=token_metadata.user_id)
 
     async def _verify_user_password(self, user_id: int, password: str) -> bool:
@@ -202,7 +202,7 @@ async def register_user(
     ) -> int:
         """Register a new user, returning the newly created user id and creating a pending registration entry"""
         if not self.enable_registration:
-            raise PermissionError("User registrations are disabled on this server")
+            raise AccessDenied("User registrations are disabled on this server")
 
         requires_email_confirmation = self.require_email_confirmation and requires_email_confirmation
 
@@ -224,7 +224,7 @@ async def register_user(
             if self.enable_registration and has_valid_email:
                 self._validate_email_domain(email)
         elif not has_valid_email:
-            raise PermissionError(
+            raise AccessDenied(
                 f"Only users with emails out of the following domains are allowed: {self.valid_email_domains}"
             )
 
@@ -254,12 +254,12 @@ async def confirm_registration(self, *, conn: Connection, token: str) -> int:
             token,
         )
         if row is None:
-            raise PermissionError("Invalid registration token")
+            raise AccessDenied("Invalid registration token")
 
         user_id = row["user_id"]
         valid_until = row["valid_until"]
         if valid_until is None or valid_until < datetime.now(tz=timezone.utc):
-            raise PermissionError("Invalid registration token")
+            raise AccessDenied("Invalid registration token")
 
         await conn.execute("delete from pending_registration where user_id = $1", user_id)
         await conn.execute("update usr set pending = false where id = $1", user_id)
@@ -354,7 +354,7 @@ async def confirm_email_change(self, *, conn: Connection, token: str) -> int:
         user_id = row["user_id"]
         valid_until = row["valid_until"]
         if valid_until is None or valid_until < datetime.now(tz=timezone.utc):
-            raise PermissionError
+            raise AccessDenied("Invalid confirmation token")
 
         await conn.execute("delete from pending_email_change where user_id = $1", user_id)
         await conn.execute("update usr set email = $2 where id = $1", user_id, row["new_email"])
@@ -383,7 +383,7 @@ async def confirm_password_recovery(self, *, conn: Connection, token: str, new_p
         user_id = row["user_id"]
         valid_until = row["valid_until"]
         if valid_until is None or valid_until < datetime.now(tz=timezone.utc):
-            raise PermissionError
+            raise AccessDenied("Invalid confirmation token")
 
         await conn.execute("delete from pending_password_recovery where user_id = $1", user_id)
         hashed_password = self._hash_password(password=new_password)

abrechnung/core/auth.py

@@ -1,5 +1,5 @@
 from sftkit.database import Connection
-from sftkit.error import InvalidArgument
+from sftkit.error import AccessDenied, InvalidArgument
 
 from abrechnung.domain.groups import GroupMember
 from abrechnung.domain.users import User
@@ -24,10 +24,10 @@ async def check_group_permissions(
         raise InvalidArgument("group not found")
 
     if can_write and not (membership.is_owner or membership.can_write):
-        raise PermissionError("write access to group denied")
+        raise AccessDenied("write access to group denied")
 
     if is_owner and not membership.is_owner:
-        raise PermissionError("owner access to group denied")
+        raise AccessDenied("owner access to group denied")
 
     return membership
 

abrechnung/http/auth.py

@@ -2,6 +2,7 @@
 
 from fastapi import Depends, HTTPException, status
 from fastapi.security import OAuth2PasswordBearer
+from sftkit.error import AccessDenied
 
 from abrechnung.application.users import UserService
 from abrechnung.domain.users import User
@@ -21,7 +22,7 @@ async def get_current_user(
 ) -> User:
     try:
         return await user_service.get_user_from_token(token=token)
-    except PermissionError:
+    except AccessDenied:
         raise HTTPException(
             status_code=status.HTTP_401_UNAUTHORIZED,
             detail="Invalid authentication credentials",

abrechnung/http/routers/auth.py

@@ -3,6 +3,7 @@
 from fastapi import APIRouter, Depends, HTTPException, status
 from fastapi.security import OAuth2PasswordRequestForm
 from pydantic import BaseModel, EmailStr
+from sftkit.error import AccessDenied
 
 from abrechnung.application.users import InvalidPassword, UserService
 from abrechnung.config import Config
@@ -256,7 +257,7 @@ async def confirm_password_recovery(
 ):
     try:
         await user_service.confirm_password_recovery(token=payload.token, new_password=payload.new_password)
-    except PermissionError as e:
+    except AccessDenied as e:
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(e))
 
 

frontend/apps/web/src/core/api.ts

@@ -1,10 +1,11 @@
-import { Api, IConnectionStatusProvider } from "@abrechnung/api";
+import { Api, ApiError, IConnectionStatusProvider } from "@abrechnung/api";
 import {
     fetchBaseQuery,
     type BaseQueryFn,
     type FetchArgs,
     type FetchBaseQueryError,
 } from "@reduxjs/toolkit/query/react";
+import { toast } from "react-toastify";
 
 export const siteHost = window.location.host;
 export const baseURL = `${window.location.protocol}//${siteHost}`;
@@ -34,3 +35,11 @@ export const apiBaseQuery: BaseQueryFn<string | FetchArgs, unknown, FetchBaseQue
     const rawBaseQuery = fetchBaseQuery({ baseUrl: baseURL, prepareHeaders: prepareAuthHeaders });
     return rawBaseQuery(args, api, extraOptions);
 };
+
+export const handleApiError = (err: ApiError) => {
+    let message = err.name;
+    if (typeof err.body?.message === "string") {
+        message = err.body.message;
+    }
+    toast.error(message);
+};

frontend/apps/web/src/pages/auth/ConfirmEmailChange.tsx

@@ -3,7 +3,7 @@ import React, { useState } from "react";
 import { useParams } from "react-router";
 import { toast } from "react-toastify";
 import { Loading } from "@abrechnung/components";
-import { api } from "@/core/api";
+import { api, handleApiError } from "@/core/api";
 import { useTitle } from "@/core/utils";
 import { Trans, useTranslation } from "react-i18next";
 
@@ -25,9 +25,7 @@ export const ConfirmEmailChange: React.FC = () => {
             .then((value) => {
                 setStatus("success");
             })
-            .catch((error) => {
-                toast.error(error);
-            });
+            .catch(handleApiError);
     };
 
     if (status === "success") {

frontend/apps/web/src/pages/auth/ConfirmPasswordRecovery.tsx

@@ -2,7 +2,7 @@ import { Alert, Box, Button, Container, CssBaseline, Link, Stack, Typography } f
 import React, { useState } from "react";
 import { Link as RouterLink, useParams } from "react-router";
 import { z } from "zod";
-import { api } from "@/core/api";
+import { api, handleApiError } from "@/core/api";
 import i18n from "@/i18n";
 import { Trans, useTranslation } from "react-i18next";
 import { useTitle } from "@/core/utils";
@@ -24,7 +24,6 @@ type FormSchema = z.infer<typeof validationSchema>;
 export const ConfirmPasswordRecovery: React.FC = () => {
     const { t } = useTranslation();
     const [status, setStatus] = useState("idle");
-    const [error, setError] = useState(null);
     const { token } = useParams();
 
     useTitle(t("auth.confirmPasswordRecovery.tabTitle"));
@@ -50,12 +49,11 @@ export const ConfirmPasswordRecovery: React.FC = () => {
             .confirmPasswordRecovery({ requestBody: { new_password: values.password, token } })
             .then(() => {
                 setStatus("success");
-                setError(null);
                 resetForm();
             })
             .catch((err) => {
+                handleApiError(err);
                 setStatus("error");
-                setError(err.toString());
             });
     };
 
@@ -73,11 +71,6 @@ export const ConfirmPasswordRecovery: React.FC = () => {
                 <Typography component="h1" variant="h5">
                     {t("auth.confirmPasswordRecovery.header")}
                 </Typography>
-                {error && (
-                    <Alert sx={{ mt: 4 }} severity="error">
-                        {error}
-                    </Alert>
-                )}
                 {status === "success" ? (
                     <Alert sx={{ mt: 4 }} severity="success">
                         <Trans Key="auth.confirmPasswordRecovery.successfulLinkToLogin">