Terminally fail retrying deliveries with a non-retry target type (closes #82)
All checks were successful
check / check (push) Successful in 4m4s
All checks were successful
check / check (push) Successful in 4m4s
Restart recovery and the 60s retry sweep both looked an orphaned `retrying` delivery's target up in the registry and silently returned when it did not implement `rescheduler`. If a target's type was edited from a retry type (`http`/`slack`) to a fire-and-forget type (`database`/`log`) or an unknown one while a delivery was still retrying, that delivery stayed `retrying` forever. Both sites now hand the delivery to one shared helper, `failUnretryableRetry`, which records a `DeliveryResult` naming the current target type as the reason and marks the delivery `failed`. It logs at warn, not error: this is operator-caused state, not a system fault. Re-dispatching under the new type was rejected as it would perform a delivery the operator never asked for; the event itself stays in the per-webhook event database, so manual redelivery can recover it deliberately. Fire-and-forget targets never set status `retrying` under normal operation, so this path stays unreachable for them in practice.
This commit is contained in:
@@ -453,8 +453,9 @@ func (e *Engine) recoverRetryingDeliveries(
|
||||
// recoverSingleRetry hands an orphaned retrying delivery back
|
||||
// to its target to recompute the remaining backoff, then
|
||||
// reschedules it. Targets that do not own durable retries
|
||||
// (fire-and-forget) never produce retrying deliveries, so
|
||||
// they are skipped.
|
||||
// (fire-and-forget) never produce retrying deliveries, so a
|
||||
// delivery found in that state has had its target's type
|
||||
// changed underneath it and is terminally failed.
|
||||
func (e *Engine) recoverSingleRetry(
|
||||
webhookDB *gorm.DB,
|
||||
webhookID string,
|
||||
@@ -475,6 +476,10 @@ func (e *Engine) recoverSingleRetry(
|
||||
|
||||
rs, ok := e.targets[target.Type].(rescheduler)
|
||||
if !ok {
|
||||
e.failUnretryableRetry(
|
||||
webhookDB, webhookID, d, &target,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
@@ -649,8 +654,8 @@ func (e *Engine) sweepWebhookRetries(
|
||||
|
||||
// sweepSingleRetry re-enqueues an orphaned retrying delivery
|
||||
// whose backoff window has elapsed, delegating the backoff
|
||||
// decision to the delivery's target. Targets that do not own
|
||||
// durable retries are skipped.
|
||||
// decision to the delivery's target. A delivery whose target
|
||||
// no longer owns durable retries is terminally failed.
|
||||
func (e *Engine) sweepSingleRetry(
|
||||
webhookDB *gorm.DB,
|
||||
webhookID string,
|
||||
@@ -670,6 +675,10 @@ func (e *Engine) sweepSingleRetry(
|
||||
|
||||
rs, ok := e.targets[target.Type].(rescheduler)
|
||||
if !ok {
|
||||
e.failUnretryableRetry(
|
||||
webhookDB, webhookID, d, &target,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
@@ -710,6 +719,59 @@ func (e *Engine) sweepSingleRetry(
|
||||
}
|
||||
}
|
||||
|
||||
// failUnretryableRetry terminally fails an orphaned retrying
|
||||
// delivery whose target type no longer supports retries. Both
|
||||
// restart recovery and the periodic sweep call it, so the
|
||||
// terminal transition exists once.
|
||||
//
|
||||
// This is only reachable when a target's type has been changed
|
||||
// out from under an in-flight retrying delivery (or the type is
|
||||
// unknown to the registry): fire-and-forget targets never set
|
||||
// status retrying themselves. Re-dispatching under the new type
|
||||
// would be a delivery the operator never asked for, and leaving
|
||||
// the row retrying strands it forever, so the delivery is
|
||||
// failed with a recorded reason and can be redelivered
|
||||
// manually. Logged at warn, not error: this is operator-caused
|
||||
// state, not a system fault.
|
||||
func (e *Engine) failUnretryableRetry(
|
||||
webhookDB *gorm.DB,
|
||||
webhookID string,
|
||||
d *database.Delivery,
|
||||
target *database.Target,
|
||||
) {
|
||||
e.log.Warn(
|
||||
"failing orphaned retrying delivery: target "+
|
||||
"type no longer supports retries",
|
||||
"webhook_id", webhookID,
|
||||
"delivery_id", d.ID,
|
||||
"target_id", target.ID,
|
||||
"target_name", target.Name,
|
||||
"target_type", target.Type,
|
||||
)
|
||||
|
||||
reason := fmt.Sprintf(
|
||||
"target type %q does not support retries; "+
|
||||
"delivery was left retrying by a previous "+
|
||||
"target type and has been failed terminally",
|
||||
target.Type,
|
||||
)
|
||||
|
||||
e.recordResult(
|
||||
webhookDB,
|
||||
d,
|
||||
e.countAttempts(webhookDB, d.ID)+1,
|
||||
false,
|
||||
0,
|
||||
"",
|
||||
reason,
|
||||
0,
|
||||
)
|
||||
|
||||
e.updateDeliveryStatus(
|
||||
webhookDB, d, database.DeliveryStatusFailed,
|
||||
)
|
||||
}
|
||||
|
||||
// processDelivery dispatches a delivery to the target that
|
||||
// owns its type. Unknown target types fail the delivery.
|
||||
func (e *Engine) processDelivery(
|
||||
|
||||
Reference in New Issue
Block a user