Skip to content

Eng 2714 add check for callbacks on requeue#7431

Merged
Vagoasdf merged 9 commits intomainfrom
ENG-2714_add-check-for-callbacks-on-requeue
Feb 20, 2026
Merged

Eng 2714 add check for callbacks on requeue#7431
Vagoasdf merged 9 commits intomainfrom
ENG-2714_add-check-for-callbacks-on-requeue

Conversation

@Vagoasdf
Copy link
Copy Markdown
Contributor

@Vagoasdf Vagoasdf commented Feb 19, 2026

Ticket ENG-2714

Description Of Changes

Fix requeue_interrupted_tasks incorrectly erroring privacy requests that are legitimately awaiting an async webhook (callback) response.

When a connector uses the callback strategy (e.g. gateway_api with erase_after), the request runner exits after the initial request and waits for the external system to hit our webhook. Downstream tasks remain in pending status with no Celery task ID because they depend on gateway completing first. The periodic requeue_interrupted_tasks job sees those pending tasks with no task ID, determines they are "stuck," and errors the entire privacy request -- even though it is legitimately waiting for a callback.

This was already handled for async polling tasks (via _has_polling_tasks), but not for async callback (webhook) tasks.

Code Changes

  • Replaced _has_polling_tasks with _has_async_tasks_awaiting_external_completion -- a unified helper that returns True if the privacy request has any RequestTask with async_type in (polling, callback).

Steps to Confirm

  1. Set up a SaaS connector with async_config: strategy: callback on its erasure endpoint, and a second connector with erase_after pointing at the first.
  2. Create an erasure policy whose rules target the data categories in both connectors' datasets (e.g. user.unique_id).
  3. Submit a privacy request against that erasure policy with a test identity.
  4. Observe the request state:
    The callback connector's erasure task enters awaiting_processing with async_type=callback (waiting for webhook).
    The dependent connector's erasure task stays pending (blocked by erase_after).
  5. The overall privacy request stays in_processing.
  6. Wait for requeue_interrupted_tasks to fire (runs every 300 seconds by default, configurable via interrupted_task_requeue_interval).
  7. Verify the privacy request status:
    With fix: Status remains in_processing -- the request is correctly recognized as awaiting external callback completion.
    Without fix: Status would change to error because the dependent connector's pending task (no cached Celery task ID) was incorrectly treated as stuck.

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

@Vagoasdf Vagoasdf marked this pull request as ready for review February 19, 2026 19:49
@Vagoasdf Vagoasdf requested a review from a team as a code owner February 19, 2026 19:49
@Vagoasdf Vagoasdf requested review from adamsachs and removed request for a team February 19, 2026 19:49
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 19, 2026

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

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Feb 19, 2026 7:54pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Feb 19, 2026 7:54pm

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 19, 2026

Greptile Summary

This PR extends the interrupted task requeue logic to prevent false positive cancellations of privacy requests that have callback-based async tasks. The helper function _has_polling_tasks was renamed to _has_async_tasks_awaiting_external_completion and now checks for both AsyncTaskType.polling and AsyncTaskType.callback tasks, ensuring that requests awaiting webhooks or other external callbacks are not prematurely canceled when they appear stuck.

Key changes:

  • Function renamed from _has_polling_tasks to _has_async_tasks_awaiting_external_completion for clarity
  • Extended async type check to include both polling and callback types using .in_([AsyncTaskType.polling, AsyncTaskType.callback])
  • Updated log messages to reflect the broader scope of async tasks awaiting external completion
  • Added two comprehensive test cases verifying that both callback and polling async tasks with stuck subtasks are not canceled

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change is well-contained, logically sound, and properly tested. The function rename improves code clarity, the logic extension correctly handles callback async tasks alongside polling tasks, and comprehensive test coverage validates both scenarios. The change follows existing patterns in the codebase.
  • No files require special attention

Important Files Changed

Filename Overview
changelog/7418-add-check-for-callback-on-requeue.yaml Changelog entry correctly documents the bug fix with appropriate type and PR reference
src/fides/api/service/privacy_request/request_service.py Renamed function to _has_async_tasks_awaiting_external_completion and expanded to check both polling and callback async types, preventing false positive task cancellations
tests/task/test_requeue_interrupted_tasks.py Added comprehensive test coverage for callback and polling async task scenarios, verifying that stuck subtasks don't trigger cancellation

Last reviewed commit: 329a040

Copy link
Copy Markdown
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, no comments

Edit Code Review Agent Settings | Greptile

Copy link
Copy Markdown
Contributor

@Linker44 Linker44 left a comment

Choose a reason for hiding this comment

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

GTG!

@Vagoasdf Vagoasdf added this pull request to the merge queue Feb 20, 2026
Merged via the queue into main with commit 5e17fa8 Feb 20, 2026
53 checks passed
@Vagoasdf Vagoasdf deleted the ENG-2714_add-check-for-callbacks-on-requeue branch February 20, 2026 16:14
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.

2 participants