mirror of
https://github.com/basicmachines-co/basic-memory
synced 2026-06-21 13:47:35 +00:00
fix: Terminate sync immediately when project is deleted (#366)
Signed-off-by: phernandez <paul@basicmachines.co> Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -135,7 +135,20 @@ class EntityRepository(Repository[Entity]):
|
||||
)
|
||||
return found
|
||||
|
||||
except IntegrityError:
|
||||
except IntegrityError as e:
|
||||
# Check if this is a FOREIGN KEY constraint failure
|
||||
error_str = str(e)
|
||||
if "FOREIGN KEY constraint failed" in error_str:
|
||||
# Import locally to avoid circular dependency (repository -> services -> repository)
|
||||
from basic_memory.services.exceptions import SyncFatalError
|
||||
|
||||
# Project doesn't exist in database - this is a fatal sync error
|
||||
raise SyncFatalError(
|
||||
f"Cannot sync file '{entity.file_path}': "
|
||||
f"project_id={entity.project_id} does not exist in database. "
|
||||
f"The project may have been deleted. This sync will be terminated."
|
||||
) from e
|
||||
|
||||
await session.rollback()
|
||||
|
||||
# Re-query after rollback to get a fresh, attached entity
|
||||
|
||||
@@ -20,3 +20,18 @@ class DirectoryOperationError(Exception):
|
||||
"""Raised when directory operations fail"""
|
||||
|
||||
pass
|
||||
|
||||
|
||||
class SyncFatalError(Exception):
|
||||
"""Raised when sync encounters a fatal error that prevents continuation.
|
||||
|
||||
Fatal errors include:
|
||||
- Project deleted during sync (FOREIGN KEY constraint)
|
||||
- Database corruption
|
||||
- Critical system failures
|
||||
|
||||
When this exception is raised, the entire sync operation should be terminated
|
||||
immediately rather than attempting to continue with remaining files.
|
||||
"""
|
||||
|
||||
pass
|
||||
|
||||
@@ -22,6 +22,7 @@ from basic_memory.models import Entity, Project
|
||||
from basic_memory.repository import EntityRepository, RelationRepository, ObservationRepository
|
||||
from basic_memory.repository.search_repository import SearchRepository
|
||||
from basic_memory.services import EntityService, FileService
|
||||
from basic_memory.services.exceptions import SyncFatalError
|
||||
from basic_memory.services.link_resolver import LinkResolver
|
||||
from basic_memory.services.search_service import SearchService
|
||||
from basic_memory.services.sync_status_service import sync_status_tracker, SyncStatus
|
||||
@@ -514,6 +515,13 @@ class SyncService:
|
||||
return entity, checksum
|
||||
|
||||
except Exception as e:
|
||||
# Check if this is a fatal error (or caused by one)
|
||||
# Fatal errors like project deletion should terminate sync immediately
|
||||
if isinstance(e, SyncFatalError) or isinstance(e.__cause__, SyncFatalError):
|
||||
logger.error(f"Fatal sync error encountered, terminating sync: path={path}")
|
||||
raise
|
||||
|
||||
# Otherwise treat as recoverable file-level error
|
||||
error_msg = str(e)
|
||||
logger.error(f"Failed to sync file: path={path}, error={error_msg}")
|
||||
|
||||
|
||||
@@ -6,6 +6,7 @@ from datetime import datetime, timezone
|
||||
from basic_memory.models.knowledge import Entity
|
||||
from basic_memory.repository.entity_repository import EntityRepository
|
||||
from basic_memory.repository.project_repository import ProjectRepository
|
||||
from basic_memory.services.exceptions import SyncFatalError
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@@ -436,3 +437,32 @@ async def test_upsert_entity_permalink_conflict_within_project_only(session_make
|
||||
assert result1.project_id == project1.id
|
||||
assert result2.project_id == project2.id
|
||||
assert result3.project_id == project1.id
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_upsert_entity_with_invalid_project_id(entity_repository: EntityRepository):
|
||||
"""Test that upserting with non-existent project_id raises clear error.
|
||||
|
||||
This tests the fix for issue #188 where sync fails with FOREIGN KEY constraint
|
||||
violations when a project is deleted during sync operations.
|
||||
"""
|
||||
# Create entity with non-existent project_id
|
||||
entity = Entity(
|
||||
title="Test Entity",
|
||||
entity_type="note",
|
||||
file_path="test.md",
|
||||
permalink="test",
|
||||
project_id=99999, # This project doesn't exist
|
||||
content_type="text/markdown",
|
||||
created_at=datetime.now(timezone.utc),
|
||||
updated_at=datetime.now(timezone.utc),
|
||||
)
|
||||
|
||||
# Should raise SyncFatalError with clear message about missing project
|
||||
with pytest.raises(SyncFatalError) as exc_info:
|
||||
await entity_repository.upsert_entity(entity)
|
||||
|
||||
error_msg = str(exc_info.value)
|
||||
assert "project_id=99999 does not exist" in error_msg
|
||||
assert "project may have been deleted" in error_msg.lower()
|
||||
assert "sync will be terminated" in error_msg.lower()
|
||||
|
||||
@@ -1565,3 +1565,79 @@ async def test_circuit_breaker_handles_checksum_computation_failure(
|
||||
failure_info = sync_service._file_failures["checksum_fail.md"]
|
||||
assert failure_info.count == 1
|
||||
assert failure_info.last_checksum == "" # Empty when checksum fails
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_fatal_error_terminates_sync_immediately(
|
||||
sync_service: SyncService, project_config: ProjectConfig, entity_service: EntityService
|
||||
):
|
||||
"""Test that SyncFatalError terminates sync immediately without circuit breaker retry.
|
||||
|
||||
This tests the fix for issue #188 where project deletion during sync should
|
||||
terminate immediately rather than retrying each file 3 times.
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
from basic_memory.services.exceptions import SyncFatalError
|
||||
|
||||
project_dir = project_config.home
|
||||
|
||||
# Create multiple test files
|
||||
await create_test_file(
|
||||
project_dir / "file1.md",
|
||||
dedent(
|
||||
"""
|
||||
---
|
||||
type: knowledge
|
||||
---
|
||||
# File 1
|
||||
Content 1
|
||||
"""
|
||||
),
|
||||
)
|
||||
await create_test_file(
|
||||
project_dir / "file2.md",
|
||||
dedent(
|
||||
"""
|
||||
---
|
||||
type: knowledge
|
||||
---
|
||||
# File 2
|
||||
Content 2
|
||||
"""
|
||||
),
|
||||
)
|
||||
await create_test_file(
|
||||
project_dir / "file3.md",
|
||||
dedent(
|
||||
"""
|
||||
---
|
||||
type: knowledge
|
||||
---
|
||||
# File 3
|
||||
Content 3
|
||||
"""
|
||||
),
|
||||
)
|
||||
|
||||
# Mock entity_service.create_entity_from_markdown to raise SyncFatalError on first file
|
||||
# This simulates project being deleted during sync
|
||||
async def mock_create_entity_from_markdown(*args, **kwargs):
|
||||
raise SyncFatalError(
|
||||
"Cannot sync file 'file1.md': project_id=99999 does not exist in database. "
|
||||
"The project may have been deleted. This sync will be terminated."
|
||||
)
|
||||
|
||||
with patch.object(
|
||||
entity_service, "create_entity_from_markdown", side_effect=mock_create_entity_from_markdown
|
||||
):
|
||||
# Sync should raise SyncFatalError and terminate immediately
|
||||
with pytest.raises(SyncFatalError, match="project_id=99999 does not exist"):
|
||||
await sync_service.sync(project_dir)
|
||||
|
||||
# Verify that circuit breaker did NOT record this as a file-level failure
|
||||
# (SyncFatalError should bypass circuit breaker and re-raise immediately)
|
||||
assert "file1.md" not in sync_service._file_failures
|
||||
|
||||
# Verify that no other files were attempted (sync terminated on first error)
|
||||
# If circuit breaker was used, we'd see file1 in failures
|
||||
# If sync continued, we'd see attempts for file2 and file3
|
||||
|
||||
Reference in New Issue
Block a user