diff --git a/server/mergin/sync/public_api_controller.py b/server/mergin/sync/public_api_controller.py index c0ec9fb0..b3bf42df 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 @@ -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 diff --git a/server/mergin/sync/public_api_v2_controller.py b/server/mergin/sync/public_api_v2_controller.py index 7fe2bbfa..32c05595 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 b1fa74da..330f58fa 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, @@ -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"""