fix(handlers): preserve typed errors and attribute partial state in account deletion

.cspell/kernelworx-words.txt

@@ -117,6 +117,7 @@ EEYQ
 esac
 esbuild
 eslint
+excinfo
 exfiltration
 exportable
 falsey
@@ -313,6 +314,7 @@ refetched
 regionAbbrev
 regularuser
 relogin
+relookup
 renderer
 repo
 repr

src/handlers/account_operations.py

@@ -8,7 +8,7 @@
 from typing import TYPE_CHECKING, Any, Dict
 
 import boto3
-from botocore.exceptions import ClientError
+from botocore.exceptions import BotoCoreError, ClientError
 
 # Sibling handler modules use a same-package relative import, which resolves both
 # in the Lambda zip (package `handlers`) and in unit tests (package `src.handlers`).
@@ -19,12 +19,14 @@
     from utils.appsync_types import get_caller_id
     from utils.cognito import retry_on_transient_errors
     from utils.cognito_filters import cognito_user_filter
+    from utils.dynamodb import is_transient_client_error
     from utils.errors import AppError, ErrorCode
     from utils.logging import get_logger
 except ModuleNotFoundError:  # pragma: no cover
     from ..utils.appsync_types import get_caller_id
     from ..utils.cognito import retry_on_transient_errors
     from ..utils.cognito_filters import cognito_user_filter
+    from ..utils.dynamodb import is_transient_client_error
     from ..utils.errors import AppError, ErrorCode
     from ..utils.logging import get_logger
 
@@ -96,7 +98,9 @@ def delete_my_account(event: Dict[str, Any], context: Any) -> bool:
         True if account was deleted successfully
 
     Raises:
-        AppError: If deletion error occurs
+        AppError: If deletion error occurs. When the data sweep succeeded but
+            the Cognito delete failed, the error names the partial state so
+            the caller knows the account needs a retry to complete.
     """
     logger.info("delete_my_account handler invoked")
 
@@ -111,24 +115,52 @@ def delete_my_account(event: Dict[str, Any], context: Any) -> bool:
 
     cognito = boto3.client("cognito-idp")
 
-    # Handler-specific Cognito error handling stays in this try block: the
-    # decorator only covers the generic unexpected-exception path, not typed
-    # ClientError mapping.
+    # Pre-check Cognito lookup to fail early on authorization/network/config issues
     try:
-        # Pre-check Cognito lookup to fail early on authorization/network/config issues
-        try:
-            username = _lookup_cognito_user_with_retry(cognito, user_pool_id, account_id)
-            if not username:
-                logger.warning(f"User not found in Cognito with sub: {account_id}")
-        except ClientError as e:
-            logger.error("Cognito lookup failed before deletion", account_id=account_id, error=str(e), exc_info=True)
-            raise AppError(ErrorCode.INTERNAL_ERROR, "Failed to delete account")
-
-        delete_all_user_data(account_id, logger)
-        _delete_user_from_cognito(cognito, user_pool_id, account_id, username, logger)
-        logger.info("Account deletion completed successfully")
-        return True
+        username = _lookup_cognito_user_with_retry(cognito, user_pool_id, account_id)
+        if not username:
+            logger.warning(f"User not found in Cognito with sub: {account_id}")
+    except (ClientError, BotoCoreError) as e:
+        logger.error("Cognito lookup failed before deletion", account_id=account_id, error=str(e), exc_info=True)
+        raise AppError(ErrorCode.INTERNAL_ERROR, "Failed to delete account") from e
 
-    except ClientError as e:
-        logger.error("Cognito error during account deletion", account_id=account_id, error=str(e), exc_info=True)
-        raise AppError(ErrorCode.INTERNAL_ERROR, "Failed to delete account")
+    # The data sweep runs before the Cognito delete and is safe to re-run, so a
+    # retry after any failure below converges; a sweep failure here leaves the
+    # Cognito user untouched.
+    delete_all_user_data(account_id, logger)
+
+    try:
+        _delete_user_from_cognito(cognito, user_pool_id, account_id, username, logger)
+    except (ClientError, BotoCoreError, AppError) as e:
+        # A typed AppError from the Cognito helper (or a future RESOURCE_BUSY)
+        # propagates with its own code and message intact; only a raw
+        # botocore fault is attributed here. BotoCoreError stays in the catch
+        # because a transport failure after the request was sent is precisely
+        # the unknown-state case below. Only two facts are then established:
+        # the sweep completed and the Cognito delete did not report success.
+        # Whether the user survives is unknown (the delete may have landed,
+        # and a failed re-lookup of an absent user also lands here), so the
+        # message says exactly that.
+        if isinstance(e, AppError):
+            raise
+        error_code = (
+            ErrorCode.RESOURCE_BUSY
+            if isinstance(e, ClientError) and is_transient_client_error(e)
+            else ErrorCode.INTERNAL_ERROR
+        )
+        logger.error(
+            "Account data swept but the Cognito delete did not report success; "
+            "the account's Cognito state is unknown and needs a retry or manual completion",
+            account_id=account_id,
+            error=str(e),
+            error_code=error_code,
+            exc_info=True,
+        )
+        raise AppError(
+            error_code,
+            "Account data was deleted but the Cognito user could not be confirmed deleted; "
+            "retry to complete the deletion",
+        ) from e
+
+    logger.info("Account deletion completed successfully")
+    return True

src/handlers/admin_operations.py

@@ -33,7 +33,7 @@
 from decimal import Decimal, InvalidOperation
 from typing import TYPE_CHECKING, Any, Callable, Dict, NoReturn, Optional, cast
 
-from botocore.exceptions import ClientError
+from botocore.exceptions import BotoCoreError, ClientError
 
 # Sibling handler modules use a same-package relative import, which resolves both
 # in the Lambda zip (package `handlers`) and in unit tests (package `src.handlers`).
@@ -838,6 +838,11 @@ def _delete_user_from_cognito(
                 actor_sub=actor_sub,
             )
             return
+        if error_code in _THROTTLING_ERROR_CODES:
+            logger.warning("Cognito admin_delete_user throttled", error=str(e), error_code=error_code)
+            # Retryable at the purge boundary, which re-raises the typed
+            # error with its code intact.
+            raise AppError(ErrorCode.RESOURCE_BUSY, "Failed to delete user from Cognito") from e
         logger.error("Cognito admin_delete_user failed", error=str(e), error_code=error_code)
         raise AppError(ErrorCode.INTERNAL_ERROR, "Failed to delete user from Cognito") from e
 
@@ -986,7 +991,9 @@ def admin_purge_user_account(event: Dict[str, Any], context: Any) -> bool:
         AppError: If not admin, self-purge is attempted, the user does not
             exist, a supplied profile still exists (CONFLICT), or a deletion
             error occurs. An absent Cognito user is treated as idempotent
-            success.
+            success. When the sweep succeeded but the Cognito delete failed,
+            the error names the partial state so the operator knows the
+            account needs a retry to complete.
     """
     logger = get_logger(__name__)
 
@@ -1019,8 +1026,37 @@ def admin_purge_user_account(event: Dict[str, Any], context: Any) -> bool:
     delete_user_s3_reports(profiles_for_sweep, logger)
     delete_all_user_qr_codes(account_id, logger)
 
+    # The sweep runs before the Cognito delete and is safe to re-run, so a
+    # retry after a failure below converges; a failure here leaves the Cognito
+    # user untouched. When the sweep succeeded but the Cognito delete does not
+    # report success, the partial state is logged loudly with an error_code
+    # field and surfaced honestly to the caller: the Cognito user's state is
+    # unknown, not asserted to survive, because the delete may have landed and
+    # still raised afterwards.
     if username:
-        _delete_user_from_cognito(cognito, user_pool_id, username, email or "", logger, actor_sub)
+        try:
+            _delete_user_from_cognito(cognito, user_pool_id, username, email or "", logger, actor_sub)
+        except (ClientError, BotoCoreError, AppError) as e:
+            # A typed AppError from the Cognito helper propagates with its own
+            # code and message intact; only a raw botocore fault is attributed
+            # here. BotoCoreError stays in the catch because a transport
+            # failure after the request was sent is precisely the
+            # unknown-state case below.
+            if isinstance(e, AppError):
+                raise
+            logger.error(
+                "Account data swept but the Cognito delete did not report success; "
+                "the account's Cognito state is unknown and needs a retry or manual completion",
+                account_id=account_id,
+                error=str(e),
+                error_code=ErrorCode.INTERNAL_ERROR,
+                exc_info=True,
+            )
+            raise AppError(
+                ErrorCode.INTERNAL_ERROR,
+                "Account data was deleted but the Cognito user could not be confirmed deleted; "
+                "retry the purge to complete the deletion",
+            ) from e
     else:
         logger.info("Cognito user already absent", account_id=account_id, actor_sub=actor_sub)
 

src/handlers/deletion_cascade.py

@@ -19,7 +19,7 @@
 
 from typing import TYPE_CHECKING, Any, Dict
 
-from botocore.exceptions import ClientError
+from botocore.exceptions import BotoCoreError, ClientError
 
 # Sibling handler modules use a same-package relative import, which resolves both
 # in the Lambda zip (package `handlers`) and in unit tests (package `src.handlers`).
@@ -32,11 +32,13 @@
 
 # Handle both Lambda (absolute) and unit test (relative) imports
 try:  # pragma: no cover
-    from utils.dynamodb import tables
+    from utils.dynamodb import is_transient_client_error, tables
+    from utils.errors import AppError, ErrorCode
     from utils.logging import get_logger
     from utils.payment_methods import delete_all_user_qr_codes
 except ModuleNotFoundError:  # pragma: no cover
-    from ..utils.dynamodb import tables
+    from ..utils.dynamodb import is_transient_client_error, tables
+    from ..utils.errors import AppError, ErrorCode
     from ..utils.logging import get_logger
     from ..utils.payment_methods import delete_all_user_qr_codes
 
@@ -252,22 +254,48 @@ def delete_all_user_data(account_id: str, logger: Any = None) -> None:
 
     profiles = get_user_profiles(normalize_account_id(account_id))
 
-    delete_user_orders(profiles, log)
-    delete_user_campaigns(profiles, log)
-    delete_user_shares(profiles, log)
-    delete_invites_for_owned_profiles(profiles, log)
-    delete_inbound_shares(account_id, log)
-    delete_user_s3_reports(profiles, log)
-    delete_user_profiles(account_id, profiles, log)
-    # Catalogs are preserved per product design and should never be deleted.
-    # Delete payment method QR codes from S3 per captain decision
-    delete_all_user_qr_codes(account_id, log)
+    swept = {
+        "orders": delete_user_orders(profiles, log),
+        "campaigns": delete_user_campaigns(profiles, log),
+        "shares": delete_user_shares(profiles, log),
+        "invites": delete_invites_for_owned_profiles(profiles, log),
+        "inbound_shares": delete_inbound_shares(account_id, log),
+        "s3_reports": delete_user_s3_reports(profiles, log),
+        "profiles": delete_user_profiles(account_id, profiles, log),
+        # Catalogs are preserved per product design and should never be deleted.
+        # Delete payment method QR codes from S3 per captain decision
+        "qr_codes": delete_all_user_qr_codes(account_id, log),
+    }
 
     account_id_key = normalize_account_id(account_id)
     try:
         tables.accounts.delete_item(Key={"accountId": account_id_key})
         log.info("Deleted account from DynamoDB", account_id=account_id_key)
-    except ClientError as e:
-        log.error("Failed to delete account from DynamoDB", error=str(e), account_id=account_id_key)
-        raise
+    except (ClientError, BotoCoreError) as e:
+        # Every sub-cascade above completed, so this is the most likely partial
+        # state of the whole cascade: all user data is gone but the account
+        # record — and the Cognito sign-in that outlives it in the self-service
+        # flow — survives. Say exactly that; a throttled failure is retryable.
+        # BotoCoreError joins the catch for the same transport-fault class:
+        # a failure after the request was sent leaves the same swept/survived
+        # split, and only a ClientError can be classified as throttling.
+        error_code = (
+            ErrorCode.RESOURCE_BUSY
+            if isinstance(e, ClientError) and is_transient_client_error(e)
+            else ErrorCode.INTERNAL_ERROR
+        )
+        log.error(
+            "All user data was swept but the account record could not be deleted; "
+            "the account record and Cognito sign-in survive and need a retry or manual completion",
+            account_id=account_id_key,
+            swept=swept,
+            error=str(e),
+            error_code=error_code,
+            exc_info=True,
+        )
+        raise AppError(
+            error_code,
+            "All user data was deleted but the account record could not be removed, "
+            "so the sign-in remains active; retry to complete the deletion",
+        ) from e
     log.info("Deleted all user data from DynamoDB")