Merge pull request #209 from MerginMaps/improve-client-error

Improve ClientError  handling

Author: MarcelGeo

.github/workflows/autotests.yml

@@ -1,7 +1,7 @@
 name: Auto Tests
 on: [push]
 env:
-  TEST_MERGIN_URL: https://test.dev.merginmaps.com/
+  TEST_MERGIN_URL: https://app.dev.merginmaps.com/
   TEST_API_USERNAME: test_plugin
   TEST_API_PASSWORD: ${{ secrets.MERGINTEST_API_PASSWORD }}
   TEST_API_USERNAME2: test_plugin2

mergin/client.py

@@ -17,7 +17,7 @@
 import typing
 import warnings
 
-from .common import ClientError, LoginError, InvalidProject
+from .common import ClientError, LoginError, InvalidProject, ErrorCode
 from .merginproject import MerginProject
 from .client_pull import (
     download_file_finalize,
@@ -205,19 +205,20 @@ def _do_request(self, request):
         try:
             return self.opener.open(request)
         except urllib.error.HTTPError as e:
-            if e.headers.get("Content-Type", "") == "application/problem+json":
-                info = json.load(e)
-                err_detail = info.get("detail")
-            else:
-                err_detail = e.read().decode("utf-8")
+            server_response = json.load(e)
+
+            # We first to try to get the value from the response otherwise we set a default value
+            err_detail = server_response.get("detail", e.read().decode("utf-8"))
+            server_code = server_response.get("code", None)
 
-            error_msg = (
-                f"HTTP Error: {e.code} {e.reason}\n"
-                f"URL: {request.get_full_url()}\n"
-                f"Method: {request.get_method()}\n"
-                f"Detail: {err_detail}"
+            raise ClientError(
+                detail=err_detail,
+                url=request.get_full_url(),
+                server_code=server_code,
+                server_response=server_response,
+                http_error=e.code,
+                http_method=request.get_method(),
             )
-            raise ClientError(error_msg)
         except urllib.error.URLError as e:
             # e.g. when DNS resolution fails (no internet connection?)
             raise ClientError("Error requesting " + request.full_url + ": " + str(e))
@@ -429,9 +430,9 @@ def create_workspace(self, workspace_name):
 
         try:
             self.post("/v1/workspace", params, {"Content-Type": "application/json"})
-        except Exception as e:
-            detail = f"Workspace name: {workspace_name}"
-            raise ClientError(str(e), detail)
+        except ClientError as e:
+            e.extra = f"Workspace name: {workspace_name}"
+            raise e
 
     def create_project(self, project_name, is_public=False, namespace=None):
         """
@@ -478,9 +479,9 @@ def create_project(self, project_name, is_public=False, namespace=None):
             namespace = self.username()
         try:
             self.post(f"/v1/project/{namespace}", params, {"Content-Type": "application/json"})
-        except Exception as e:
-            detail = f"Namespace: {namespace}, project name: {project_name}"
-            raise ClientError(str(e), detail)
+        except ClientError as e:
+            e.extra = f"Namespace: {namespace}, project name: {project_name}"
+            raise e
 
     def create_project_and_push(self, project_name, directory, is_public=False, namespace=None):
         """
@@ -776,9 +777,9 @@ def set_project_access(self, project_path, access):
         try:
             request = urllib.request.Request(url, data=json.dumps(params).encode(), headers=json_headers, method="PUT")
             self._do_request(request)
-        except Exception as e:
-            detail = f"Project path: {project_path}"
-            raise ClientError(str(e), detail)
+        except ClientError as e:
+            e.extra = f"Project path: {project_path}"
+            raise e
 
     def add_user_permissions_to_project(self, project_path, usernames, permission_level):
         """

mergin/common.py

@@ -1,5 +1,5 @@
 import os
-
+from enum import Enum
 
 CHUNK_SIZE = 100 * 1024 * 1024
 
@@ -10,8 +10,35 @@
 this_dir = os.path.dirname(os.path.realpath(__file__))
 
 
+# Error code from the public API, add to the end of enum as we handle more eror
+class ErrorCode(Enum):
+    ProjectsLimitHit = "ProjectsLimitHit"
+    StorageLimitHit = "StorageLimitHit"
+
+
 class ClientError(Exception):
-    pass
+    def __init__(self, detail, url=None, server_code=None, server_response=None, http_error=None, http_method=None):
+        self.detail = detail
+        self.url = url
+        self.http_error = http_error
+        self.http_method = http_method
+
+        self.server_code = server_code
+        self.server_response = server_response
+
+        self.extra = None
+
+    def __str__(self):
+        string_res = f"Detail: {self.detail}\n"
+        if self.http_error:
+            string_res += f"HTTP Error: {self.http_error}\n"
+        if self.url:
+            string_res += f"URL: {self.url}\n"
+        if self.http_method:
+            string_res += f"Method: {self.http_method}\n"
+        if self.extra:
+            string_res += f"{self.extra}\n"
+        return string_res
 
 
 class LoginError(Exception):

mergin/test/test_client.py

@@ -19,6 +19,7 @@
     decode_token_data,
     TokenError,
     ServerType,
+    ErrorCode,
 )
 from ..client_push import push_project_async, push_project_cancel
 from ..client_pull import (
@@ -796,7 +797,7 @@ def test_available_storage_validation(mcStorage):
     assert got_right_err
 
     # Expecting empty project
-    project_info = get_project_info(mcStorage, API_USER, test_project)
+    project_info = get_project_info(mcStorage, STORAGE_WORKSPACE, test_project)
     assert project_info["version"] == "v0"
     assert project_info["disk_usage"] == 0
 
@@ -2629,3 +2630,34 @@ def test_editor_push(mc: MerginClient, mc2: MerginClient):
             conflicted_file = project_file
     # There is no conflicted qgs file
     assert conflicted_file is None
+
+
+def test_error_push_already_named_project(mc: MerginClient):
+    test_project = "test_push_already_existing"
+    project = API_USER + "/" + test_project
+    project_dir = os.path.join(TMP_DIR, test_project)
+
+    with pytest.raises(ClientError) as e:
+        mc.create_project_and_push(test_project, project_dir)
+    assert e.value.detail == "Project with the same name already exists"
+    assert e.value.http_error == 409
+    assert e.value.http_method == "POST"
+    assert e.value.url == f"{mc.url}v1/project/test_plugin"
+
+
+def test_error_projects_limit_hit(mcStorage: MerginClient):
+    test_project = "test_another_project_above_projects_limit"
+    test_project_fullname = STORAGE_WORKSPACE + "/" + test_project
+
+    project_dir = os.path.join(TMP_DIR, test_project, API_USER)
+
+    with pytest.raises(ClientError) as e:
+        mcStorage.create_project_and_push(test_project_fullname, project_dir)
+    assert e.value.server_code == ErrorCode.ProjectsLimitHit.value
+    assert (
+        e.value.detail
+        == "Maximum number of projects is reached. Please upgrade your subscription to create new projects (ProjectsLimitHit)"
+    )
+    assert e.value.http_error == 422
+    assert e.value.http_method == "POST"
+    assert e.value.url == f"{mcStorage.url}v1/project/testpluginstorage"

mergin/utils.py

@@ -275,3 +275,22 @@ def is_mergin_config(path: str) -> bool:
     """Check if the given path is for file mergin-config.json"""
     filename = os.path.basename(path).lower()
     return filename == "mergin-config.json"
+
+
+def bytes_to_human_size(bytes: int):
+    """
+    Convert bytes to human readable size
+    example  :
+    bytes_to_human_size(5600000) -> "5.3 MB"
+    """
+    precision = 1
+    if bytes < 1e-5:
+        return "0.0 MB"
+    elif bytes < 1024.0 * 1024.0:
+        return f"{round( bytes / 1024.0, precision )} KB"
+    elif bytes < 1024.0 * 1024.0 * 1024.0:
+        return f"{round( bytes / 1024.0 / 1024.0, precision)} MB"
+    elif bytes < 1024.0 * 1024.0 * 1024.0 * 1024.0:
+        return f"{round( bytes / 1024.0 / 1024.0 / 1024.0, precision )} GB"
+    else:
+        return f"{round( bytes / 1024.0 / 1024.0 / 1024.0 / 1024.0, precision )} TB"