From bc4a3c7557c56d01a4de40ed0f9f68d05a6bcfea Mon Sep 17 00:00:00 2001 From: pacnpal <183241239+pacnpal@users.noreply.github.com> Date: Sun, 4 Jan 2026 18:36:23 -0500 Subject: [PATCH] refactor: Replace direct logger.error calls with `capture_and_log` in accounts services and conditionally pass `error_id` during `ApplicationError` creation. --- backend/apps/accounts/services.py | 5 ++- .../accounts/services/notification_service.py | 3 +- .../services/social_provider_service.py | 14 ++++--- .../services/user_deletion_service.py | 4 +- backend/apps/core/services/error_service.py | 37 +++++++++++-------- backend/apps/core/services/media_service.py | 4 +- 6 files changed, 40 insertions(+), 27 deletions(-) diff --git a/backend/apps/accounts/services.py b/backend/apps/accounts/services.py index 9b098489..02d036eb 100644 --- a/backend/apps/accounts/services.py +++ b/backend/apps/accounts/services.py @@ -26,6 +26,7 @@ from django.utils.crypto import get_random_string from django_forwardemail.services import EmailService from .models import EmailVerification, User, UserDeletionRequest, UserProfile +from apps.core.utils import capture_and_log logger = logging.getLogger(__name__) @@ -130,7 +131,7 @@ class AccountService: html=email_html, ) except Exception as e: - logger.error(f"Failed to send password change confirmation email: {e}") + capture_and_log(e, 'Send password change confirmation email', source='service', severity='medium') @staticmethod def initiate_email_change( @@ -206,7 +207,7 @@ class AccountService: html=email_html, ) except Exception as e: - logger.error(f"Failed to send email verification: {e}") + capture_and_log(e, 'Send email verification', source='service', severity='medium') @staticmethod def verify_email_change(*, token: str) -> dict[str, Any]: diff --git a/backend/apps/accounts/services/notification_service.py b/backend/apps/accounts/services/notification_service.py index 30de90e0..346e80b4 100644 --- a/backend/apps/accounts/services/notification_service.py +++ b/backend/apps/accounts/services/notification_service.py @@ -17,6 +17,7 @@ from django.utils import timezone from django_forwardemail.services import EmailService from apps.accounts.models import NotificationPreference, User, UserNotification +from apps.core.utils import capture_and_log logger = logging.getLogger(__name__) @@ -264,7 +265,7 @@ class NotificationService: logger.info(f"Email notification sent to {user.email} for notification {notification.id}") except Exception as e: - logger.error(f"Failed to send email notification {notification.id}: {str(e)}") + capture_and_log(e, f'Send email notification {notification.id}', source='service') @staticmethod def get_user_notifications( diff --git a/backend/apps/accounts/services/social_provider_service.py b/backend/apps/accounts/services/social_provider_service.py index c7bee3a9..99dd6d1b 100644 --- a/backend/apps/accounts/services/social_provider_service.py +++ b/backend/apps/accounts/services/social_provider_service.py @@ -20,6 +20,8 @@ if TYPE_CHECKING: else: User = get_user_model() +from apps.core.utils import capture_and_log + logger = logging.getLogger(__name__) @@ -62,7 +64,7 @@ class SocialProviderService: return True, "Provider can be safely disconnected." except Exception as e: - logger.error(f"Error checking disconnect permission for user {user.id}, provider {provider}: {e}") + capture_and_log(e, f'Check disconnect permission for user {user.id}, provider {provider}', source='service') return False, "Unable to verify disconnection safety. Please try again." @staticmethod @@ -97,7 +99,7 @@ class SocialProviderService: return connected_providers except Exception as e: - logger.error(f"Error getting connected providers for user {user.id}: {e}") + capture_and_log(e, f'Get connected providers for user {user.id}', source='service') return [] @staticmethod @@ -140,7 +142,7 @@ class SocialProviderService: return available_providers except Exception as e: - logger.error(f"Error getting available providers: {e}") + capture_and_log(e, 'Get available providers', source='service') return [] @staticmethod @@ -177,7 +179,7 @@ class SocialProviderService: return True, f"{provider.title()} account disconnected successfully." except Exception as e: - logger.error(f"Error disconnecting {provider} for user {user.id}: {e}") + capture_and_log(e, f'Disconnect {provider} for user {user.id}', source='service') return False, f"Failed to disconnect {provider} account. Please try again." @staticmethod @@ -210,7 +212,7 @@ class SocialProviderService: } except Exception as e: - logger.error(f"Error getting auth status for user {user.id}: {e}") + capture_and_log(e, f'Get auth status for user {user.id}', source='service') return {"error": "Unable to retrieve authentication status"} @staticmethod @@ -236,5 +238,5 @@ class SocialProviderService: return True, f"Provider '{provider}' is valid and available." except Exception as e: - logger.error(f"Error validating provider {provider}: {e}") + capture_and_log(e, f'Validate provider {provider}', source='service') return False, "Unable to validate provider." diff --git a/backend/apps/accounts/services/user_deletion_service.py b/backend/apps/accounts/services/user_deletion_service.py index 75ec16d8..2e3a3895 100644 --- a/backend/apps/accounts/services/user_deletion_service.py +++ b/backend/apps/accounts/services/user_deletion_service.py @@ -18,6 +18,8 @@ from django.db import transaction from django.template.loader import render_to_string from django.utils import timezone +from apps.core.utils import capture_and_log + logger = logging.getLogger(__name__) User = get_user_model() @@ -292,5 +294,5 @@ class UserDeletionService: logger.info(f"Deletion verification email sent to {user.email}") except Exception as e: - logger.error(f"Failed to send deletion verification email to {user.email}: {str(e)}") + capture_and_log(e, f'Send deletion verification email to {user.email}', source='service') raise diff --git a/backend/apps/core/services/error_service.py b/backend/apps/core/services/error_service.py index 8ebb4c8e..fb05eaac 100644 --- a/backend/apps/core/services/error_service.py +++ b/backend/apps/core/services/error_service.py @@ -130,23 +130,28 @@ class ErrorService: # Merge request_context into metadata merged_metadata = {**(metadata or {}), "request_context": request_context} + # Build create kwargs, only including error_id if provided + create_kwargs = { + "error_type": error_type, + "error_message": error_message[:5000], # Limit message length + "error_stack": error_stack[:10000], # Limit stack length + "error_code": error_code, + "severity": severity, + "source": source, + "endpoint": endpoint, + "http_method": http_method, + "user_agent": user_agent[:1000], + "user": user, + "ip_address_hash": ip_address_hash, + "metadata": merged_metadata, + "environment": environment or {}, + } + # Only include error_id if explicitly provided, else let model default + if error_id is not None: + create_kwargs["error_id"] = error_id + # Create and save error - app_error = ApplicationError.objects.create( - error_id=error_id or None, # Let model generate if not provided - error_type=error_type, - error_message=error_message[:5000], # Limit message length - error_stack=error_stack[:10000], # Limit stack length - error_code=error_code, - severity=severity, - source=source, - endpoint=endpoint, - http_method=http_method, - user_agent=user_agent[:1000], - user=user, - ip_address_hash=ip_address_hash, - metadata=merged_metadata, - environment=environment or {}, - ) + app_error = ApplicationError.objects.create(**create_kwargs) logger.info( f"Captured error {app_error.short_error_id}: {error_type} from {source}" diff --git a/backend/apps/core/services/media_service.py b/backend/apps/core/services/media_service.py index 73ed37b1..f34dbb6b 100644 --- a/backend/apps/core/services/media_service.py +++ b/backend/apps/core/services/media_service.py @@ -14,6 +14,8 @@ from django.conf import settings from django.core.files.uploadedfile import UploadedFile from PIL import ExifTags, Image +from apps.core.utils import capture_and_log + logger = logging.getLogger(__name__) @@ -193,5 +195,5 @@ class MediaService: "available_space": "unknown", } except Exception as e: - logger.error(f"Failed to get storage stats: {str(e)}") + capture_and_log(e, 'Get storage stats', source='service', severity='low') return {"error": str(e)}