Fail a pending delivery whose target was deleted (closes #293)

Restart recovery and the pending sweep skipped a pending delivery
whose target was missing from the batch's target map, and the sweep
did so again every minute for the life of the database. A miss now
asks loadTarget: no row fails the delivery with a recorded reason,
through the ownership-gated function the retrying paths already use,
renamed failMissingTarget with its log line and reason text made to
fit both statuses. Any other error leaves the delivery pending,
because the map is also empty when its query failed.

Model: opus-5-5
This commit is contained in:
2026-09-29 04:13:05 +00:00
parent e0b211f960
commit a0bbfca28d
2 changed files with 208 additions and 32 deletions
+35 -23
View File
@@ -728,9 +728,7 @@ func (e *Engine) recoverSingleRetry(
// webhook on one bad read would be a far larger fault than
// the strand it is meant to clear.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTargetRetry(
webhookDB, webhookID, d,
)
e.failMissingTarget(webhookDB, webhookID, d)
return
}
@@ -1133,9 +1131,7 @@ func (e *Engine) sweepSingleRetry(
// Deleted is terminal, unreadable is not; see
// recoverSingleRetry.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTargetRetry(
webhookDB, webhookID, d,
)
e.failMissingTarget(webhookDB, webhookID, d)
return
}
@@ -1249,19 +1245,19 @@ func (e *Engine) failUnretryableRetry(
e.failDelivery(webhookDB, d, target.Type, reason)
}
// failMissingTargetRetry terminally fails an orphaned retrying
// delivery whose target row is gone. Both restart recovery and the
// periodic sweep call it, so the transition exists once.
// failMissingTarget terminally fails a recovered delivery, pending or
// retrying, whose target row is gone. Restart recovery and the periodic
// sweep call it for both statuses, so the transition exists once.
//
// Until it existed both paths logged the failed lookup and returned,
// which left the delivery retrying for the life of the database and
// Until it existed those paths logged the failed lookup and moved on,
// which left the delivery where it was for the life of the database and
// the sweep repeating the same error every minute forever. Failing it
// with a recorded reason is the treatment the other orphaned-retry
// cases already get, so all of them read alike in the event log.
//
// Logged at warn rather than error: a deleted target is an operator
// action, not a system fault.
func (e *Engine) failMissingTargetRetry(
func (e *Engine) failMissingTarget(
webhookDB *gorm.DB,
webhookID string,
d *database.Delivery,
@@ -1277,10 +1273,10 @@ func (e *Engine) failMissingTargetRetry(
targetType, reason := e.missingTargetReason(d.TargetID)
e.log.Warn(
"failing orphaned retrying delivery: "+
"its target no longer exists",
"failing recovered delivery: its target no longer exists",
"webhook_id", webhookID,
"delivery_id", d.ID,
"status", d.Status,
"target_id", d.TargetID,
"target_type", targetType,
)
@@ -1314,15 +1310,14 @@ func (e *Engine) missingTargetReason(
if err != nil {
return "", fmt.Sprintf(
"target %s no longer exists; the delivery "+
"cannot be retried and has been failed "+
"terminally",
"has been failed terminally",
targetID,
)
}
return target.Type, fmt.Sprintf(
"target %q (type %s) was deleted; the delivery "+
"cannot be retried and has been failed terminally",
"has been failed terminally",
target.Name, target.Type,
)
}
@@ -2021,13 +2016,30 @@ func (e *Engine) sendRecoveredDeliveries(
target, ok := targetMap[deliveries[i].TargetID]
if !ok {
e.log.Error(
"target not found for delivery",
"delivery_id", deliveries[i].ID,
"target_id", deliveries[i].TargetID,
)
// A missing entry does not mean the target is gone: the
// map is also empty when its query failed. Only a lookup
// that finds no row ends the delivery; any other error
// leaves it pending for the next sweep. See
// recoverSingleRetry.
var err error
continue
target, err = e.loadTarget(deliveries[i].TargetID)
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, &deliveries[i])
continue
}
if err != nil {
e.log.Error(
"failed to load target for recovered delivery",
"delivery_id", deliveries[i].ID,
"target_id", deliveries[i].TargetID,
"error", err,
)
continue
}
}
if !e.takeForRedispatch(