Skip to content

Fix error handling _do_request() #248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions mergin/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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))
Expand Down
27 changes: 26 additions & 1 deletion mergin/test/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
decode_token_data,
TokenError,
ServerType,
WorkspaceRole,
)
from ..client_push import push_project_async, push_project_cancel
from ..client_pull import (
Expand Down Expand Up @@ -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/[email protected]")

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("[email protected]", "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