Skip to content

Retry failed queued email notifications - #811

Merged
somethingwithproof merged 10 commits into
Cacti:fix/notification-claim-recoveryfrom
somethingwithproof:fix/retry-notification-failures
Aug 18, 2026
Merged

Retry failed queued email notifications#811
somethingwithproof merged 10 commits into
Cacti:fix/notification-claim-recoveryfrom
somethingwithproof:fix/retry-notification-failures

Conversation

@somethingwithproof

@somethingwithproof somethingwithproof commented Aug 18, 2026

Copy link
Copy Markdown
Member

Summary

  • retry failed queued email notifications up to five attempts
  • schedule bounded exponential backoff from one to eight minutes
  • update grouped deliveries with one owner-scoped CASE statement instead of one query per row
  • keep successful and terminal deliveries claimed while releasing only retryable rows
  • expose retry state in the queue UI with header/cell ordering kept consistent
  • normalize retry errors to valid UTF-8 and the queue schema limit
  • prove the worker still owns every row updated by retry bookkeeping
  • document retry timing and terminal behavior

Closes #784

Stack

This PR is intentionally based on #813. Notification ownership, leases, orphan recovery, and completion safety live only in #813; this PR adds retry policy and UI behavior on top.

Linux Docker validation

  • Pest unit suite: 139 tests, 381 assertions
  • patch coverage against fix/notification-claim-recovery: 100% of 80 measurable changed executable lines
  • PHP 7.4 syntax lint: pass
  • composer.json: tested as supplied and not edited

Overlap audit

No other open plugin_thold PR implements bounded queue retry policy, next-attempt scheduling, grouped retry bookkeeping, or retry UI status. The ownership implementation is not duplicated here because #813 is the explicit base.

@somethingwithproof somethingwithproof self-assigned this Aug 18, 2026
@somethingwithproof
somethingwithproof marked this pull request as ready for review August 18, 2026 02:47
Copilot AI lite review requested due to automatic review settings August 18, 2026 02:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Implements retry semantics for queued email notifications in the Thold plugin by tracking delivery attempts and deferring reprocessing until a scheduled next_attempt, while exposing this state in the notification queue UI and adding a schema migration plus unit coverage.

Changes:

  • Add attempt_count/next_attempt tracking and bounded exponential backoff logic for queued email delivery failures.
  • Ensure claim/drain queries skip notifications that are not yet eligible for retry.
  • Add schema migration (1.8.3), UI columns, documentation, and focused unit tests.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
thold_notify.php Claim only retry-eligible rows (next_attempt ready) before draining.
thold_functions.php Add retry delay + delivery recorder; apply retry gating to queue drains; integrate recorder into send flow.
includes/database.php Add 1.8.3 migration + table definition updates for attempt_count and next_attempt plus index.
notify_queue.php Display attempt count and next retry time in the queue UI.
README.md Document the queue retry behaviour.
INFO Bump plugin version to 1.8.3.
CHANGELOG.md Record the issue #784 retry feature.
tests/Unit/NotificationQueueRetryTest.php Unit coverage for retry delay/recording and retry gating.
tests/Unit/NotificationQueueClaimTest.php Update claim expectations to include retry gating clause.
tests/bootstrap-unit.php Minor formatting/alignment update.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread README.md
Comment thread thold_functions.php Outdated
…y' into fix/retry-notification-failures

# Conflicts:
#	CHANGELOG.md
#	README.md
#	tests/Unit/NotificationQueueClaimTest.php
#	thold_notify.php
@somethingwithproof
somethingwithproof changed the base branch from develop to fix/notification-claim-recovery August 18, 2026 04:49
@somethingwithproof
somethingwithproof changed the base branch from fix/notification-claim-recovery to develop August 18, 2026 07:49
@somethingwithproof
somethingwithproof changed the base branch from develop to fix/notification-claim-recovery August 18, 2026 07:51
@somethingwithproof
somethingwithproof merged commit 0661e29 into Cacti:fix/notification-claim-recovery Aug 18, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notification queue: process claim is never applied, and failed sends are discarded

2 participants