Fix integrity error handling

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
 
@@ -729,15 +731,16 @@ def wrapper(*args, **kwargs):
             ):
                 error_type = "project_push"
 
-            if not e.description:  # custom error cases (e.g. StorageLimitHit)
-                e.description = e.response.json["detail"]
+            description = (
+                e.description if e.description else e.response.json.get("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")
-
             raise
 
     return wrapper

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:

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,
@@ -339,7 +340,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",
@@ -361,6 +362,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"""