From dfd6536f08e956be5bd725064fd1a610ceeaedbb Mon Sep 17 00:00:00 2001
From: Blaine Jester <blainesworld@gmail.com>
Date: Fri, 23 Jun 2023 11:23:02 -0700
Subject: [PATCH] Update change tracker to re-raise; update copy operation to
 log exceptions

---
 .../contentcuration/viewsets/base.py          |  1 +
 .../contentcuration/viewsets/contentnode.py   | 23 ++++++++-----------
 .../contentcuration/viewsets/sync/utils.py    | 10 +++++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/contentcuration/contentcuration/viewsets/base.py b/contentcuration/contentcuration/viewsets/base.py
index d83b3f9e9a..18e1771de0 100644
--- a/contentcuration/contentcuration/viewsets/base.py
+++ b/contentcuration/contentcuration/viewsets/base.py
@@ -960,6 +960,7 @@ def update_progress(progress=None):
         task_object.status = states.FAILURE
         task_object.traceback = traceback.format_exc()
         task_object.save()
+        raise
     finally:
         if task_object.status == states.STARTED:
             # No error reported, cleanup.
diff --git a/contentcuration/contentcuration/viewsets/contentnode.py b/contentcuration/contentcuration/viewsets/contentnode.py
index ca636c95a4..7366d5f28a 100644
--- a/contentcuration/contentcuration/viewsets/contentnode.py
+++ b/contentcuration/contentcuration/viewsets/contentnode.py
@@ -71,7 +71,7 @@
 from contentcuration.viewsets.sync.constants import CREATED
 from contentcuration.viewsets.sync.constants import DELETED
 from contentcuration.viewsets.sync.utils import generate_update_event
-
+from contentcuration.viewsets.sync.utils import log_sync_exception
 
 channel_query = Channel.objects.filter(main_tree__tree_id=OuterRef("tree_id"))
 
@@ -908,9 +908,11 @@ def copy_from_changes(self, changes):
         for copy in changes:
             # Copy change will have key, must also have other attributes, defined in `copy`
             # Just pass as keyword arguments here to let copy do the validation
-            copy_errors = self.copy(copy["key"], **copy)
-            if copy_errors:
-                copy.update({"errors": copy_errors})
+            try:
+                self.copy(copy["key"], **copy)
+            except Exception as e:
+                log_sync_exception(e, user=self.request.user, change=copy)
+                copy["errors"] = [str(e)]
                 errors.append(copy)
         return errors
 
@@ -924,23 +926,18 @@ def copy(
         excluded_descendants=None,
         **kwargs
     ):
-        try:
-            target, position = self.validate_targeting_args(target, position)
-        except ValidationError as e:
-            return str(e)
+        target, position = self.validate_targeting_args(target, position)
 
         try:
             source = self.get_queryset().get(pk=from_key)
         except ContentNode.DoesNotExist:
-            error = ValidationError("Copy source node does not exist")
-            return str(error)
+            raise ValidationError("Copy source node does not exist")
 
         # Affected channel for the copy is the target's channel
         channel_id = target.channel_id
 
         if ContentNode.filter_by_pk(pk=pk).exists():
-            error = ValidationError("Copy pk already exists")
-            return str(error)
+            raise ValidationError("Copy pk already exists")
 
         can_edit_source_channel = ContentNode.filter_edit_queryset(
             ContentNode.filter_by_pk(pk=source.id), user=self.request.user
@@ -969,8 +966,6 @@ def copy(
                 created_by_id=self.request.user.id,
             )
 
-        return None
-
     def perform_create(self, serializer, change=None):
         instance = serializer.save()
 
diff --git a/contentcuration/contentcuration/viewsets/sync/utils.py b/contentcuration/contentcuration/viewsets/sync/utils.py
index fcfaa8a569..f86b9685d8 100644
--- a/contentcuration/contentcuration/viewsets/sync/utils.py
+++ b/contentcuration/contentcuration/viewsets/sync/utils.py
@@ -1,5 +1,7 @@
 import logging
 
+from django.conf import settings
+
 from contentcuration.utils.sentry import report_exception
 from contentcuration.viewsets.sync.constants import ALL_TABLES
 from contentcuration.viewsets.sync.constants import CHANNEL
@@ -92,7 +94,9 @@ def log_sync_exception(e, user=None, change=None, changes=None):
     elif changes is not None:
         contexts["changes"] = changes
 
-    report_exception(e, user=user, contexts=contexts)
+    # in production, we'll get duplicates in Sentry if we log the exception here.
+    if settings.DEBUG:
+        # make sure we leave a record in the logs just in case.
+        logging.exception(e)
 
-    # make sure we leave a record in the logs just in case.
-    logging.exception(e)
+    report_exception(e, user=user, contexts=contexts)