From bb0a39a60502688a9962876aac91f19e20900854 Mon Sep 17 00:00:00 2001
From: Ralf Grubenmann <ralf.grubenmann@sdsc.ethz.ch>
Date: Fri, 28 Apr 2023 13:05:00 +0200
Subject: [PATCH] feat(core): raise proper errors when Renku metadata is
 corrupt (#3393)

---
 renku/core/errors.py                     | 14 ++++++++++++-
 renku/infrastructure/database.py         |  5 ++++-
 renku/ui/service/errors.py               | 15 ++++++++++++++
 renku/ui/service/views/error_handlers.py |  4 ++++
 tests/core/metadata/test_database.py     | 25 ++++++++++++++++++++++++
 5 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/renku/core/errors.py b/renku/core/errors.py
index f3f94b7a75..0da28254c1 100644
--- a/renku/core/errors.py
+++ b/renku/core/errors.py
@@ -708,7 +708,19 @@ class NotebookSessionImageNotExistError(RenkuException):
 
 
 class MetadataMergeError(RenkuException):
-    """Raise when merging of metadata failed."""
+    """Raised when merging of metadata failed."""
+
+
+class MetadataCorruptError(RenkuException):
+    """Raised when metadata is corrupt and couldn't be loaded."""
+
+    def __init__(self, path: Union[str, Path]) -> None:
+        message = f"Metadata file '{path}' couldn't be loaded because it is corrupted."
+        with open(path) as f:
+            content = f.read()
+        if all(pattern in content for pattern in ["<<<<<<<", "=======", ">>>>>>>"]):
+            message += "\nThis is likely due to an unresolved git merge conflict in the file."
+        super().__init__(message)
 
 
 class MinimumVersionError(RenkuException):
diff --git a/renku/infrastructure/database.py b/renku/infrastructure/database.py
index fceebb668d..10128e7f02 100644
--- a/renku/infrastructure/database.py
+++ b/renku/infrastructure/database.py
@@ -833,7 +833,10 @@ def load(self, filename: str, absolute: bool = False):
                 with self.zstd_decompressor.stream_reader(file) as zfile:
                     data = json.load(zfile)
             else:
-                data = json.load(file)
+                try:
+                    data = json.load(file)
+                except json.JSONDecodeError:
+                    raise errors.MetadataCorruptError(path)
         return data
 
 
diff --git a/renku/ui/service/errors.py b/renku/ui/service/errors.py
index 5967ea66b4..a8f3e58e20 100644
--- a/renku/ui/service/errors.py
+++ b/renku/ui/service/errors.py
@@ -301,6 +301,21 @@ def __init__(self, exception=None):
         super().__init__(exception=exception)
 
 
+class UserProjectMetadataCorruptError(ServiceError):
+    """The metadata in the project os corrupt and couldn't be loaded."""
+
+    code = SVC_ERROR_USER + 120
+    userMessage = "The Renku metadata in the project is corrupt and couldn't be loaded: {error_message}"
+    devMessage = "Couldn't parse project metadata due to a JSON parsing error: {error_message}"
+
+    def __init__(self, exception=None, error_message=ERROR_NOT_AVAILABLE):
+        super().__init__(
+            userMessage=self.userMessage.format(error_message=error_message),
+            devMessage=self.devMessage.format(error_message=error_message),
+            exception=exception,
+        )
+
+
 class UserDatasetsMultipleImagesError(ServiceError):
     """Multiple images dataset have the same priority."""
 
diff --git a/renku/ui/service/views/error_handlers.py b/renku/ui/service/views/error_handlers.py
index 929e17b648..7751fc60ca 100644
--- a/renku/ui/service/views/error_handlers.py
+++ b/renku/ui/service/views/error_handlers.py
@@ -33,6 +33,7 @@
     GitCommandError,
     GitError,
     InvalidTemplateError,
+    MetadataCorruptError,
     MigrationError,
     MigrationRequired,
     MinimumVersionError,
@@ -73,6 +74,7 @@
     UserNonRenkuProjectError,
     UserOutdatedProjectError,
     UserProjectCreationError,
+    UserProjectMetadataCorruptError,
     UserProjectTemplateReferenceError,
     UserRepoBranchInvalidError,
     UserRepoNoAccessError,
@@ -146,6 +148,8 @@ def decorated_function(*args, **kwargs):
         """Represents decorated function."""
         try:
             return f(*args, **kwargs)
+        except MetadataCorruptError as e:
+            raise UserProjectMetadataCorruptError(e, str(e))
         except MigrationRequired as e:
             raise UserOutdatedProjectError(e)
         except UninitializedProject as e:
diff --git a/tests/core/metadata/test_database.py b/tests/core/metadata/test_database.py
index a3dfe941af..823f237a9a 100644
--- a/tests/core/metadata/test_database.py
+++ b/tests/core/metadata/test_database.py
@@ -21,6 +21,7 @@
 from persistent.list import PersistentList
 from persistent.mapping import PersistentMapping
 
+from renku.core import errors
 from renku.domain_model.entity import Entity
 from renku.domain_model.provenance.activity import Activity, Usage
 from renku.domain_model.workflow.plan import Plan
@@ -414,3 +415,27 @@ def test_database_immutable_object(database):
 
     assert isinstance(usage_1, Usage)
     assert usage_1 is usage_2
+
+
+@pytest.mark.parametrize(
+    "content,error_message",
+    [
+        ("{'a':<' ' b}", ".*file' couldn't be loaded because it is corrupted.$"),
+        (
+            "{'name': \n<<<<<<< HEAD:file\n'a'\n=======\n'b'\n>>>>>>> abcdefg:file\n}",
+            ".*file' couldn't be loaded because it is corrupted.\n"
+            "This is likely due to an unresolved git merge conflict in the file.$",
+        ),
+    ],
+)
+def test_database_corrupt_metadata(tmpdir, content, error_message):
+    """Test raising correct error for corrupt metadata."""
+    from renku.infrastructure.database import Storage
+
+    storage = Storage(tmpdir)
+
+    with open(storage.path / "file", "w") as f:
+        f.write(content)
+
+    with pytest.raises(expected_exception=errors.MetadataCorruptError, match=error_message):
+        storage.load("file")