From c09ff9866f72aab3d49a697729a2ac97f410b633 Mon Sep 17 00:00:00 2001 From: pwei1018 Date: Wed, 18 Feb 2026 09:26:34 -0800 Subject: [PATCH] fix: Prevent `ObjectDeletedError` by caching notification response data directly on the object before session expiry. --- .../src/notify_api/resources/v1/notify.py | 16 ++++++-------- .../src/notify_api/services/notify_service.py | 21 +++++++++++++------ .../unit/services/test_notify_service.py | 11 ++++++---- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/notify-service/notify-api/src/notify_api/resources/v1/notify.py b/notify-service/notify-api/src/notify_api/resources/v1/notify.py index 01f01892..d130b8b5 100644 --- a/notify-service/notify-api/src/notify_api/resources/v1/notify.py +++ b/notify-service/notify-api/src/notify_api/resources/v1/notify.py @@ -34,19 +34,15 @@ def send_notification(body: NotificationRequest): """Create and send EMAIL notification endpoint.""" body.notify_type = Notification.NotificationType.EMAIL notification = notify.queue_publish(body) - # Eagerly build response dict to avoid ObjectDeletedError if the - # delivery service processes and deletes the row before we respond. + # Eagerly build response dict to avoid ObjectDeletedError / + # DetachedInstanceError if the delivery service processes and deletes + # the row (or the session expires attributes) before we respond. try: response = notification.json except Exception: - # Fallback if the notification row was already deleted - response = { - "id": getattr(notification, "id", None), - "recipients": getattr(notification, "recipients", None), - "notifyStatus": getattr(notification.status_code, "name", None) - if hasattr(notification, "status_code") and notification.status_code - else None, - } + # Use the cached response dict that was captured while attributes + # were still fresh (set in _process_single_recipient). + response = getattr(notification, "_cached_response", {"id": None, "notifyStatus": "QUEUED"}) return jsonify(response), HTTPStatus.OK diff --git a/notify-service/notify-api/src/notify_api/services/notify_service.py b/notify-service/notify-api/src/notify_api/services/notify_service.py index 940983c9..c8219c98 100644 --- a/notify-service/notify-api/src/notify_api/services/notify_service.py +++ b/notify-service/notify-api/src/notify_api/services/notify_service.py @@ -28,7 +28,6 @@ NotificationRequest, SafeList, ) -from notify_api.models.db import db from notify_api.services.gcp_queue import GcpQueue, queue logger = StructuredLogging.get_logger() @@ -355,7 +354,13 @@ def _process_single_recipient( try: # Create notification record notification = Notification.create_notification(notification_request, recipient, provider) - notification_data["notificationId"] = notification.id + + # Capture notification ID while attributes are fresh (before any + # commit expires them). Store a cached response dict so the API + # endpoint can return it even if the row is later deleted or the + # session state is expired/detached. + cached_id = notification.id + notification_data["notificationId"] = cached_id # Create and publish cloud event cloud_event = NotifyService._create_cloud_event(provider, notification_data) @@ -369,10 +374,14 @@ def _process_single_recipient( # Update notification status NotifyService._update_notification_status(notification, provider, Notification.NotificationStatus.QUEUED) - # Expunge from session so accessing attributes later won't trigger - # a lazy-load SELECT (which would fail if the delivery service - # already processed and deleted the row). - db.session.expunge(notification) + # Attach a cached response so the caller can safely build a JSON + # reply without touching SQLAlchemy-managed attributes (which may + # be expired or belong to a deleted row by now). + notification._cached_response = { + "id": cached_id, + "recipients": recipient, + "notifyStatus": Notification.NotificationStatus.QUEUED.name, + } return notification diff --git a/notify-service/notify-api/tests/unit/services/test_notify_service.py b/notify-service/notify-api/tests/unit/services/test_notify_service.py index 2e75f204..23915774 100644 --- a/notify-service/notify-api/tests/unit/services/test_notify_service.py +++ b/notify-service/notify-api/tests/unit/services/test_notify_service.py @@ -383,17 +383,18 @@ def test_queue_publish_exception(app): assert result.status_code == Notification.NotificationStatus.FAILURE @staticmethod - @patch("notify_api.services.notify_service.db") @patch("notify_api.services.notify_service.queue") @patch("notify_api.services.notify_service.GcpQueue") @patch("notify_api.services.notify_service.Notification") - def test_process_single_recipient_success(mock_notification_class, mock_gcp_queue, mock_queue, mock_db): + def test_process_single_recipient_success(mock_notification_class, mock_gcp_queue, mock_queue): """Test successful single recipient processing.""" # Setup mocks mock_notification = Mock() mock_notification.id = "test-notification-id" mock_notification_class.create_notification.return_value = mock_notification - mock_notification_class.NotificationStatus.QUEUED = "QUEUED" + mock_queued_status = Mock() + mock_queued_status.name = "QUEUED" + mock_notification_class.NotificationStatus.QUEUED = mock_queued_status mock_gcp_queue.to_queue_message.return_value = "test-queue-message" mock_queue.publish.return_value = "test-future" @@ -418,7 +419,9 @@ def test_process_single_recipient_success(mock_notification_class, mock_gcp_queu mock_request, "test@example.com", "GC_NOTIFY" ) mock_update_status.assert_called_once() - mock_db.session.expunge.assert_called_once_with(mock_notification) + # Verify cached response is set for safe access after session expiry + assert result._cached_response["id"] == "test-notification-id" + assert result._cached_response["recipients"] == "test@example.com" @staticmethod @patch("notify_api.services.notify_service.queue")