Merge pull request #248 from MerginMaps/fix-error-handling-_do_request

Fix error handling  _do_request()

Author: MarcelGeo

mergin/client.py

@@ -228,25 +228,27 @@ def _do_request(self, request):
         try:
             return self.opener.open(request)
         except urllib.error.HTTPError as e:
-            server_response = json.load(e)
 
             err_detail = None
+            server_response = None
             server_code = None
-            # Try to get error detail
-            if isinstance(server_response, dict):
-                server_code = server_response.get("code")
-                err_detail = server_response.get("detail")
-                if not err_detail:
-                    # Extract all field-specific errors and format them
-                    err_detail = "\n".join(
-                        f"{key}: {', '.join(map(str, value))}"
-                        for key, value in server_response.items()
-                        if isinstance(value, list)
-                    ) or str(
-                        server_response
-                    )  # Fallback to raw response if structure is unexpected
-            else:
-                err_detail = str(server_response)
+
+            if e.fp:
+                server_response = e.fp.read().decode("utf-8")
+                if (
+                    e.headers.get("Content-Type", "") == "application/problem+json"
+                    or e.headers.get("Content-Type", "") == "application/json"
+                ):
+                    json_response = json.loads(server_response)
+                    if isinstance(json_response, dict):
+                        err_detail = json_response.get(
+                            "detail", None
+                        )  # `detail` should be present in MM server response
+                        server_code = json_response.get("code", None)
+                    if err_detail is None:
+                        err_detail = server_response
+                else:
+                    err_detail = server_response
 
             raise ClientError(
                 detail=err_detail,
@@ -256,6 +258,7 @@ def _do_request(self, request):
                 http_error=e.code,
                 http_method=request.get_method(),
             )
+
         except urllib.error.URLError as e:
             # e.g. when DNS resolution fails (no internet connection?)
             raise ClientError("Error requesting " + request.full_url + ": " + str(e))

mergin/test/test_client.py

@@ -20,6 +20,7 @@
     decode_token_data,
     TokenError,
     ServerType,
+    WorkspaceRole,
 )
 from ..client_push import push_project_async, push_project_cancel
 from ..client_pull import (
@@ -2884,5 +2885,29 @@ def test_mc_without_login():
     assert config["server_configured"]
 
     # without login should not be able to access workspaces
-    with pytest.raises(ClientError, match="Authentication information is missing or invalid."):
+    with pytest.raises(ClientError) as e:
         mc.workspaces_list()
+
+    assert e.value.http_error == 401
+    assert e.value.detail == '"Authentication information is missing or invalid."\n'
+
+
+def test_do_request_error_handling(mc: MerginClient):
+
+    with pytest.raises(ClientError) as e:
+        mc.get("/v2/sso/connections?email=bad@email.com")
+
+    assert e.value.http_error == 404
+    assert (
+        e.value.detail
+        == "The requested URL was not found on the server. If you entered the URL manually please check your spelling and try again."
+    )
+    assert ": 404," in e.value.server_response
+
+    workspaces = mc.workspaces_list()
+
+    with pytest.raises(ClientError) as e:
+        mc.create_user("test@email.com", "123", workspace_id=workspaces[0]["id"], workspace_role=WorkspaceRole.GUEST)
+
+    assert e.value.http_error == 400
+    assert "Passwords must be at least 8 characters long." in e.value.detail