From c2124bec7b940e855aaba97f2babb1faceeea566 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:18:07 +0200 Subject: [PATCH 01/11] Add info-level execution logs for privacy request status transitions 8 code paths transition a privacy request's status without creating an ExecutionLog entry, making it impossible to troubleshoot stuck requests in the UI. This adds an "info" ExecutionLogStatus and logs all status transitions: requires_input (manual webhooks), paused (webhook halt, batch email), awaiting_access_review, resumed from webhook, and pre-approval not eligible. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../models/privacy_request/privacy_request.py | 22 +++++++++++++ src/fides/api/models/worker_task.py | 1 + .../privacy_request/access_review_hooks.py | 8 +++++ .../privacy_request/email_batch_service.py | 8 +++++ .../privacy_request/request_runner_service.py | 32 ++++++++++++++++++ .../v1/endpoints/privacy_request_endpoints.py | 16 +++++++++ .../privacy_request/test_execution_logs.py | 33 +++++++++++++++++++ 7 files changed, 120 insertions(+) diff --git a/src/fides/api/models/privacy_request/privacy_request.py b/src/fides/api/models/privacy_request/privacy_request.py index 4f941b449cc..fd5f4a36d41 100644 --- a/src/fides/api/models/privacy_request/privacy_request.py +++ b/src/fides/api/models/privacy_request/privacy_request.py @@ -1651,6 +1651,28 @@ def add_error_execution_log( }, ) + def add_info_execution_log( + self, + db: Session, + connection_key: Optional[str], + dataset_name: Optional[str], + collection_name: Optional[str], + message: str, + action_type: ActionType, + ) -> ExecutionLog: + return ExecutionLog.create( + db=db, + data={ + "privacy_request_id": self.id, + "connection_key": connection_key, + "dataset_name": dataset_name, + "collection_name": collection_name, + "status": ExecutionLogStatus.info, + "message": message, + "action_type": action_type, + }, + ) + class PrivacyRequestError(Base): """The DB ORM model to track PrivacyRequests error message status.""" diff --git a/src/fides/api/models/worker_task.py b/src/fides/api/models/worker_task.py index 955a4f461d2..da837e3231c 100644 --- a/src/fides/api/models/worker_task.py +++ b/src/fides/api/models/worker_task.py @@ -18,6 +18,7 @@ class ExecutionLogStatus(enum.Enum): retrying = "retrying" skipped = "skipped" polling = "polling" + info = "info" @classmethod def in_progress_statuses(cls) -> Set["ExecutionLogStatus"]: diff --git a/src/fides/api/service/privacy_request/access_review_hooks.py b/src/fides/api/service/privacy_request/access_review_hooks.py index 0c1c25a74c0..749bc8e9f3f 100644 --- a/src/fides/api/service/privacy_request/access_review_hooks.py +++ b/src/fides/api/service/privacy_request/access_review_hooks.py @@ -111,6 +111,14 @@ def should_wait_for_access_review( privacy_request.status = PrivacyRequestStatus.awaiting_access_review privacy_request.save(db=session) + privacy_request.add_info_execution_log( + session, + connection_key=None, + dataset_name=None, + collection_name=None, + message="Request paused for access package review", + action_type=ActionType.access, + ) logger.info( "Privacy request '{}' paused for access package review.", privacy_request.id, diff --git a/src/fides/api/service/privacy_request/email_batch_service.py b/src/fides/api/service/privacy_request/email_batch_service.py index 6bb62c96598..52bc392d58a 100644 --- a/src/fides/api/service/privacy_request/email_batch_service.py +++ b/src/fides/api/service/privacy_request/email_batch_service.py @@ -159,6 +159,14 @@ def requeue_privacy_requests_after_email_send( ) privacy_request.status = PrivacyRequestStatus.paused privacy_request.save(db=db) + privacy_request.add_info_execution_log( + db, + connection_key=None, + dataset_name=None, + collection_name=None, + message="Request paused after batch email send, resuming from post-webhooks step", + action_type=privacy_request.policy.get_action_type(), + ) queue_privacy_request( privacy_request_id=privacy_request.id, diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index 0f0b73afc9f..e7bf29312aa 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -175,6 +175,14 @@ def get_manual_webhook_access_inputs( logger.info(exc) privacy_request.status = PrivacyRequestStatus.requires_input privacy_request.save(db) + privacy_request.add_info_execution_log( + db, + connection_key=None, + dataset_name=None, + collection_name=None, + message=f"Manual webhook access input missing: {exc}", + action_type=ActionType.access, + ) return ManualWebhookResults( manual_data_for_upload=manual_inputs_for_upload, manual_data_for_storage=manual_inputs_for_storage, @@ -213,6 +221,14 @@ def get_manual_webhook_erasure_inputs( logger.info(exc) privacy_request.status = PrivacyRequestStatus.requires_input privacy_request.save(db) + privacy_request.add_info_execution_log( + db, + connection_key=None, + dataset_name=None, + collection_name=None, + message=f"Manual webhook erasure input missing: {exc}", + action_type=ActionType.erasure, + ) return ManualWebhookResults( manual_data_for_upload=manual_inputs, manual_data_for_storage=manual_inputs, @@ -783,6 +799,14 @@ def run_privacy_request( except PrivacyRequestPaused as exc: privacy_request.pause_processing(session) + privacy_request.add_info_execution_log( + session, + connection_key=None, + dataset_name=None, + collection_name=None, + message=f"Request paused by webhook halt instruction: {exc}", + action_type=privacy_request.policy.get_action_type(), + ) _log_warning(exc, CONFIG.dev_mode) return @@ -1376,6 +1400,14 @@ def run_webhooks_and_report_status( webhook.key, ) privacy_request.pause_processing(db) + privacy_request.add_info_execution_log( + db, + connection_key=webhook.key, + dataset_name=None, + collection_name=None, + message=f"Request paused by webhook: {webhook.key}", + action_type=privacy_request.policy.get_action_type(), + ) initiate_paused_privacy_request_followup(privacy_request) return False except ClientUnsuccessfulException as exc: diff --git a/src/fides/api/v1/endpoints/privacy_request_endpoints.py b/src/fides/api/v1/endpoints/privacy_request_endpoints.py index e17b20ff38c..ff141ee4606 100644 --- a/src/fides/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fides/api/v1/endpoints/privacy_request_endpoints.py @@ -827,6 +827,14 @@ def resume_privacy_request( privacy_request.status = PrivacyRequestStatus.in_processing privacy_request.save(db=db) + privacy_request.add_info_execution_log( + db, + connection_key=webhook.key, + dataset_name=None, + collection_name=None, + message=f"Request resumed from webhook pause: {webhook.key}", + action_type=privacy_request.policy.get_action_type(), + ) queue_privacy_request( privacy_request_id=privacy_request.id, @@ -1332,6 +1340,14 @@ def mark_privacy_request_pre_approve_not_eligible( if privacy_request: privacy_request.status = PrivacyRequestStatus.pre_approval_not_eligible privacy_request.save(db) + privacy_request.add_info_execution_log( + db, + connection_key=None, + dataset_name=None, + collection_name=None, + message=f"Pre-approval webhook returned not eligible: {webhook.name}", + action_type=privacy_request.policy.get_action_type(), + ) def _handle_manual_webhook_input( diff --git a/tests/fides/ops/models/privacy_request/test_execution_logs.py b/tests/fides/ops/models/privacy_request/test_execution_logs.py index df34d0e6a0f..36a95761bda 100644 --- a/tests/fides/ops/models/privacy_request/test_execution_logs.py +++ b/tests/fides/ops/models/privacy_request/test_execution_logs.py @@ -8,6 +8,7 @@ ExecutionLog, can_run_checkpoint, ) +from fides.api.models.privacy_request.privacy_request import PrivacyRequest from fides.api.models.worker_task import ExecutionLogStatus from fides.api.schemas.policy import ActionType, CurrentStep @@ -144,3 +145,35 @@ def test_execution_log_large_data(db, execution_log_data): assert retrieved_execution_log.message == large_message assert retrieved_execution_log.fields_affected == large_fields_affected execution_log.delete(db) + + +def test_execution_log_info_status(db, execution_log_data): + """Test that execution logs can be created with the info status.""" + execution_log_data["status"] = "info" + execution_log = ExecutionLog.create(db, data=execution_log_data) + + retrieved = ( + db.query(ExecutionLog).filter_by(privacy_request_id="test_id").first() + ) + assert retrieved is not None + assert retrieved.status == ExecutionLogStatus.info + execution_log.delete(db) + + +def test_add_info_execution_log(db, privacy_request, policy): + """Test the add_info_execution_log convenience method on PrivacyRequest.""" + log = privacy_request.add_info_execution_log( + db, + connection_key="test_connection", + dataset_name=None, + collection_name=None, + message="Request paused for testing", + action_type=ActionType.access, + ) + + assert log.status == ExecutionLogStatus.info + assert log.connection_key == "test_connection" + assert log.message == "Request paused for testing" + assert log.action_type == ActionType.access + assert log.privacy_request_id == privacy_request.id + log.delete(db) From efcf9d4b12a9785a77ccacdea4a1dee620ba48ca Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:18:29 +0200 Subject: [PATCH 02/11] Add changelog entry for ENG-3924 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../8251-info-execution-logs-for-status-transitions.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/8251-info-execution-logs-for-status-transitions.yaml diff --git a/changelog/8251-info-execution-logs-for-status-transitions.yaml b/changelog/8251-info-execution-logs-for-status-transitions.yaml new file mode 100644 index 00000000000..3da2c5802f1 --- /dev/null +++ b/changelog/8251-info-execution-logs-for-status-transitions.yaml @@ -0,0 +1,4 @@ +type: Changed +description: Added info-level execution logs for privacy request status transitions (requires_input, paused, awaiting_access_review, pre_approval_not_eligible) to improve troubleshooting +pr: 8251 +labels: [] From b95bb7e96aa3d81347aca93a3368c6d4a115b786 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:18:52 +0200 Subject: [PATCH 03/11] Apply ruff formatting Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ConsentAnalyticsWidgetMockup.tsx | 372 ++++++++++++++++++ .../consent/reporting/analytics-mockup.tsx | 28 ++ .../privacy_request/test_execution_logs.py | 4 +- 3 files changed, 401 insertions(+), 3 deletions(-) create mode 100644 clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx create mode 100644 clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx diff --git a/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx b/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx new file mode 100644 index 00000000000..6bcc9104da5 --- /dev/null +++ b/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx @@ -0,0 +1,372 @@ +/** + * MOCKUP ONLY — for design review, not production use. + * + * Consent Analytics Widget mockup for the Consent Report page. + * Follows the Action Center ProgressCard visual pattern with + * filter dropdowns similar to MonitorListSearchForm. + * + * Ticket: ENG-3623 + */ + +import { + Badge, + Card, + Col, + Descriptions, + Divider, + Flex, + Form, + Row, + Select, + StackedBarChart, + Statistic, + Text, + Title, + Tooltip, +} from "fidesui"; + +// --------------------------------------------------------------------------- +// Mock data – replace with RTK Query hooks against +// /dashboard/consent-analytics when API is ready +// --------------------------------------------------------------------------- + +const MOCK_NOTICES = [ + { label: "All notices", value: "all" }, + { label: "Performance", value: "performance" }, + { label: "Targeting / Advertising", value: "targeting" }, + { label: "Functional", value: "functional" }, + { label: "Analytics", value: "analytics" }, +]; + +const MOCK_REGIONS = [ + { label: "All regions", value: "all" }, + { label: "US - California", value: "us_ca" }, + { label: "US - Virginia", value: "us_va" }, + { label: "EU - France", value: "eu_fr" }, + { label: "EU - Germany", value: "eu_de" }, + { label: "UK", value: "gb" }, +]; + +const MOCK_DOMAINS = [ + { label: "All domains", value: "all" }, + { label: "www.example.com", value: "www.example.com" }, + { label: "app.example.com", value: "app.example.com" }, + { label: "shop.example.com", value: "shop.example.com" }, +]; + +const MOCK_TIME_PERIODS = [ + { label: "Past 24 hours", value: "24h" }, + { label: "Past 7 days", value: "7d" }, + { label: "Past 30 days", value: "30d" }, + { label: "Past 90 days", value: "90d" }, +]; + +// --------------------------------------------------------------------------- +// Consent Rate Card — mirrors ProgressCard layout +// --------------------------------------------------------------------------- + +interface ConsentRateCardProps { + title: string; + ratePercent: number; + uniqueUsers: number; + totalUsers: number; + barData: { optIn: number; optOut: number; noAction: number }; + breakdownLabel: string; + breakdown: { label: string; value: number }[]; +} + +const ConsentRateCard = ({ + title, + ratePercent, + uniqueUsers, + totalUsers, + barData, + breakdownLabel, + breakdown, +}: ConsentRateCardProps) => { + const barTotal = barData.optIn + barData.optOut + barData.noAction; + + return ( + + + {title} + + + {/* Large stat */} + + + Unique devices + + + + {barData.optIn.toLocaleString()} + + + {barData.optOut.toLocaleString()} + + + {barData.noAction.toLocaleString()} + + + + } + > + + + + + {uniqueUsers.toLocaleString()} of {totalUsers.toLocaleString()} unique + devices + + + {/* Stacked bar */} +
+ + 0 ? ( +
+ + {breakdownLabel} + + + {breakdown.map(({ label, value }) => ( + + {value}% + + ))} + +
+ ) : undefined + } + > + + + + + + +
+ + ); +}; + +// --------------------------------------------------------------------------- +// Main Widget +// --------------------------------------------------------------------------- + +const ConsentAnalyticsWidgetMockup = () => { + return ( + + {/* Filter bar — mirrors MonitorListSearchForm */} +
+ + + + + + +
+ + {/* Stats cards row — mirrors MonitorStats horizontal layout */} + + {/* Card 1: Overall Opt-in Rate */} + + + + + {/* Card 2: Opt-out Rate */} + + + + + {/* Card 3: Overall Consent Rate */} + + + + + {/* Card 4: Notices Served */} + + + Notices served + + + total notices served to unique devices +
+ + + 22,140 + + + 5,410 + + + 2,480 + + +
+ + + + {/* TCF section — conditionally shown when TCF notices exist */} + + + + TCF consent + + + + + 856 of 5,784 TCF interactions + + + + + + 3,946 of 5,784 TCF interactions + + + + + + 982 of 5,784 TCF interactions + + + + + + + ); +}; + +export default ConsentAnalyticsWidgetMockup; diff --git a/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx b/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx new file mode 100644 index 00000000000..4503cf3226c --- /dev/null +++ b/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx @@ -0,0 +1,28 @@ +/** + * Standalone mockup page — visit /consent/reporting/analytics-mockup + * to preview the Consent Analytics Widget in isolation. + * + * Delete this file before merging to main. + */ + +import FixedLayout from "~/features/common/FixedLayout"; +import PageHeader from "~/features/common/PageHeader"; +import ConsentAnalyticsWidgetMockup from "~/features/consent-reporting/ConsentAnalyticsWidgetMockup"; + +const AnalyticsMockupPage = () => { + return ( + + + + + ); +}; + +export default AnalyticsMockupPage; diff --git a/tests/fides/ops/models/privacy_request/test_execution_logs.py b/tests/fides/ops/models/privacy_request/test_execution_logs.py index 36a95761bda..dd442541228 100644 --- a/tests/fides/ops/models/privacy_request/test_execution_logs.py +++ b/tests/fides/ops/models/privacy_request/test_execution_logs.py @@ -152,9 +152,7 @@ def test_execution_log_info_status(db, execution_log_data): execution_log_data["status"] = "info" execution_log = ExecutionLog.create(db, data=execution_log_data) - retrieved = ( - db.query(ExecutionLog).filter_by(privacy_request_id="test_id").first() - ) + retrieved = db.query(ExecutionLog).filter_by(privacy_request_id="test_id").first() assert retrieved is not None assert retrieved.status == ExecutionLogStatus.info execution_log.delete(db) From de15d012660d0534641b151479652f3504ed5f2c Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:19:06 +0200 Subject: [PATCH 04/11] Remove accidentally staged unrelated files Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ConsentAnalyticsWidgetMockup.tsx | 372 ------------------ .../consent/reporting/analytics-mockup.tsx | 28 -- 2 files changed, 400 deletions(-) delete mode 100644 clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx delete mode 100644 clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx diff --git a/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx b/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx deleted file mode 100644 index 6bcc9104da5..00000000000 --- a/clients/admin-ui/src/features/consent-reporting/ConsentAnalyticsWidgetMockup.tsx +++ /dev/null @@ -1,372 +0,0 @@ -/** - * MOCKUP ONLY — for design review, not production use. - * - * Consent Analytics Widget mockup for the Consent Report page. - * Follows the Action Center ProgressCard visual pattern with - * filter dropdowns similar to MonitorListSearchForm. - * - * Ticket: ENG-3623 - */ - -import { - Badge, - Card, - Col, - Descriptions, - Divider, - Flex, - Form, - Row, - Select, - StackedBarChart, - Statistic, - Text, - Title, - Tooltip, -} from "fidesui"; - -// --------------------------------------------------------------------------- -// Mock data – replace with RTK Query hooks against -// /dashboard/consent-analytics when API is ready -// --------------------------------------------------------------------------- - -const MOCK_NOTICES = [ - { label: "All notices", value: "all" }, - { label: "Performance", value: "performance" }, - { label: "Targeting / Advertising", value: "targeting" }, - { label: "Functional", value: "functional" }, - { label: "Analytics", value: "analytics" }, -]; - -const MOCK_REGIONS = [ - { label: "All regions", value: "all" }, - { label: "US - California", value: "us_ca" }, - { label: "US - Virginia", value: "us_va" }, - { label: "EU - France", value: "eu_fr" }, - { label: "EU - Germany", value: "eu_de" }, - { label: "UK", value: "gb" }, -]; - -const MOCK_DOMAINS = [ - { label: "All domains", value: "all" }, - { label: "www.example.com", value: "www.example.com" }, - { label: "app.example.com", value: "app.example.com" }, - { label: "shop.example.com", value: "shop.example.com" }, -]; - -const MOCK_TIME_PERIODS = [ - { label: "Past 24 hours", value: "24h" }, - { label: "Past 7 days", value: "7d" }, - { label: "Past 30 days", value: "30d" }, - { label: "Past 90 days", value: "90d" }, -]; - -// --------------------------------------------------------------------------- -// Consent Rate Card — mirrors ProgressCard layout -// --------------------------------------------------------------------------- - -interface ConsentRateCardProps { - title: string; - ratePercent: number; - uniqueUsers: number; - totalUsers: number; - barData: { optIn: number; optOut: number; noAction: number }; - breakdownLabel: string; - breakdown: { label: string; value: number }[]; -} - -const ConsentRateCard = ({ - title, - ratePercent, - uniqueUsers, - totalUsers, - barData, - breakdownLabel, - breakdown, -}: ConsentRateCardProps) => { - const barTotal = barData.optIn + barData.optOut + barData.noAction; - - return ( - - - {title} - - - {/* Large stat */} - - - Unique devices - - - - {barData.optIn.toLocaleString()} - - - {barData.optOut.toLocaleString()} - - - {barData.noAction.toLocaleString()} - - - - } - > - - - - - {uniqueUsers.toLocaleString()} of {totalUsers.toLocaleString()} unique - devices - - - {/* Stacked bar */} -
- - 0 ? ( -
- - {breakdownLabel} - - - {breakdown.map(({ label, value }) => ( - - {value}% - - ))} - -
- ) : undefined - } - > - - - - - - -
- - ); -}; - -// --------------------------------------------------------------------------- -// Main Widget -// --------------------------------------------------------------------------- - -const ConsentAnalyticsWidgetMockup = () => { - return ( - - {/* Filter bar — mirrors MonitorListSearchForm */} -
- - - - - - -
- - {/* Stats cards row — mirrors MonitorStats horizontal layout */} - - {/* Card 1: Overall Opt-in Rate */} - - - - - {/* Card 2: Opt-out Rate */} - - - - - {/* Card 3: Overall Consent Rate */} - - - - - {/* Card 4: Notices Served */} - - - Notices served - - - total notices served to unique devices -
- - - 22,140 - - - 5,410 - - - 2,480 - - -
- - - - {/* TCF section — conditionally shown when TCF notices exist */} - - - - TCF consent - - - - - 856 of 5,784 TCF interactions - - - - - - 3,946 of 5,784 TCF interactions - - - - - - 982 of 5,784 TCF interactions - - - - - - - ); -}; - -export default ConsentAnalyticsWidgetMockup; diff --git a/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx b/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx deleted file mode 100644 index 4503cf3226c..00000000000 --- a/clients/admin-ui/src/pages/consent/reporting/analytics-mockup.tsx +++ /dev/null @@ -1,28 +0,0 @@ -/** - * Standalone mockup page — visit /consent/reporting/analytics-mockup - * to preview the Consent Analytics Widget in isolation. - * - * Delete this file before merging to main. - */ - -import FixedLayout from "~/features/common/FixedLayout"; -import PageHeader from "~/features/common/PageHeader"; -import ConsentAnalyticsWidgetMockup from "~/features/consent-reporting/ConsentAnalyticsWidgetMockup"; - -const AnalyticsMockupPage = () => { - return ( - - - - - ); -}; - -export default AnalyticsMockupPage; From ccb977f8138f06a1d7aa7b83012ffee394305dc2 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:21:14 +0200 Subject: [PATCH 05/11] Add info status color mapping to execution log UI Co-Authored-By: Claude Opus 4.6 (1M context) --- clients/admin-ui/src/features/privacy-requests/types.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clients/admin-ui/src/features/privacy-requests/types.ts b/clients/admin-ui/src/features/privacy-requests/types.ts index cf140d9386f..05a98169d2b 100644 --- a/clients/admin-ui/src/features/privacy-requests/types.ts +++ b/clients/admin-ui/src/features/privacy-requests/types.ts @@ -25,6 +25,7 @@ export enum ExecutionLogStatus { RETRYING = "retrying", SKIPPED = "skipped", POLLING = "polling", + INFO = "info", // Audit log statuses for pre-approval webhooks APPROVED = "approved", DENIED = "denied", @@ -43,6 +44,7 @@ export const ExecutionLogStatusLabels: Record = { [ExecutionLogStatus.RETRYING]: "Retrying", [ExecutionLogStatus.SKIPPED]: "Skipped", [ExecutionLogStatus.POLLING]: "Awaiting polling", + [ExecutionLogStatus.INFO]: "Info", [ExecutionLogStatus.APPROVED]: "Approved", [ExecutionLogStatus.DENIED]: "Denied", [ExecutionLogStatus.PRE_APPROVAL_WEBHOOK_TRIGGERED]: "Webhooks triggered", @@ -63,6 +65,7 @@ export const ExecutionLogStatusColors: Record< [ExecutionLogStatus.PAUSED]: undefined, [ExecutionLogStatus.RETRYING]: undefined, [ExecutionLogStatus.POLLING]: CUSTOM_TAG_COLOR.WARNING, + [ExecutionLogStatus.INFO]: CUSTOM_TAG_COLOR.INFO, [ExecutionLogStatus.APPROVED]: CUSTOM_TAG_COLOR.SUCCESS, [ExecutionLogStatus.DENIED]: CUSTOM_TAG_COLOR.WARNING, [ExecutionLogStatus.PRE_APPROVAL_WEBHOOK_TRIGGERED]: CUSTOM_TAG_COLOR.INFO, From ffa7ffbe6c55262ea7e18cc2325407b9a1a8295d Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 13:31:27 +0200 Subject: [PATCH 06/11] Fix mypy errors: add type: ignore for get_action_type() returning Optional Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/api/service/privacy_request/email_batch_service.py | 2 +- .../api/service/privacy_request/request_runner_service.py | 4 ++-- src/fides/api/v1/endpoints/privacy_request_endpoints.py | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fides/api/service/privacy_request/email_batch_service.py b/src/fides/api/service/privacy_request/email_batch_service.py index 52bc392d58a..3d6d903b622 100644 --- a/src/fides/api/service/privacy_request/email_batch_service.py +++ b/src/fides/api/service/privacy_request/email_batch_service.py @@ -165,7 +165,7 @@ def requeue_privacy_requests_after_email_send( dataset_name=None, collection_name=None, message="Request paused after batch email send, resuming from post-webhooks step", - action_type=privacy_request.policy.get_action_type(), + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] ) queue_privacy_request( diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index e7bf29312aa..4cfc03f3f12 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -805,7 +805,7 @@ def run_privacy_request( dataset_name=None, collection_name=None, message=f"Request paused by webhook halt instruction: {exc}", - action_type=privacy_request.policy.get_action_type(), + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] ) _log_warning(exc, CONFIG.dev_mode) return @@ -1406,7 +1406,7 @@ def run_webhooks_and_report_status( dataset_name=None, collection_name=None, message=f"Request paused by webhook: {webhook.key}", - action_type=privacy_request.policy.get_action_type(), + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] ) initiate_paused_privacy_request_followup(privacy_request) return False diff --git a/src/fides/api/v1/endpoints/privacy_request_endpoints.py b/src/fides/api/v1/endpoints/privacy_request_endpoints.py index ff141ee4606..2ffa82946c6 100644 --- a/src/fides/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fides/api/v1/endpoints/privacy_request_endpoints.py @@ -833,7 +833,7 @@ def resume_privacy_request( dataset_name=None, collection_name=None, message=f"Request resumed from webhook pause: {webhook.key}", - action_type=privacy_request.policy.get_action_type(), + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] ) queue_privacy_request( @@ -1346,7 +1346,7 @@ def mark_privacy_request_pre_approve_not_eligible( dataset_name=None, collection_name=None, message=f"Pre-approval webhook returned not eligible: {webhook.name}", - action_type=privacy_request.policy.get_action_type(), + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] ) From 024755d6ea953274063851e233bf51b018a13c61 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 14:11:36 +0200 Subject: [PATCH 07/11] Scope info execution logs to pause cases only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove info logs from requires_input, pre_approval_not_eligible, resumed, and awaiting_access_review transitions — those statuses are already self-explanatory. Keep info logs only for paused transitions where the reason (webhook halt vs batch email) is not obvious from status alone. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../privacy_request/access_review_hooks.py | 8 ------ .../privacy_request/request_runner_service.py | 16 ----------- .../v1/endpoints/privacy_request_endpoints.py | 16 ----------- .../privacy_request/test_execution_logs.py | 20 ------------- .../test_request_runner_service.py | 28 +++++++++++++++++++ 5 files changed, 28 insertions(+), 60 deletions(-) diff --git a/src/fides/api/service/privacy_request/access_review_hooks.py b/src/fides/api/service/privacy_request/access_review_hooks.py index 749bc8e9f3f..0c1c25a74c0 100644 --- a/src/fides/api/service/privacy_request/access_review_hooks.py +++ b/src/fides/api/service/privacy_request/access_review_hooks.py @@ -111,14 +111,6 @@ def should_wait_for_access_review( privacy_request.status = PrivacyRequestStatus.awaiting_access_review privacy_request.save(db=session) - privacy_request.add_info_execution_log( - session, - connection_key=None, - dataset_name=None, - collection_name=None, - message="Request paused for access package review", - action_type=ActionType.access, - ) logger.info( "Privacy request '{}' paused for access package review.", privacy_request.id, diff --git a/src/fides/api/service/privacy_request/request_runner_service.py b/src/fides/api/service/privacy_request/request_runner_service.py index 4cfc03f3f12..a10acc05ab3 100644 --- a/src/fides/api/service/privacy_request/request_runner_service.py +++ b/src/fides/api/service/privacy_request/request_runner_service.py @@ -175,14 +175,6 @@ def get_manual_webhook_access_inputs( logger.info(exc) privacy_request.status = PrivacyRequestStatus.requires_input privacy_request.save(db) - privacy_request.add_info_execution_log( - db, - connection_key=None, - dataset_name=None, - collection_name=None, - message=f"Manual webhook access input missing: {exc}", - action_type=ActionType.access, - ) return ManualWebhookResults( manual_data_for_upload=manual_inputs_for_upload, manual_data_for_storage=manual_inputs_for_storage, @@ -221,14 +213,6 @@ def get_manual_webhook_erasure_inputs( logger.info(exc) privacy_request.status = PrivacyRequestStatus.requires_input privacy_request.save(db) - privacy_request.add_info_execution_log( - db, - connection_key=None, - dataset_name=None, - collection_name=None, - message=f"Manual webhook erasure input missing: {exc}", - action_type=ActionType.erasure, - ) return ManualWebhookResults( manual_data_for_upload=manual_inputs, manual_data_for_storage=manual_inputs, diff --git a/src/fides/api/v1/endpoints/privacy_request_endpoints.py b/src/fides/api/v1/endpoints/privacy_request_endpoints.py index 2ffa82946c6..e17b20ff38c 100644 --- a/src/fides/api/v1/endpoints/privacy_request_endpoints.py +++ b/src/fides/api/v1/endpoints/privacy_request_endpoints.py @@ -827,14 +827,6 @@ def resume_privacy_request( privacy_request.status = PrivacyRequestStatus.in_processing privacy_request.save(db=db) - privacy_request.add_info_execution_log( - db, - connection_key=webhook.key, - dataset_name=None, - collection_name=None, - message=f"Request resumed from webhook pause: {webhook.key}", - action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] - ) queue_privacy_request( privacy_request_id=privacy_request.id, @@ -1340,14 +1332,6 @@ def mark_privacy_request_pre_approve_not_eligible( if privacy_request: privacy_request.status = PrivacyRequestStatus.pre_approval_not_eligible privacy_request.save(db) - privacy_request.add_info_execution_log( - db, - connection_key=None, - dataset_name=None, - collection_name=None, - message=f"Pre-approval webhook returned not eligible: {webhook.name}", - action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] - ) def _handle_manual_webhook_input( diff --git a/tests/fides/ops/models/privacy_request/test_execution_logs.py b/tests/fides/ops/models/privacy_request/test_execution_logs.py index dd442541228..7319f199e44 100644 --- a/tests/fides/ops/models/privacy_request/test_execution_logs.py +++ b/tests/fides/ops/models/privacy_request/test_execution_logs.py @@ -8,7 +8,6 @@ ExecutionLog, can_run_checkpoint, ) -from fides.api.models.privacy_request.privacy_request import PrivacyRequest from fides.api.models.worker_task import ExecutionLogStatus from fides.api.schemas.policy import ActionType, CurrentStep @@ -156,22 +155,3 @@ def test_execution_log_info_status(db, execution_log_data): assert retrieved is not None assert retrieved.status == ExecutionLogStatus.info execution_log.delete(db) - - -def test_add_info_execution_log(db, privacy_request, policy): - """Test the add_info_execution_log convenience method on PrivacyRequest.""" - log = privacy_request.add_info_execution_log( - db, - connection_key="test_connection", - dataset_name=None, - collection_name=None, - message="Request paused for testing", - action_type=ActionType.access, - ) - - assert log.status == ExecutionLogStatus.info - assert log.connection_key == "test_connection" - assert log.message == "Request paused for testing" - assert log.action_type == ActionType.access - assert log.privacy_request_id == privacy_request.id - log.delete(db) diff --git a/tests/fides/ops/service/privacy_request/test_request_runner_service.py b/tests/fides/ops/service/privacy_request/test_request_runner_service.py index b56bb592099..f73a8406277 100644 --- a/tests/fides/ops/service/privacy_request/test_request_runner_service.py +++ b/tests/fides/ops/service/privacy_request/test_request_runner_service.py @@ -3012,3 +3012,31 @@ def test_consent_runner_soft_time_limit( if "soft time limit" in (log.message or "").lower() ] assert len(timeout_logs) == 1 + + +class TestInfoExecutionLogs: + """Tests that info-level execution logs are created when requests are paused.""" + + @mock.patch( + "fides.api.models.privacy_request.PrivacyRequest.trigger_policy_webhook" + ) + def test_webhook_halt_creates_info_log( + self, + mock_trigger_policy_webhook, + db, + privacy_request, + policy_pre_execution_webhooks, + ): + """When a webhook returns halt=true, an info execution log explains why the request paused.""" + mock_trigger_policy_webhook.side_effect = PrivacyRequestPaused( + "Request received to halt" + ) + + proceed = run_webhooks_and_report_status(db, privacy_request, PolicyPreWebhook) + assert not proceed + assert privacy_request.status == PrivacyRequestStatus.paused + + info_logs = privacy_request.execution_logs.filter_by(status="info").all() + assert len(info_logs) == 1 + assert "paused by webhook" in info_logs[0].message.lower() + assert info_logs[0].connection_key == policy_pre_execution_webhooks[0].key From a83dfdc5ae2dbfe2b41d67a598753782411e8634 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 14:12:44 +0200 Subject: [PATCH 08/11] Update changelog to reflect scoped-down info logs Co-Authored-By: Claude Opus 4.6 (1M context) --- changelog/8251-info-execution-logs-for-status-transitions.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/8251-info-execution-logs-for-status-transitions.yaml b/changelog/8251-info-execution-logs-for-status-transitions.yaml index 3da2c5802f1..dfcc871d5bb 100644 --- a/changelog/8251-info-execution-logs-for-status-transitions.yaml +++ b/changelog/8251-info-execution-logs-for-status-transitions.yaml @@ -1,4 +1,4 @@ type: Changed -description: Added info-level execution logs for privacy request status transitions (requires_input, paused, awaiting_access_review, pre_approval_not_eligible) to improve troubleshooting +description: Added info-level execution logs for paused privacy requests to surface why a request was paused (webhook halt or batch email send) pr: 8251 labels: [] From d8ae01bc8f0ee7ce3d6c42ce411b5a56ecf558ee Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 14:17:36 +0200 Subject: [PATCH 09/11] Add Celery on_failure handler for worker-level DSR task deaths When a privacy request task dies at the worker level (OOM kill, hard timeout, broker disconnect), no execution log is created. The on_failure callback on DatabaseTask catches these failures, writes an error execution log with the failure reason, and marks the request as errored. Skips if the in-task BaseException handler already handled the error. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/api/tasks/__init__.py | 55 ++++++++++++ tests/fides/ops/tasks/test_database_task.py | 95 +++++++++++++++++++++ 2 files changed, 150 insertions(+) diff --git a/src/fides/api/tasks/__init__.py b/src/fides/api/tasks/__init__.py index 9951b615874..253396a8169 100644 --- a/src/fides/api/tasks/__init__.py +++ b/src/fides/api/tasks/__init__.py @@ -58,6 +58,61 @@ class DatabaseTask(Task): # pylint: disable=W0223 _task_engine = None _sessionmaker = None + def on_failure( + self, exc: BaseException, task_id: str, args: tuple, kwargs: dict, einfo: Any + ) -> None: + """Log an execution log when a privacy request task fails at the worker level. + + Catches failures that bypass the task's own exception handling: hard time + limit exceeded, worker killed, broker disconnect, etc. Skips if the + in-task BaseException catch-all already handled it (status already error). + Only applies to tasks with a privacy_request_id kwarg; other tasks are ignored. + """ + privacy_request_id = kwargs.get("privacy_request_id") + if not privacy_request_id: + return + + try: + session = self.get_new_session() + try: + from fides.api.models.privacy_request import PrivacyRequest + from fides.api.schemas.privacy_request import PrivacyRequestStatus + + privacy_request = ( + session.query(PrivacyRequest) + .filter(PrivacyRequest.id == privacy_request_id) + .first() + ) + if not privacy_request: + return + + if privacy_request.status == PrivacyRequestStatus.error: + return + + logger.error( + "Privacy request '{}' failed at worker level: {}", + privacy_request_id, + str(exc), + ) + privacy_request.add_error_execution_log( + session, + connection_key=None, + dataset_name="Worker task failure", + collection_name=None, + message=f"Task failed at worker level: {type(exc).__name__}: {exc}", + action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] + ) + privacy_request.error_processing(db=session) + session.commit() + finally: + session.close() + except Exception: # pylint: disable=broad-except + logger.error( + "Failed to log worker-level failure for privacy request '{}': {}", + privacy_request_id, + str(exc), + ) + # This retry will attempt to connect 5 times with an exponential backoff (2, 4, 8, 16 seconds between each attempt). # The original error will be re-raised if the retries are successful. All attempts are shown in the logs. @retry( diff --git a/tests/fides/ops/tasks/test_database_task.py b/tests/fides/ops/tasks/test_database_task.py index ce799a0b64d..af0e6a44ab0 100644 --- a/tests/fides/ops/tasks/test_database_task.py +++ b/tests/fides/ops/tasks/test_database_task.py @@ -1,6 +1,7 @@ # pylint: disable=protected-access from unittest import mock +from unittest.mock import MagicMock, Mock, patch import pytest from sqlalchemy.engine import Engine @@ -73,3 +74,97 @@ def test_max_retries_exceeded(mock_db_task, always_failing_session_maker): with task.get_new_session(): pass assert always_failing_session_maker.call_count == NEW_SESSION_RETRIES + + +class TestDatabaseTaskOnFailure: + """Tests for the on_failure handler that logs worker-level task deaths.""" + + def test_on_failure_skips_non_privacy_request_tasks(self): + """Tasks without privacy_request_id in kwargs are ignored.""" + task = DatabaseTask() + task.on_failure( + exc=RuntimeError("boom"), + task_id="test-task-id", + args=(), + kwargs={"some_other_param": "value"}, + einfo=None, + ) + # No exception raised, no DB interaction + + @patch.object(DatabaseTask, "get_new_session") + def test_on_failure_creates_error_log_for_worker_death(self, mock_get_session): + """When a privacy request task dies at the worker level, an error + execution log is created and the request is marked as errored.""" + mock_session = MagicMock() + mock_get_session.return_value = mock_session + + mock_privacy_request = MagicMock() + mock_privacy_request.status = MagicMock() + mock_privacy_request.status.__eq__ = lambda self, other: False # not already errored + mock_privacy_request.policy.get_action_type.return_value = "access" + + mock_session.query.return_value.filter.return_value.first.return_value = ( + mock_privacy_request + ) + + task = DatabaseTask() + task.on_failure( + exc=RuntimeError("Worker killed by OOM"), + task_id="test-task-id", + args=(), + kwargs={"privacy_request_id": "test-pr-id"}, + einfo=None, + ) + + mock_privacy_request.add_error_execution_log.assert_called_once() + call_kwargs = mock_privacy_request.add_error_execution_log.call_args + assert "Worker killed by OOM" in call_kwargs[1]["message"] or "Worker killed by OOM" in str(call_kwargs) + assert call_kwargs[1]["dataset_name"] == "Worker task failure" + + mock_privacy_request.error_processing.assert_called_once_with(db=mock_session) + mock_session.commit.assert_called_once() + mock_session.close.assert_called_once() + + @patch.object(DatabaseTask, "get_new_session") + def test_on_failure_skips_already_errored_request(self, mock_get_session): + """If the in-task exception handler already handled the error, on_failure is a no-op.""" + mock_session = MagicMock() + mock_get_session.return_value = mock_session + + # Simulate PrivacyRequestStatus.error comparison + from fides.api.schemas.privacy_request import PrivacyRequestStatus + + mock_privacy_request = MagicMock() + mock_privacy_request.status = PrivacyRequestStatus.error + + mock_session.query.return_value.filter.return_value.first.return_value = ( + mock_privacy_request + ) + + task = DatabaseTask() + task.on_failure( + exc=RuntimeError("boom"), + task_id="test-task-id", + args=(), + kwargs={"privacy_request_id": "test-pr-id"}, + einfo=None, + ) + + mock_privacy_request.add_error_execution_log.assert_not_called() + mock_privacy_request.error_processing.assert_not_called() + mock_session.close.assert_called_once() + + @patch.object(DatabaseTask, "get_new_session") + def test_on_failure_handles_db_errors_gracefully(self, mock_get_session): + """If the DB is unavailable during on_failure, the error is logged but not raised.""" + mock_get_session.side_effect = OperationalError("DB down", None, None) + + task = DatabaseTask() + # Should not raise + task.on_failure( + exc=RuntimeError("original error"), + task_id="test-task-id", + args=(), + kwargs={"privacy_request_id": "test-pr-id"}, + einfo=None, + ) From c92421b19ae1e66c9d5ec1e3c789b2b161b8effd Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 14:17:57 +0200 Subject: [PATCH 10/11] Apply ruff formatting to test file Co-Authored-By: Claude Opus 4.6 (1M context) --- tests/fides/ops/tasks/test_database_task.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/fides/ops/tasks/test_database_task.py b/tests/fides/ops/tasks/test_database_task.py index af0e6a44ab0..de20974450b 100644 --- a/tests/fides/ops/tasks/test_database_task.py +++ b/tests/fides/ops/tasks/test_database_task.py @@ -100,7 +100,9 @@ def test_on_failure_creates_error_log_for_worker_death(self, mock_get_session): mock_privacy_request = MagicMock() mock_privacy_request.status = MagicMock() - mock_privacy_request.status.__eq__ = lambda self, other: False # not already errored + mock_privacy_request.status.__eq__ = ( + lambda self, other: False + ) # not already errored mock_privacy_request.policy.get_action_type.return_value = "access" mock_session.query.return_value.filter.return_value.first.return_value = ( @@ -118,7 +120,9 @@ def test_on_failure_creates_error_log_for_worker_death(self, mock_get_session): mock_privacy_request.add_error_execution_log.assert_called_once() call_kwargs = mock_privacy_request.add_error_execution_log.call_args - assert "Worker killed by OOM" in call_kwargs[1]["message"] or "Worker killed by OOM" in str(call_kwargs) + assert "Worker killed by OOM" in call_kwargs[1][ + "message" + ] or "Worker killed by OOM" in str(call_kwargs) assert call_kwargs[1]["dataset_name"] == "Worker task failure" mock_privacy_request.error_processing.assert_called_once_with(db=mock_session) From 6f305031c42b686bb2ac957c60cc157ab2f0c2d0 Mon Sep 17 00:00:00 2001 From: eastandwestwind Date: Thu, 21 May 2026 14:19:34 +0200 Subject: [PATCH 11/11] =?UTF-8?q?Revert=20on=5Ffailure=20handler=20?= =?UTF-8?q?=E2=80=94=20moving=20to=20separate=20PR=20(ENG-3925)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 (1M context) --- src/fides/api/tasks/__init__.py | 55 ------------ tests/fides/ops/tasks/test_database_task.py | 99 --------------------- 2 files changed, 154 deletions(-) diff --git a/src/fides/api/tasks/__init__.py b/src/fides/api/tasks/__init__.py index 253396a8169..9951b615874 100644 --- a/src/fides/api/tasks/__init__.py +++ b/src/fides/api/tasks/__init__.py @@ -58,61 +58,6 @@ class DatabaseTask(Task): # pylint: disable=W0223 _task_engine = None _sessionmaker = None - def on_failure( - self, exc: BaseException, task_id: str, args: tuple, kwargs: dict, einfo: Any - ) -> None: - """Log an execution log when a privacy request task fails at the worker level. - - Catches failures that bypass the task's own exception handling: hard time - limit exceeded, worker killed, broker disconnect, etc. Skips if the - in-task BaseException catch-all already handled it (status already error). - Only applies to tasks with a privacy_request_id kwarg; other tasks are ignored. - """ - privacy_request_id = kwargs.get("privacy_request_id") - if not privacy_request_id: - return - - try: - session = self.get_new_session() - try: - from fides.api.models.privacy_request import PrivacyRequest - from fides.api.schemas.privacy_request import PrivacyRequestStatus - - privacy_request = ( - session.query(PrivacyRequest) - .filter(PrivacyRequest.id == privacy_request_id) - .first() - ) - if not privacy_request: - return - - if privacy_request.status == PrivacyRequestStatus.error: - return - - logger.error( - "Privacy request '{}' failed at worker level: {}", - privacy_request_id, - str(exc), - ) - privacy_request.add_error_execution_log( - session, - connection_key=None, - dataset_name="Worker task failure", - collection_name=None, - message=f"Task failed at worker level: {type(exc).__name__}: {exc}", - action_type=privacy_request.policy.get_action_type(), # type: ignore[arg-type] - ) - privacy_request.error_processing(db=session) - session.commit() - finally: - session.close() - except Exception: # pylint: disable=broad-except - logger.error( - "Failed to log worker-level failure for privacy request '{}': {}", - privacy_request_id, - str(exc), - ) - # This retry will attempt to connect 5 times with an exponential backoff (2, 4, 8, 16 seconds between each attempt). # The original error will be re-raised if the retries are successful. All attempts are shown in the logs. @retry( diff --git a/tests/fides/ops/tasks/test_database_task.py b/tests/fides/ops/tasks/test_database_task.py index de20974450b..ce799a0b64d 100644 --- a/tests/fides/ops/tasks/test_database_task.py +++ b/tests/fides/ops/tasks/test_database_task.py @@ -1,7 +1,6 @@ # pylint: disable=protected-access from unittest import mock -from unittest.mock import MagicMock, Mock, patch import pytest from sqlalchemy.engine import Engine @@ -74,101 +73,3 @@ def test_max_retries_exceeded(mock_db_task, always_failing_session_maker): with task.get_new_session(): pass assert always_failing_session_maker.call_count == NEW_SESSION_RETRIES - - -class TestDatabaseTaskOnFailure: - """Tests for the on_failure handler that logs worker-level task deaths.""" - - def test_on_failure_skips_non_privacy_request_tasks(self): - """Tasks without privacy_request_id in kwargs are ignored.""" - task = DatabaseTask() - task.on_failure( - exc=RuntimeError("boom"), - task_id="test-task-id", - args=(), - kwargs={"some_other_param": "value"}, - einfo=None, - ) - # No exception raised, no DB interaction - - @patch.object(DatabaseTask, "get_new_session") - def test_on_failure_creates_error_log_for_worker_death(self, mock_get_session): - """When a privacy request task dies at the worker level, an error - execution log is created and the request is marked as errored.""" - mock_session = MagicMock() - mock_get_session.return_value = mock_session - - mock_privacy_request = MagicMock() - mock_privacy_request.status = MagicMock() - mock_privacy_request.status.__eq__ = ( - lambda self, other: False - ) # not already errored - mock_privacy_request.policy.get_action_type.return_value = "access" - - mock_session.query.return_value.filter.return_value.first.return_value = ( - mock_privacy_request - ) - - task = DatabaseTask() - task.on_failure( - exc=RuntimeError("Worker killed by OOM"), - task_id="test-task-id", - args=(), - kwargs={"privacy_request_id": "test-pr-id"}, - einfo=None, - ) - - mock_privacy_request.add_error_execution_log.assert_called_once() - call_kwargs = mock_privacy_request.add_error_execution_log.call_args - assert "Worker killed by OOM" in call_kwargs[1][ - "message" - ] or "Worker killed by OOM" in str(call_kwargs) - assert call_kwargs[1]["dataset_name"] == "Worker task failure" - - mock_privacy_request.error_processing.assert_called_once_with(db=mock_session) - mock_session.commit.assert_called_once() - mock_session.close.assert_called_once() - - @patch.object(DatabaseTask, "get_new_session") - def test_on_failure_skips_already_errored_request(self, mock_get_session): - """If the in-task exception handler already handled the error, on_failure is a no-op.""" - mock_session = MagicMock() - mock_get_session.return_value = mock_session - - # Simulate PrivacyRequestStatus.error comparison - from fides.api.schemas.privacy_request import PrivacyRequestStatus - - mock_privacy_request = MagicMock() - mock_privacy_request.status = PrivacyRequestStatus.error - - mock_session.query.return_value.filter.return_value.first.return_value = ( - mock_privacy_request - ) - - task = DatabaseTask() - task.on_failure( - exc=RuntimeError("boom"), - task_id="test-task-id", - args=(), - kwargs={"privacy_request_id": "test-pr-id"}, - einfo=None, - ) - - mock_privacy_request.add_error_execution_log.assert_not_called() - mock_privacy_request.error_processing.assert_not_called() - mock_session.close.assert_called_once() - - @patch.object(DatabaseTask, "get_new_session") - def test_on_failure_handles_db_errors_gracefully(self, mock_get_session): - """If the DB is unavailable during on_failure, the error is logged but not raised.""" - mock_get_session.side_effect = OperationalError("DB down", None, None) - - task = DatabaseTask() - # Should not raise - task.on_failure( - exc=RuntimeError("original error"), - task_id="test-task-id", - args=(), - kwargs={"privacy_request_id": "test-pr-id"}, - einfo=None, - )