From f5bd99603dbb6212b5d30615f7dbe6b40066ada6 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Wed, 15 Apr 2026 17:58:05 +0300 Subject: [PATCH 1/2] fix logging and emails --- notifications/file_event_notifications.py | 6 +++--- osf_tests/test_archiver.py | 13 +++++++++++-- tests/test_events.py | 1 + website/archiver/listeners.py | 2 +- website/archiver/tasks.py | 16 +++++++++++++--- website/archiver/utils.py | 15 +++++++++++++++ 6 files changed, 44 insertions(+), 9 deletions(-) diff --git a/notifications/file_event_notifications.py b/notifications/file_event_notifications.py index d57d4d8e9cf..da2ab78431d 100644 --- a/notifications/file_event_notifications.py +++ b/notifications/file_event_notifications.py @@ -275,7 +275,7 @@ def perform(self): 'user_fullname': self.user.fullname, 'message': self.html_message, 'profile_image_url': self.profile_image_url, - 'localized_timestamp': self.timestamp, + 'localized_timestamp': str(self.timestamp), 'url': self.url, }, is_digest=True, @@ -312,7 +312,7 @@ def perform(self): 'user_fullname': self.user.fullname, 'message': self.html_message, 'profile_image_url': self.profile_image_url, - 'localized_timestamp': self.timestamp, + 'localized_timestamp': str(self.timestamp), 'url': self.url, }, is_digest=True, @@ -332,7 +332,7 @@ def perform(self): 'user_fullname': self.user.fullname, 'message': self.html_message, 'profile_image_url': self.profile_image_url, - 'localized_timestamp': self.timestamp, + 'localized_timestamp': str(self.timestamp), 'url': self.url, }, is_digest=True, diff --git a/osf_tests/test_archiver.py b/osf_tests/test_archiver.py index 1987c1088ee..c139799d035 100644 --- a/osf_tests/test_archiver.py +++ b/osf_tests/test_archiver.py @@ -16,6 +16,7 @@ ARCHIVER_INITIATED, ) from website.archiver import utils as archiver_utils +from website.archiver import tasks as archiver_tasks from website.app import * # noqa: F403 from website.archiver import listeners from website.archiver.tasks import * # noqa: F403 @@ -458,6 +459,15 @@ def test_stat_addon(self): assert res.target_name == 'osfstorage' assert res.disk_usage == 128 + 256 + def test_compact_traceback_uses_last_lines(self): + traceback_text = '\n'.join(f'line {line_num}' for line_num in range(50)) + compact = archiver_tasks._compact_traceback(traceback_text, max_lines=5, max_chars=1000) + + assert compact == '\n'.join(f'line {line_num}' for line_num in range(45, 50)) + + def test_compact_traceback_handles_empty(self): + assert archiver_tasks._compact_traceback(None) is None + @mock.patch('website.archiver.tasks.archive_addon.delay') def test_archive_node_pass(self, mock_archive_addon): settings.MAX_ARCHIVE_SIZE = 1024 ** 3 @@ -959,8 +969,7 @@ def test_archive_callback_done_errors(self): assert call_args[1] == self.src assert call_args[2] == self.dst assert call_args[3] == self.user - assert call_args[3] == self.user - assert list(call_args[4]) == list(self.dst.archive_job.target_addons.all()) + assert call_args[4] == self.dst.archive_job.target_info() def test_archive_callback_updates_archiving_state_when_done(self): proj = factories.NodeFactory() diff --git a/tests/test_events.py b/tests/test_events.py index b73e55882ba..532bceff807 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -450,6 +450,7 @@ def test_user_performing_action_no_email(self): self.event.perform() assert len(notifications['emits']) == 1 assert notifications['emits'][0]['type'] == NotificationTypeEnum.ADDON_FILE_COPIED + assert isinstance(notifications['emits'][0]['kwargs']['event_context']['localized_timestamp'], str) class TestSubscriptionManipulations(OsfTestCase): diff --git a/website/archiver/listeners.py b/website/archiver/listeners.py index 6dd4f2dbd1b..d0f8f3b3c03 100644 --- a/website/archiver/listeners.py +++ b/website/archiver/listeners.py @@ -53,7 +53,7 @@ def archive_callback(dst): root.registered_from, root, root.registered_user, - dst.archive_job.target_addons.all(), + dst.archive_job.target_info(), ) diff --git a/website/archiver/tasks.py b/website/archiver/tasks.py index 197b41b886b..407585527e2 100644 --- a/website/archiver/tasks.py +++ b/website/archiver/tasks.py @@ -59,6 +59,7 @@ def create_app_context(): logger = get_task_logger(__name__) + class ArchiverSizeExceeded(Exception): def __init__(self, result): @@ -88,12 +89,13 @@ class ArchiverTask(celery.Task): def on_failure(self, exc, task_id, args, kwargs, einfo): job = ArchiveJob.load(kwargs.get('job_pk')) + compact_traceback = utils.compact_traceback(einfo) if not job: archiver_state_exc = ArchiverStateError({ 'exception': exc, 'args': args, 'kwargs': kwargs, - 'einfo': einfo, + 'einfo': compact_traceback, }) sentry.log_exception(archiver_state_exc) raise archiver_state_exc @@ -102,7 +104,7 @@ def on_failure(self, exc, task_id, args, kwargs, einfo): # already captured return - src, dst, user = job.info() + src, dst, _ = job.info() errors = [] if isinstance(exc, ArchiverSizeExceeded): dst.archive_status = ARCHIVER_SIZE_EXCEEDED @@ -122,15 +124,23 @@ def on_failure(self, exc, task_id, args, kwargs, einfo): } else: dst.archive_status = ARCHIVER_UNCAUGHT_ERROR - errors = [str(einfo)] if einfo else [] + errors = [f'{exc.__class__.__name__}: {exc}'] + if compact_traceback: + errors.append(f'Traceback tail:\n{compact_traceback}') dst.save() + # Always capture full exception; keep log_message payload compact. + sentry.log_exception(exc) sentry.log_message( f'An error occurred while archiving node: {src._id} and registration: {dst._id}', extra_data={ 'source node guid': src._id, 'registration node guid': dst._id, 'task_id': task_id, + 'task_name': self.name, + 'exception_type': exc.__class__.__name__, + 'exception_message': str(exc), + 'traceback_tail': compact_traceback, 'errors': errors, }, ) diff --git a/website/archiver/utils.py b/website/archiver/utils.py index f124388423c..a2b0a9d5778 100644 --- a/website/archiver/utils.py +++ b/website/archiver/utils.py @@ -384,3 +384,18 @@ def _validate_updated_responses(registration, file_input_qids, updated_responses registration=registration, missing_files=missing_responses ) + + +def compact_traceback(einfo, max_lines=25, max_chars=4000): + """Return a short traceback tail for logs/notifications.""" + if not einfo: + return None + traceback_text = str(einfo) + if not traceback_text: + return None + + lines = traceback_text.splitlines() + compact = '\n'.join(lines[-max_lines:]) + if len(compact) > max_chars: + compact = compact[-max_chars:] + return compact From ae9adba8922c7c3823008efd8d3f8774a056e0d9 Mon Sep 17 00:00:00 2001 From: Anton Krytskyi Date: Wed, 15 Apr 2026 18:36:42 +0300 Subject: [PATCH 2/2] fix tests --- osf_tests/test_archiver.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/osf_tests/test_archiver.py b/osf_tests/test_archiver.py index c139799d035..5b67375a5a2 100644 --- a/osf_tests/test_archiver.py +++ b/osf_tests/test_archiver.py @@ -16,7 +16,6 @@ ARCHIVER_INITIATED, ) from website.archiver import utils as archiver_utils -from website.archiver import tasks as archiver_tasks from website.app import * # noqa: F403 from website.archiver import listeners from website.archiver.tasks import * # noqa: F403 @@ -461,12 +460,12 @@ def test_stat_addon(self): def test_compact_traceback_uses_last_lines(self): traceback_text = '\n'.join(f'line {line_num}' for line_num in range(50)) - compact = archiver_tasks._compact_traceback(traceback_text, max_lines=5, max_chars=1000) + compact = archiver_utils.compact_traceback(traceback_text, max_lines=5, max_chars=1000) assert compact == '\n'.join(f'line {line_num}' for line_num in range(45, 50)) def test_compact_traceback_handles_empty(self): - assert archiver_tasks._compact_traceback(None) is None + assert archiver_utils.compact_traceback(None) is None @mock.patch('website.archiver.tasks.archive_addon.delay') def test_archive_node_pass(self, mock_archive_addon):