From ec80700a8db8c2c22dca626af2828001eca7ba9f Mon Sep 17 00:00:00 2001 From: "marcel.kocisek" Date: Wed, 13 Aug 2025 17:32:12 +0200 Subject: [PATCH 1/2] resolveunhandled description --- server/mergin/sync/public_api_controller.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index ab0908d1..3d6793e3 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -728,12 +728,19 @@ def wrapper(*args, **kwargs): == "/v2.mergin_sync_public_api_v2_controller_create_project_version" ): error_type = "project_push" + description = "" + if isinstance(e, IntegrityError): + description = "Database integrity error" + else: + description = ( + e.description + if e.description + else e.response.json.get("detail", "") + ) - if not e.description: # custom error cases (e.g. StorageLimitHit) - e.description = e.response.json["detail"] if project: project.sync_failed( - user_agent, error_type, str(e.description), current_user.id + user_agent, error_type, str(description), current_user.id ) else: logging.warning("Missing project info in sync failure") From ea4d51d558246adcd269344c9f4e2a22ca27f5e3 Mon Sep 17 00:00:00 2001 From: Martin Varga Date: Fri, 15 Aug 2025 13:47:50 +0200 Subject: [PATCH 2/2] Fix integrity error handling in push --- server/mergin/sync/public_api_controller.py | 20 ++++++--------- .../mergin/sync/public_api_v2_controller.py | 13 +++++----- server/mergin/tests/test_public_api_v2.py | 25 +++++++++++++++++-- 3 files changed, 38 insertions(+), 20 deletions(-) diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index 3d6793e3..8eaf59b3 100644 --- a/server/mergin/sync/public_api_controller.py +++ b/server/mergin/sync/public_api_controller.py @@ -32,7 +32,7 @@ from gevent import sleep import base64 -from werkzeug.exceptions import HTTPException +from werkzeug.exceptions import HTTPException, Conflict from mergin.sync.forms import project_name_validation from .interfaces import WorkspaceRole @@ -707,7 +707,9 @@ def wrapper(*args, **kwargs): if status_code >= 400: raise HTTPException(response=response) return response, status_code - except (HTTPException, IntegrityError) as e: + except IntegrityError: + raise Conflict("Database integrity error") + except HTTPException as e: if e.code in [401, 403, 404]: raise # nothing to do, just propagate downstream @@ -728,15 +730,10 @@ def wrapper(*args, **kwargs): == "/v2.mergin_sync_public_api_v2_controller_create_project_version" ): error_type = "project_push" - description = "" - if isinstance(e, IntegrityError): - description = "Database integrity error" - else: - description = ( - e.description - if e.description - else e.response.json.get("detail", "") - ) + + description = ( + e.description if e.description else e.response.json.get("detail", "") + ) if project: project.sync_failed( @@ -744,7 +741,6 @@ def wrapper(*args, **kwargs): ) else: logging.warning("Missing project info in sync failure") - raise return wrapper diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index e503edad..8a83f808 100644 --- a/server/mergin/sync/public_api_v2_controller.py +++ b/server/mergin/sync/public_api_v2_controller.py @@ -12,7 +12,7 @@ from flask import abort, jsonify, current_app from flask_login import current_user from marshmallow import ValidationError -from psycopg2 import IntegrityError +from sqlalchemy.exc import IntegrityError from ..app import db from ..auth import auth_required @@ -235,10 +235,10 @@ def create_project_version(id): if request.json.get("check_only", False): return NoContent, 204 - # while processing data, block other uploads - upload = Upload(project, version, upload_changes, current_user.id) - db.session.add(upload) try: + # while processing data, block other uploads + upload = Upload(project, version, upload_changes, current_user.id) + db.session.add(upload) # Creating blocking upload can fail, e.g. in case of racing condition db.session.commit() except IntegrityError: @@ -257,9 +257,10 @@ def create_project_version(id): current_user.id, ) - # Try again after cleanup - db.session.add(upload) try: + # Try again after cleanup + upload = Upload(project, version, upload_changes, current_user.id) + db.session.add(upload) db.session.commit() move_to_tmp(upload.upload_dir) except IntegrityError as err: diff --git a/server/mergin/tests/test_public_api_v2.py b/server/mergin/tests/test_public_api_v2.py index 1d5e0d7c..51f465c8 100644 --- a/server/mergin/tests/test_public_api_v2.py +++ b/server/mergin/tests/test_public_api_v2.py @@ -4,7 +4,7 @@ import os import shutil from unittest.mock import patch -from psycopg2 import IntegrityError +from sqlalchemy.exc import IntegrityError import pytest from datetime import datetime, timedelta, timezone @@ -18,6 +18,7 @@ StorageLimitHit, UploadError, ) +from mergin.sync.files import ChangesSchema from mergin.sync.models import ( Project, ProjectRole, @@ -335,7 +336,7 @@ def test_create_version_failures(client): project.locked_until = None db.session.commit() - # try to finish the transaction which would fail on version created integrity error, e.g. race conditions + # try to finish the transaction which would fail on storage limit with patch.object( Configuration, "GLOBAL_STORAGE", @@ -357,6 +358,26 @@ def test_create_version_failures(client): assert response.status_code == 422 assert response.json["code"] == UploadError.code + # try to finish the transaction which would fail on existing Upload integrity error, e.g. race conditions + with patch.object( + Upload, + "__init__", + side_effect=IntegrityError("Cannot insert upload", None, None), + ): + response = client.post(f"v2/projects/{project.id}/versions", json=data) + assert response.status_code == 409 + assert response.json["code"] == AnotherUploadRunning.code + + # try to finish the transaction which would fail on unexpected integrity error + # patch of ChangesSchema is just a workaround to trigger and error + with patch.object( + ChangesSchema, + "validate", + side_effect=IntegrityError("Cannot insert upload", None, None), + ): + response = client.post(f"v2/projects/{project.id}/versions", json=data) + assert response.status_code == 409 + def test_upload_chunk(client): """Test pushing a chunk to a project"""