Terminally fail retrying deliveries with a non-retry target type (closes #82) (#104)
Some checks failed
check / check (push) Has been cancelled
Some checks failed
check / check (push) Has been cancelled
A delivery left in `retrying` whose target type was edited to a fire-and-forget or unknown type was skipped forever by both restart recovery and the retry sweep. Both paths now record a result row and mark it `failed`.
This commit was merged in pull request #104.
This commit is contained in:
@@ -741,6 +741,193 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("expected retry task from recovery")
|
||||
}
|
||||
|
||||
// Regression guard: a target that still supports retries
|
||||
// must be rescheduled, never terminally failed, and must
|
||||
// not gain a synthetic result row.
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, d.ID,
|
||||
database.DeliveryStatusRetrying,
|
||||
)
|
||||
|
||||
assert.Len(t, iResults(t, s.WebhookDB, d.ID), 1)
|
||||
}
|
||||
|
||||
// --- Retrying deliveries whose target type changed ---
|
||||
|
||||
// iSeedRetryingWithType seeds a retrying delivery with one
|
||||
// recorded failed attempt against a target of the given type,
|
||||
// standing in for a target whose type was edited in the main
|
||||
// database while the delivery was still retrying.
|
||||
func iSeedRetryingWithType(
|
||||
t *testing.T,
|
||||
s iSetup,
|
||||
targetType database.TargetType,
|
||||
) string {
|
||||
t.Helper()
|
||||
|
||||
targetID := uuid.New().String()
|
||||
|
||||
iCreateTarget(t, s.MainDB, targetID,
|
||||
s.WebhookID, "mutated-target", targetType,
|
||||
iHTTPConfig("http://example.com/hook"), 5,
|
||||
)
|
||||
|
||||
event := iSeedEvent(
|
||||
t, s.WebhookDB, s.WebhookID,
|
||||
`{"orphaned":"retry"}`,
|
||||
)
|
||||
|
||||
d := iSeedDelivery(
|
||||
t, s.WebhookDB, event.ID, targetID,
|
||||
database.DeliveryStatusRetrying,
|
||||
)
|
||||
|
||||
iSeedFailedResult(t, s.WebhookDB, d.ID)
|
||||
|
||||
return d.ID
|
||||
}
|
||||
|
||||
// iResults loads a delivery's results in attempt order.
|
||||
func iResults(
|
||||
t *testing.T, db *gorm.DB, deliveryID string,
|
||||
) []database.DeliveryResult {
|
||||
t.Helper()
|
||||
|
||||
var results []database.DeliveryResult
|
||||
|
||||
require.NoError(t, db.
|
||||
Where("delivery_id = ?", deliveryID).
|
||||
Order("attempt_num").
|
||||
Find(&results).Error)
|
||||
|
||||
return results
|
||||
}
|
||||
|
||||
// iAssertTerminallyFailed asserts the delivery ended failed
|
||||
// with a result row recording why, and was not rescheduled.
|
||||
func iAssertTerminallyFailed(
|
||||
t *testing.T,
|
||||
s iSetup,
|
||||
deliveryID string,
|
||||
targetType database.TargetType,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, deliveryID,
|
||||
database.DeliveryStatusFailed,
|
||||
)
|
||||
|
||||
results := iResults(t, s.WebhookDB, deliveryID)
|
||||
require.Len(t, results, 2)
|
||||
|
||||
last := results[1]
|
||||
|
||||
assert.False(t, last.Success)
|
||||
assert.Equal(t, 2, last.AttemptNum)
|
||||
|
||||
assert.Contains(
|
||||
t, last.Error, string(targetType),
|
||||
)
|
||||
|
||||
assert.Contains(
|
||||
t, last.Error, "does not support retries",
|
||||
)
|
||||
|
||||
assert.Empty(t, s.Engine.ExportRetryCh())
|
||||
}
|
||||
|
||||
func TestRecoverSingleRetry_TypeNoLongerRetries(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
|
||||
iCreateWebhook(
|
||||
t, s.MainDB, s.WebhookID, "mutated-type",
|
||||
)
|
||||
|
||||
deliveryID := iSeedRetryingWithType(
|
||||
t, s, database.TargetTypeLog,
|
||||
)
|
||||
|
||||
s.Engine.ExportRecoverWebhookDeliveries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
iAssertTerminallyFailed(
|
||||
t, s, deliveryID, database.TargetTypeLog,
|
||||
)
|
||||
}
|
||||
|
||||
func TestSweepSingleRetry_TypeNoLongerRetries(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
|
||||
iCreateWebhook(
|
||||
t, s.MainDB, s.WebhookID, "mutated-type-sweep",
|
||||
)
|
||||
|
||||
deliveryID := iSeedRetryingWithType(
|
||||
t, s, database.TargetTypeDatabase,
|
||||
)
|
||||
|
||||
s.Engine.ExportSweepWebhookRetries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
iAssertTerminallyFailed(
|
||||
t, s, deliveryID, database.TargetTypeDatabase,
|
||||
)
|
||||
}
|
||||
|
||||
func TestRecoverSingleRetry_UnknownTargetType(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
|
||||
iCreateWebhook(
|
||||
t, s.MainDB, s.WebhookID, "unknown-type",
|
||||
)
|
||||
|
||||
unknown := database.TargetType("not-a-target-type")
|
||||
|
||||
deliveryID := iSeedRetryingWithType(t, s, unknown)
|
||||
|
||||
s.Engine.ExportRecoverWebhookDeliveries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
iAssertTerminallyFailed(t, s, deliveryID, unknown)
|
||||
}
|
||||
|
||||
func TestSweepSingleRetry_UnknownTargetType(
|
||||
t *testing.T,
|
||||
) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
|
||||
iCreateWebhook(
|
||||
t, s.MainDB, s.WebhookID, "unknown-type-sweep",
|
||||
)
|
||||
|
||||
unknown := database.TargetType("not-a-target-type")
|
||||
|
||||
deliveryID := iSeedRetryingWithType(t, s, unknown)
|
||||
|
||||
s.Engine.ExportSweepWebhookRetries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
iAssertTerminallyFailed(t, s, deliveryID, unknown)
|
||||
}
|
||||
|
||||
// iSeedFailedResult creates a failed delivery result.
|
||||
|
||||
Reference in New Issue
Block a user