Improve code quality (#129)

* Reformat json

* Fix security issue 3

* Refactor error handling

* Refactor json error handling

.devcontainer/devcontainer.json

@@ -5,7 +5,10 @@
   },
   "customizations": {
     "codespaces": {
-      "openFiles": ["README.md", "src/pypaperless/api.py"]
+      "openFiles": [
+        "README.md",
+        "src/pypaperless/api.py"
+      ]
     },
     "vscode": {
       "extensions": [
@@ -29,7 +32,9 @@
         "coverage-gutters.showGutterCoverage": true,
         "coverage-gutters.showLineCoverage": true,
         "coverage-gutters.xmlname": "coverage.xml",
-        "python.analysis.extraPaths": ["${workspaceFolder}/src"],
+        "python.analysis.extraPaths": [
+          "${workspaceFolder}/src"
+        ],
         "python.defaultInterpreterPath": ".venv/bin/python",
         "python.formatting.provider": "ruff",
         "python.testing.cwd": "${workspaceFolder}",
@@ -38,7 +43,9 @@
         ],
         "python.testing.pytestEnabled": true,
         "ruff.importStrategy": "fromEnvironment",
-        "ruff.interpreter": [".venv/bin/python"],
+        "ruff.interpreter": [
+          ".venv/bin/python"
+        ],
         "terminal.integrated.defaultProfile.linux": "zsh"
       }
     }

src/pypaperless/api.py

@@ -11,7 +11,7 @@
 
 from . import helpers
 from .const import API_PATH, PaperlessResource
-from .exceptions import BadJsonResponseError, JsonResponseWithError, PaperlessError, RequestError
+from .exceptions import BadJsonResponseError, JsonResponseWithError
 from .models.base import HelperBase
 
 
@@ -198,8 +198,6 @@ async def generate_api_token(
             raise BadJsonResponseError(message) from exc
         except aiohttp.ClientResponseError as exc:
             raise JsonResponseWithError(payload={"error": data}) from exc
-        except Exception as exc:
-            raise exc  # noqa: TRY201
         finally:
             if not external_session:
                 await session.close()
@@ -226,7 +224,7 @@ async def initialize(self) -> None:
             self.logger.debug("Unused features: %s", ", ".join(unused))
 
         if len(missing) > 0:
-            self.logger.warning("Outdated version detected: v%s", self._version)
+            self.logger.warning("Outdated version detected.")
             self.logger.warning("Missing features: %s", ", ".join(missing))
             self.logger.warning("Consider pulling the latest version of Paperless-ngx.")
 
@@ -279,21 +277,16 @@ async def request(  # noqa: PLR0913
         # add base path
         url = f"{self._base_url}{path}" if not path.startswith("http") else path
 
-        try:
-            res = await self._session.request(
-                method=method,
-                url=url,
-                json=json,
-                data=data,
-                params=params,
-                **kwargs,
-            )
-            self.logger.debug("%s (%d): %s", method.upper(), res.status, res.url)
-            yield res
-        except PaperlessError:
-            raise
-        except Exception as exc:  # noqa: BLE001
-            raise RequestError(exc, (method, url, params), kwargs) from None
+        res = await self._session.request(
+            method=method,
+            url=url,
+            json=json,
+            data=data,
+            params=params,
+            **kwargs,
+        )
+        self.logger.debug("%s (%d): %s", method.upper(), res.status, res.url)
+        yield res
 
     async def request_json(
         self,
@@ -303,17 +296,17 @@ async def request_json(
     ) -> Any:
         """Make a request to the api and parse response json to dict."""
         async with self.request(method, endpoint, **kwargs) as res:
+            if res.content_type != "application/json":
+                raise BadJsonResponseError(res)
+
             try:
-                assert res.content_type == "application/json"  # noqa: S101
                 payload = await res.json()
+            except ValueError:
+                raise BadJsonResponseError(res) from None
 
-                if res.status == 400:
-                    raise JsonResponseWithError(payload)  # noqa: TRY301
+            if res.status == 400:
+                raise JsonResponseWithError(payload)
 
-                res.raise_for_status()
-            except (AssertionError, ValueError) as exc:
-                raise BadJsonResponseError(res) from exc
-            except Exception as exc:
-                raise exc  # noqa: TRY201
+            res.raise_for_status()
 
         return payload

src/pypaperless/exceptions.py

@@ -14,25 +14,6 @@ class AuthentificationRequiredError(PaperlessError):
     """Raise when initializing a `Paperless` instance without url/token or session."""
 
 
-class RequestError(PaperlessError):
-    """Raise when issuing a request fails."""
-
-    def __init__(
-        self,
-        exc: Exception,
-        req_args: tuple[str, str, dict[str, str | int] | None],
-        req_kwargs: dict[str, Any] | None,
-    ) -> None:
-        """Initialize a `RequestException` instance."""
-        message = f"Request error: {type(exc).__name__}\n"
-        message += f"URL: {req_args[1]}\n"
-        message += f"Method: {req_args[0].upper()}\n"
-        message += f"params={req_args[2]}\n"
-        message += f"kwargs={req_kwargs}"
-
-        super().__init__(message)
-
-
 class BadJsonResponseError(PaperlessError):
     """Raise when response is no valid json."""
 

tests/test_common.py

@@ -6,7 +6,6 @@
 
 import aiohttp
 import pytest
-from aiohttp.http_exceptions import InvalidURLError
 from aioresponses import aioresponses
 
 from pypaperless import Paperless
@@ -15,7 +14,6 @@
     BadJsonResponseError,
     DraftNotSupportedError,
     JsonResponseWithError,
-    RequestError,
 )
 from pypaperless.models import Page
 from pypaperless.models.base import HelperBase, PaperlessModel
@@ -141,15 +139,6 @@ async def test_request(self, resp: aioresponses) -> None:
         async with api.request("post", PAPERLESS_TEST_URL, form=form_data) as res:
             assert res.status
 
-        # test non-existing request
-        resp.get(
-            PAPERLESS_TEST_URL,
-            exception=InvalidURLError,
-        )
-        with pytest.raises(RequestError):
-            async with api.request("get", PAPERLESS_TEST_URL) as res:
-                pass
-
         # session is still open
         await api.close()
 

tests/test_models_matrix.py

@@ -3,12 +3,13 @@
 import re
 from typing import Any
 
+import aiohttp
 import pytest
 from aioresponses import CallbackResult, aioresponses
 
 from pypaperless import Paperless
 from pypaperless.const import API_PATH
-from pypaperless.exceptions import DraftFieldRequiredError, RequestError
+from pypaperless.exceptions import DraftFieldRequiredError
 from pypaperless.models import Page
 from pypaperless.models.common import PermissionTableType
 
@@ -120,7 +121,7 @@ async def test_call(
             f"{PAPERLESS_TEST_URL}{API_PATH[mapping.resource+'_single']}".format(pk=1337),
             status=404,
         )
-        with pytest.raises(RequestError):
+        with pytest.raises(aiohttp.ClientResponseError):
             await getattr(api_latest, mapping.resource)(1337)
 
 
@@ -213,7 +214,7 @@ async def test_call(
             f"{PAPERLESS_TEST_URL}{API_PATH[mapping.resource+'_single']}".format(pk=1337),
             status=404,
         )
-        with pytest.raises(RequestError):
+        with pytest.raises(aiohttp.ClientResponseError):
             await getattr(api_latest, mapping.resource)(1337)
 
     async def test_create(

tests/test_models_specific.py

@@ -3,6 +3,7 @@
 import datetime
 import re
 
+import aiohttp
 import pytest
 from aioresponses import aioresponses
 
@@ -12,7 +13,6 @@
     AsnRequestError,
     DraftFieldRequiredError,
     PrimaryKeyRequiredError,
-    RequestError,
     TaskNotFoundError,
 )
 from pypaperless.models import (
@@ -62,7 +62,7 @@ async def test_call(self, resp: aioresponses, api_latest: Paperless) -> None:
             f"{PAPERLESS_TEST_URL}{API_PATH['config_single']}".format(pk=1337),
             status=404,
         )
-        with pytest.raises(RequestError):
+        with pytest.raises(aiohttp.ClientResponseError):
             await api_latest.config(1337)
 
 
@@ -417,7 +417,7 @@ async def test_call(self, resp: aioresponses, api_latest: Paperless) -> None:
             f"{PAPERLESS_TEST_URL}{API_PATH['tasks_single']}".format(pk=1337),
             status=404,
         )
-        with pytest.raises(RequestError):
+        with pytest.raises(aiohttp.ClientResponseError):
             await api_latest.tasks(1337)
         # must raise as task_id doesn't exist
         resp.get(