Skip to content

Conversation

@galvana
Copy link
Contributor

@galvana galvana commented Jan 15, 2026

Ticket ENG-2403

Description Of Changes

This PR improves error visibility for privacy request scheduling and processing failures. Previously, when a privacy request failed to schedule (e.g., due to memory limits or Celery issues) or failed during post-processing steps, the request would get stuck in an invalid state without any indication in the activity timeline of what went wrong.

Key improvements:

  • When scheduling fails, the request now transitions to error status with a descriptive entry in the activity timeline
  • When scheduling succeeds (especially after a previous failure), a success entry is logged to clear the error styling in the UI
  • All error paths in run_privacy_request now log descriptive errors to the activity timeline
  • The retry endpoint now returns the actual error message to the frontend for display in the toast notification

Code Changes

Backend - Scheduling error handling (privacy_request_service.py):

  • Added _handle_scheduling_failure helper to mark privacy requests as errored and create execution log entries
  • Added _log_scheduling_success helper to create success execution log entries when scheduling succeeds
  • Wrapped queue_privacy_request in try/except to catch and handle scheduling failures
  • Updated _process_privacy_request_restart to raise HTTPException with the actual error message on failure
  • Updated handle_approval to catch and log scheduling failures during initial request creation

Backend - Processing error handling (request_runner_service.py):

  • Added error execution log for MisconfiguredPolicyException (policy has no rules)
  • Added error execution log for the BaseException catch-all in the main processing try block
  • Added error execution log for expired privacy requests
  • Added error execution logs for webhook failures (ClientUnsuccessfulException and PydanticValidationError)
  • Wrapped post-processing steps (email send, post webhooks, finalization) in try/except with error logging

Frontend (useReprocessPrivacyRequest.ts):

  • Removed the else if block that checked for error_message in the response - no longer needed since the backend now raises an HTTPException with the error message instead of returning a 200 with error status

Frontend (RequestDetails.tsx):

  • Removed the error alert banner since errors are now visible in the activity timeline

Frontend (types.ts):

  • Removed the error_message field from PrivacyRequestEntity type

Steps to Confirm

The tests should still pass. This PR is more to help troubleshoot stuck privacy requests that don't have any clear errors.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@galvana galvana requested a review from a team as a code owner January 15, 2026 22:00
@galvana galvana requested review from johnewart and removed request for a team January 15, 2026 22:00
@vercel
Copy link
Contributor

vercel bot commented Jan 15, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Jan 16, 2026 11:33pm
fides-privacy-center Ignored Ignored Jan 16, 2026 11:33pm

Request Review

@galvana galvana requested review from JadeCara and removed request for johnewart January 15, 2026 22:00
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 15, 2026

Greptile Summary

This PR improves error visibility and handling throughout the privacy request lifecycle by adding execution log entries and proper error state transitions when failures occur during scheduling, processing, and post-processing steps.

Key improvements:

  • Scheduling failures now properly transition requests to error state with descriptive activity timeline entries
  • Post-processing steps (email send, webhooks, finalization) wrapped in exception handling with error logging
  • Webhook failures (ClientUnsuccessfulException and PydanticValidationError) now create execution log entries
  • Retry endpoint returns actual error messages to frontend instead of 200 with error status

Issues to address before merging:

  • Test code left in production (line 967-968 in privacy_request_service.py)
  • Mid-file imports that should be moved to top of module (lines 1155-1156)

Confidence Score: 4/5

  • This PR improves error handling and logging but contains test code that must be removed before merging
  • The changes add comprehensive error handling and logging which is positive for debugging. However, the commented-out test code on lines 967-968 needs to be removed, and imports should be moved to the top of the file following code style conventions. The logic is sound but these issues prevent a higher confidence score.
  • Pay close attention to src/fides/service/privacy_request/privacy_request_service.py - contains test code that must be removed

Important Files Changed

Filename Overview
src/fides/service/privacy_request/privacy_request_service.py Added scheduling failure handlers and wrapped queue_privacy_request calls in try/catch blocks. Contains mid-file imports and test code.
src/fides/api/service/privacy_request/request_runner_service.py Added comprehensive error logging throughout privacy request processing, webhooks, and post-processing steps.
src/fides/api/task/execute_request_tasks.py Added exception handling when queueing privacy requests after terminator node.

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@JadeCara JadeCara left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor beat me to it but the temp code to cause an error should probably be removed before merging - other than that looks good to me.

galvana and others added 5 commits January 16, 2026 13:12
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants