From 49e3e0a4040d5129f125eda761b442d38422a5ce Mon Sep 17 00:00:00 2001 From: Akanshu-2u Date: Mon, 5 Jan 2026 11:22:10 +0000 Subject: [PATCH 1/5] fix: enhance error handling and logging in LMSAccountRetirementView --- .../djangoapps/user_api/accounts/views.py | 72 +++++++++++++++++-- 1 file changed, 68 insertions(+), 4 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index f338fd510177..f36973464cc0 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -6,7 +6,9 @@ """ import datetime +import json import logging +import re from functools import wraps import pytz @@ -1118,26 +1120,88 @@ def post(self, request): user_id = retirement.user.id except AttributeError: user_id = 'unknown' + + error_details = { + 'error_type': 'RetirementStateError', + 'user_id': user_id, + 'original_error': str(exc), + 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() + } + + # Store detailed error information in retirement status + try: + current_responses = json.loads(retirement.responses) if retirement.responses else [] + current_responses.append(error_details) + retirement.responses = json.dumps(current_responses) + retirement.save() + except (json.JSONDecodeError, AttributeError): + pass # Continue with logging even if response storage fails + + log_error = self._sanitize_error_message(str(exc)) log.error( 'RetirementStateError during user retirement: user_id=%s, error=%s', - user_id, str(exc) + user_id, log_error ) record_exception() - return Response(str(exc), status=status.HTTP_400_BAD_REQUEST) + return Response("RetirementStateError occurred during retirement", status=status.HTTP_400_BAD_REQUEST) except Exception as exc: # pylint: disable=broad-except try: user_id = retirement.user.id except AttributeError: user_id = 'unknown' + + error_details = { + 'error_type': type(exc).__name__, + 'user_id': user_id, + 'original_error': str(exc), + 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() + } + + # Store detailed error information in retirement status + try: + current_responses = json.loads(retirement.responses) if retirement.responses else [] + current_responses.append(error_details) + retirement.responses = json.dumps(current_responses) + retirement.save() + except (json.JSONDecodeError, AttributeError): + pass # Continue with logging even if response storage fails + + log_error = self._sanitize_error_message(str(exc)) log.error( 'Unexpected error during user retirement: user_id=%s, error=%s', - user_id, str(exc) + user_id, log_error ) record_exception() - return Response(str(exc), status=status.HTTP_500_INTERNAL_SERVER_ERROR) + return Response("Internal error occurred during retirement", status=status.HTTP_500_INTERNAL_SERVER_ERROR) return Response(status=status.HTTP_204_NO_CONTENT) + def _sanitize_error_message(self, error_message): + """ + Remove common PII from error messages while preserving debugging context. + + Args: + error_message (str): The original error message + + Returns: + str: Error message with PII removed + """ + if not error_message: + return error_message + + # Simple PII removal for the most common cases in retirement errors + message = error_message + + # Remove email addresses + message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', + '-', message, flags=re.IGNORECASE) + + # Remove username values but keep the structure + message = re.sub(r"username='[^']*'", "username='-'", message) + message = re.sub(r'username="[^"]*"', 'username="-"', message) + + return message + class AccountRetirementView(ViewSet): """ From 7f731ed523be129dfdf9e3e2d2344ac72bee31b5 Mon Sep 17 00:00:00 2001 From: Akanshu-2u Date: Mon, 5 Jan 2026 11:33:00 +0000 Subject: [PATCH 2/5] fix: removed unwanted comments --- openedx/core/djangoapps/user_api/accounts/views.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index f36973464cc0..95c4abdeb9bb 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -1128,14 +1128,13 @@ def post(self, request): 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() } - # Store detailed error information in retirement status try: current_responses = json.loads(retirement.responses) if retirement.responses else [] current_responses.append(error_details) retirement.responses = json.dumps(current_responses) retirement.save() except (json.JSONDecodeError, AttributeError): - pass # Continue with logging even if response storage fails + pass log_error = self._sanitize_error_message(str(exc)) log.error( @@ -1157,14 +1156,13 @@ def post(self, request): 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() } - # Store detailed error information in retirement status try: current_responses = json.loads(retirement.responses) if retirement.responses else [] current_responses.append(error_details) retirement.responses = json.dumps(current_responses) retirement.save() except (json.JSONDecodeError, AttributeError): - pass # Continue with logging even if response storage fails + pass log_error = self._sanitize_error_message(str(exc)) log.error( @@ -1189,14 +1187,11 @@ def _sanitize_error_message(self, error_message): if not error_message: return error_message - # Simple PII removal for the most common cases in retirement errors message = error_message - # Remove email addresses message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', '-', message, flags=re.IGNORECASE) - # Remove username values but keep the structure message = re.sub(r"username='[^']*'", "username='-'", message) message = re.sub(r'username="[^"]*"', 'username="-"', message) From f72108261c1a84e038bdba48093159e00312a773 Mon Sep 17 00:00:00 2001 From: Akanshu-2u Date: Mon, 5 Jan 2026 13:12:21 +0000 Subject: [PATCH 3/5] fix: improved error logging in database --- .../djangoapps/user_api/accounts/views.py | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index 95c4abdeb9bb..c7dce1a0000b 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -1121,22 +1121,20 @@ def post(self, request): except AttributeError: user_id = 'unknown' - error_details = { - 'error_type': 'RetirementStateError', - 'user_id': user_id, - 'original_error': str(exc), - 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() - } - + log_error = self._sanitize_error_message(str(exc)) + + # Store error information in retirement status as plain text + error_msg = f"RetirementStateError: {log_error}" try: - current_responses = json.loads(retirement.responses) if retirement.responses else [] - current_responses.append(error_details) - retirement.responses = json.dumps(current_responses) - retirement.save() - except (json.JSONDecodeError, AttributeError): - pass + if 'retirement' in locals() and retirement: + if retirement.responses: + retirement.responses += f"\\n{error_msg}" + else: + retirement.responses = error_msg + retirement.save() + except AttributeError: + pass # Continue with logging even if response storage fails - log_error = self._sanitize_error_message(str(exc)) log.error( 'RetirementStateError during user retirement: user_id=%s, error=%s', user_id, log_error @@ -1149,22 +1147,20 @@ def post(self, request): except AttributeError: user_id = 'unknown' - error_details = { - 'error_type': type(exc).__name__, - 'user_id': user_id, - 'original_error': str(exc), - 'timestamp': datetime.datetime.now(pytz.UTC).isoformat() - } - + log_error = self._sanitize_error_message(str(exc)) + + # Store error information in retirement status as plain text + error_msg = f"{type(exc).__name__}: {log_error}" try: - current_responses = json.loads(retirement.responses) if retirement.responses else [] - current_responses.append(error_details) - retirement.responses = json.dumps(current_responses) - retirement.save() - except (json.JSONDecodeError, AttributeError): - pass + if 'retirement' in locals() and retirement: + if retirement.responses: + retirement.responses += f"\\n{error_msg}" + else: + retirement.responses = error_msg + retirement.save() + except AttributeError: + pass # Continue with logging even if response storage fails - log_error = self._sanitize_error_message(str(exc)) log.error( 'Unexpected error during user retirement: user_id=%s, error=%s', user_id, log_error @@ -1189,11 +1185,19 @@ def _sanitize_error_message(self, error_message): message = error_message + # Remove email addresses message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', '-', message, flags=re.IGNORECASE) + # Remove username values in various formats message = re.sub(r"username='[^']*'", "username='-'", message) message = re.sub(r'username="[^"]*"', 'username="-"', message) + message = re.sub(r'username:\s*[^\s,]+', 'username: -', message) + message = re.sub(r'username=\s*[^\s,]+', 'username=-', message) + + # Remove common username patterns in error messages + message = re.sub(r'\bUser\s+[A-Za-z0-9._-]+\s+not found', 'User - not found', message, flags=re.IGNORECASE) + message = re.sub(r'for user\s+[A-Za-z0-9._-]+', 'for user -', message, flags=re.IGNORECASE) return message From 976267db4dd2de1cd3df02b2f3c1f2bc066100f4 Mon Sep 17 00:00:00 2001 From: Akanshu-2u Date: Mon, 5 Jan 2026 13:25:05 +0000 Subject: [PATCH 4/5] fix: fixed lint and style errors --- openedx/core/djangoapps/user_api/accounts/views.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index c7dce1a0000b..ed4187935b92 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -6,7 +6,6 @@ """ import datetime -import json import logging import re from functools import wraps @@ -1072,7 +1071,7 @@ class LMSAccountRetirementView(ViewSet): parser_classes = (JSONParser,) @request_requires_username - def post(self, request): + def post(self, request): # pylint: disable=too-many-statements """ POST /api/user/v1/accounts/retire_misc/ @@ -1122,7 +1121,7 @@ def post(self, request): user_id = 'unknown' log_error = self._sanitize_error_message(str(exc)) - + # Store error information in retirement status as plain text error_msg = f"RetirementStateError: {log_error}" try: @@ -1148,7 +1147,7 @@ def post(self, request): user_id = 'unknown' log_error = self._sanitize_error_message(str(exc)) - + # Store error information in retirement status as plain text error_msg = f"{type(exc).__name__}: {log_error}" try: @@ -1187,14 +1186,14 @@ def _sanitize_error_message(self, error_message): # Remove email addresses message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', - '-', message, flags=re.IGNORECASE) + '-', message, flags=re.IGNORECASE) # Remove username values in various formats message = re.sub(r"username='[^']*'", "username='-'", message) message = re.sub(r'username="[^"]*"', 'username="-"', message) message = re.sub(r'username:\s*[^\s,]+', 'username: -', message) message = re.sub(r'username=\s*[^\s,]+', 'username=-', message) - + # Remove common username patterns in error messages message = re.sub(r'\bUser\s+[A-Za-z0-9._-]+\s+not found', 'User - not found', message, flags=re.IGNORECASE) message = re.sub(r'for user\s+[A-Za-z0-9._-]+', 'for user -', message, flags=re.IGNORECASE) From e7ac3c463937950ae137e8426cde9b8aa9a0153a Mon Sep 17 00:00:00 2001 From: Akanshu-2u Date: Mon, 5 Jan 2026 13:53:26 +0000 Subject: [PATCH 5/5] fix: duplication and regex fault fixed --- .../djangoapps/user_api/accounts/views.py | 80 ++++++++++--------- 1 file changed, 43 insertions(+), 37 deletions(-) diff --git a/openedx/core/djangoapps/user_api/accounts/views.py b/openedx/core/djangoapps/user_api/accounts/views.py index ed4187935b92..412c62edc3f6 100644 --- a/openedx/core/djangoapps/user_api/accounts/views.py +++ b/openedx/core/djangoapps/user_api/accounts/views.py @@ -1115,24 +1115,7 @@ def post(self, request): # pylint: disable=too-many-statements record_exception() return Response(status=status.HTTP_404_NOT_FOUND) except RetirementStateError as exc: - try: - user_id = retirement.user.id - except AttributeError: - user_id = 'unknown' - - log_error = self._sanitize_error_message(str(exc)) - - # Store error information in retirement status as plain text - error_msg = f"RetirementStateError: {log_error}" - try: - if 'retirement' in locals() and retirement: - if retirement.responses: - retirement.responses += f"\\n{error_msg}" - else: - retirement.responses = error_msg - retirement.save() - except AttributeError: - pass # Continue with logging even if response storage fails + user_id, log_error = self._store_retirement_error(exc, retirement, "RetirementStateError") log.error( 'RetirementStateError during user retirement: user_id=%s, error=%s', @@ -1141,24 +1124,7 @@ def post(self, request): # pylint: disable=too-many-statements record_exception() return Response("RetirementStateError occurred during retirement", status=status.HTTP_400_BAD_REQUEST) except Exception as exc: # pylint: disable=broad-except - try: - user_id = retirement.user.id - except AttributeError: - user_id = 'unknown' - - log_error = self._sanitize_error_message(str(exc)) - - # Store error information in retirement status as plain text - error_msg = f"{type(exc).__name__}: {log_error}" - try: - if 'retirement' in locals() and retirement: - if retirement.responses: - retirement.responses += f"\\n{error_msg}" - else: - retirement.responses = error_msg - retirement.save() - except AttributeError: - pass # Continue with logging even if response storage fails + user_id, log_error = self._store_retirement_error(exc, retirement) log.error( 'Unexpected error during user retirement: user_id=%s, error=%s', @@ -1185,7 +1151,7 @@ def _sanitize_error_message(self, error_message): message = error_message # Remove email addresses - message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Z|a-z]{2,}\b', + message = re.sub(r'\b[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}\b', '-', message, flags=re.IGNORECASE) # Remove username values in various formats @@ -1200,6 +1166,46 @@ def _sanitize_error_message(self, error_message): return message + def _store_retirement_error(self, exc, retirement, error_prefix=""): + """ + Store sanitized error information in retirement status and return user_id and log_error for logging. + + Args: + exc: The exception object + retirement: The retirement object (may be None) + error_prefix: Optional prefix for the error message (e.g., "RetirementStateError") + + Returns: + tuple: (user_id, log_error) for logging purposes + """ + # Get user_id safely + try: + user_id = retirement.user.id if retirement else 'unknown' + except AttributeError: + user_id = 'unknown' + + # Sanitize error message + log_error = self._sanitize_error_message(str(exc)) + + # Create error message with prefix + if error_prefix: + error_msg = f"{error_prefix}: {log_error}" + else: + error_msg = f"{type(exc).__name__}: {log_error}" + + # Store error information in retirement status as plain text + try: + if retirement is not None: + if retirement.responses: + retirement.responses += f"\n{error_msg}" + else: + retirement.responses = error_msg + retirement.save() + except AttributeError as e: + log.warning('Failed to store error in retirement status: %s', str(e)) + + return user_id, log_error + class AccountRetirementView(ViewSet): """