1 Commits

Author SHA1 Message Date
clawbot
0384de4a7b Terminally fail retrying deliveries with a non-retry target type (closes #82)
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.
2026-08-09 05:51:51 +00:00
8 changed files with 325 additions and 601 deletions

View File

@@ -89,27 +89,10 @@ TTY detection, and security headers are always applied.
| `PORT` | HTTP listen port | `8080` |
| `DATA_DIR` | Directory for all SQLite databases | `/var/lib/webhooker` |
| `DEBUG` | Enable debug logging | `false` |
| `MAINTENANCE_MODE` | Serve the maintenance page | `false` |
| `METRICS_USERNAME` | Basic auth username for `/metrics` | `""` |
| `METRICS_PASSWORD` | Basic auth password for `/metrics` | `""` |
| `SENTRY_DSN` | Sentry error reporting DSN | `""` |
#### Invalid values abort startup
The defaults above apply **only** to variables that are unset (or set
to an empty string). A variable that is set but cannot be parsed is a
fatal configuration error: webhooker logs the offending variable and
its value and refuses to start, rather than silently running with a
substituted default. `PORT=eighty`, `DEBUG=ture`, and
`RETENTION_SWEEP_INTERVAL=1 hour` all abort startup. `PORT` must
additionally be a number in the range 165535.
Boolean variables (`DEBUG`, `MAINTENANCE_MODE`) accept exactly the
spellings Go's `strconv.ParseBool` accepts — `1`, `t`, `T`, `TRUE`,
`true`, `True`, `0`, `f`, `F`, `FALSE`, `false`, `False` — and nothing
else. `yes`, `on`, and `off` are rejected rather than quietly treated
as false.
On first startup, webhooker automatically generates a cryptographically
secure session encryption key and stores it in the database. This key
persists across restarts — no manual key management is needed.
@@ -653,6 +636,18 @@ This means:
durable fallback that ensures no retry is permanently lost, even under
extreme backpressure.
**Changing a target's type does not migrate in-flight deliveries.** Only
`http` and `slack` targets own durable retries; `database` and `log`
targets are fire-and-forget and never produce a `retrying` delivery. If a
target's `type` is edited from a retrying type to a non-retrying (or
unknown) one while one of its deliveries is still `retrying`, both
recovery paths above terminally mark that delivery `failed` and record a
`DeliveryResult` naming the current target type as the reason, logging it
at warn level. The delivery is not re-dispatched under the new type — the
operator never asked for that delivery — and the event itself remains
stored in the per-webhook event database, so it can be redelivered
manually.
### Circuit Breaker (HTTP Targets with Retries)
HTTP targets with `max_retries` > 0 are protected by a **per-target circuit breaker** that

32
TODO.md
View File

@@ -10,28 +10,28 @@
# Status
pre-1.0. No git tags exist. main (4f5ecb1) is a working webhook proxy
pre-1.0. No git tags exist. main (afe88c6) is a working webhook proxy
with auth, CSRF/SSRF protections, login rate limiting, Slack target,
policy compliance (#6), pinned lint tooling (#55), a per-webhook event
retention reaper (#63), and fail-loud configuration parsing (#80). Note:
TODO.md was deliberately deleted from this repo in f9a9569 (2026-03-01,
#6); its content was folded into the README TODO section, which this
draft reconstructs as of 2026-07-06.
policy compliance (#6), and pinned lint tooling (#55). Note: TODO.md was
deliberately deleted from this repo in f9a9569 (2026-03-01, #6); its
content was folded into the README TODO section, which this draft
reconstructs as of 2026-07-06.
# Next Step
Manual event redelivery from the web UI (replay is a core promised
capability in the README rationale).
Implement automatic event retention cleanup based on retention_days: a
periodic maintenance job that deletes Events, Deliveries, and
DeliveryResults older than the parent webhook's retention_days from each
per-webhook event database. The field exists on the Webhook model and
the README promises the behavior, but nothing enforces it, so event
databases currently grow without bound.
# Completed Steps
- 2026-08-09 Configuration parsing fails loudly on set-but-unparseable
environment values: `envInt` removed in favour of `envPositiveInt`
plus a `PORT` range check, `envBool` now parses with
`strconv.ParseBool`, and defaults apply only to unset variables (#80)
- 2026-08-07 Automatic event retention cleanup based on
`retention_days`, deleting expired events, deliveries, and delivery
results from each per-webhook event database (#63)
- 2026-08-09 Restart recovery and the 60s retry sweep terminally fail an
orphaned `retrying` delivery whose target type no longer supports
retries, recording a `DeliveryResult` with the reason instead of
leaving the delivery stuck forever (#82)
- 2026-08-07 Update golangci-lint to v2.12.2 (Docker image digest in
`Dockerfile`, release-archive sha256 pins in `script/bootstrap`),
adopt the canonical `.golangci.yml` (v2 `linters.settings` layout so
@@ -60,6 +60,8 @@ capability in the README rationale).
# Future Steps
- Manual event redelivery from the web UI (replay is a core promised
capability in the README rationale)
- Delivery status and retry management UI
- Per-webhook rate limiting in the receiver handler (per-webhook config
plus handler enforcement; global limits must not apply to receiver

View File

@@ -7,6 +7,7 @@ import (
"log/slog"
"os"
"strconv"
"strings"
"time"
"go.uber.org/fx"
@@ -30,24 +31,12 @@ const (
// defaultRetentionSweepInterval is how often the retention
// reaper deletes events older than each webhook's RetentionDays.
defaultRetentionSweepInterval = time.Hour
// maxPort is the highest valid TCP port number. The lower
// bound (at least 1) is enforced by envPositiveInt.
maxPort = 65535
)
// ErrInvalidEnvironment is returned when WEBHOOKER_ENVIRONMENT
// contains an unrecognised value.
var ErrInvalidEnvironment = errors.New("invalid environment")
// ErrNonPositiveValue is returned when an environment variable that
// requires a positive integer is set to zero or a negative number.
var ErrNonPositiveValue = errors.New("value must be positive")
// ErrInvalidPort is returned when an environment variable holding a
// TCP port number is set above the valid port range.
var ErrInvalidPort = errors.New("invalid port")
//nolint:revive // ConfigParams is a standard fx naming convention.
type ConfigParams struct {
fx.In
@@ -92,81 +81,27 @@ func envString(key string) string {
}
// envBool returns the value of the named environment variable
// parsed as a boolean. Returns defaultValue if not set. If the
// variable is set but cannot be parsed, it returns a wrapped error
// naming the key and the bad value, so startup fails loudly rather
// than silently falling back to the default.
//
// Parsing is strconv.ParseBool, which accepts 1, t, T, TRUE, true,
// True, 0, f, F, FALSE, false and False. Anything else — "yes",
// "on", or a typo like "ture" — is an error rather than a silent
// false.
func envBool(key string, defaultValue bool) (bool, error) {
v := os.Getenv(key)
if v == "" {
return defaultValue, nil
// parsed as a boolean. Returns defaultValue if not set.
func envBool(key string, defaultValue bool) bool {
if v := os.Getenv(key); v != "" {
return strings.EqualFold(v, "true") || v == "1"
}
b, err := strconv.ParseBool(v)
if err != nil {
return false, fmt.Errorf(
"invalid boolean for %s: %q: %w", key, v, err,
)
}
return b, nil
return defaultValue
}
// envPositiveInt returns the value of the named environment variable
// parsed as a positive integer. Returns defaultValue if not set. If
// the variable is set but cannot be parsed, or parses to less than
// one, it returns a wrapped error naming the key and the bad value,
// so startup fails loudly rather than silently falling back to the
// default.
func envPositiveInt(
key string,
defaultValue int,
) (int, error) {
v := os.Getenv(key)
if v == "" {
return defaultValue, nil
// envInt returns the value of the named environment variable
// parsed as an integer. Returns defaultValue if not set or
// unparseable.
func envInt(key string, defaultValue int) int {
if v := os.Getenv(key); v != "" {
i, err := strconv.Atoi(v)
if err == nil {
return i
}
}
i, err := strconv.Atoi(v)
if err != nil {
return 0, fmt.Errorf(
"invalid integer for %s: %q: %w", key, v, err,
)
}
if i < 1 {
return 0, fmt.Errorf(
"%w: %s must be at least 1, got %q",
ErrNonPositiveValue, key, v,
)
}
return i, nil
}
// envPort returns the value of the named environment variable parsed
// as a TCP port number. Returns defaultValue if not set. A set value
// that is unparseable, below 1, or above maxPort is a hard error
// naming the key and the bad value.
func envPort(key string, defaultValue int) (int, error) {
port, err := envPositiveInt(key, defaultValue)
if err != nil {
return 0, err
}
if port > maxPort {
return 0, fmt.Errorf(
"%w: %s must be at most %d, got %d",
ErrInvalidPort, key, maxPort, port,
)
}
return port, nil
return defaultValue
}
// envDuration returns the value of the named environment variable
@@ -193,52 +128,32 @@ func envDuration(
return d, nil
}
// resolveEnvironment reads WEBHOOKER_ENVIRONMENT, defaulting to
// dev, and rejects unrecognised values.
func resolveEnvironment() (string, error) {
// New creates a Config by reading environment variables.
//
//nolint:revive // lc parameter is required by fx even if unused.
func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
log := params.Logger.Get()
// Determine environment from WEBHOOKER_ENVIRONMENT env var,
// default to dev
environment := os.Getenv("WEBHOOKER_ENVIRONMENT")
if environment == "" {
environment = EnvironmentDev
}
// Validate environment
if environment != EnvironmentDev &&
environment != EnvironmentProd {
return "", fmt.Errorf(
return nil, fmt.Errorf(
"%w: WEBHOOKER_ENVIRONMENT must be '%s' or '%s', got '%s'",
ErrInvalidEnvironment,
EnvironmentDev, EnvironmentProd, environment,
)
}
return environment, nil
}
// loadFromEnv builds a Config from the environment. Every value that
// needs parsing fails loudly when it is set but unparseable: the
// documented defaults apply only to variables that are unset (or
// empty), never as a substitute for a value the operator actually
// provided.
func loadFromEnv() (*Config, error) {
environment, err := resolveEnvironment()
if err != nil {
return nil, err
}
port, err := envPort("PORT", defaultPort)
if err != nil {
return nil, err
}
debug, err := envBool("DEBUG", false)
if err != nil {
return nil, err
}
maintenanceMode, err := envBool("MAINTENANCE_MODE", false)
if err != nil {
return nil, err
}
// Parse the retention sweep interval; a set-but-unparseable value
// is a hard error so fx aborts startup rather than silently using
// the default.
retentionSweepInterval, err := envDuration(
"RETENTION_SWEEP_INTERVAL",
defaultRetentionSweepInterval,
@@ -247,36 +162,21 @@ func loadFromEnv() (*Config, error) {
return nil, err
}
return &Config{
// Load configuration values from environment variables
s := &Config{
DataDir: envString("DATA_DIR"),
Debug: debug,
MaintenanceMode: maintenanceMode,
Debug: envBool("DEBUG", false),
MaintenanceMode: envBool("MAINTENANCE_MODE", false),
Environment: environment,
MetricsUsername: envString("METRICS_USERNAME"),
MetricsPassword: envString("METRICS_PASSWORD"),
Port: port,
Port: envInt("PORT", defaultPort),
SentryDSN: envString("SENTRY_DSN"),
RetentionSweepInterval: retentionSweepInterval,
}, nil
}
// New creates a Config by reading environment variables.
//
//nolint:revive // lc parameter is required by fx even if unused.
func New(lc fx.Lifecycle, params ConfigParams) (*Config, error) {
log := params.Logger.Get()
// A set-but-unparseable value anywhere in the environment is a
// hard error, so fx aborts startup rather than running with a
// silently substituted default.
s, err := loadFromEnv()
if err != nil {
return nil, err
log: log,
params: &params,
}
s.log = log
s.params = &params
// Set default DataDir. All SQLite databases (main application
// DB and per-webhook event DBs) live here. The same default is
// used regardless of environment; override with DATA_DIR if

View File

@@ -1,409 +0,0 @@
package config_test
import (
"os"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.uber.org/fx"
"sneak.berlin/go/webhooker/internal/config"
"sneak.berlin/go/webhooker/internal/globals"
"sneak.berlin/go/webhooker/internal/logger"
)
// testEnvKey is a throwaway variable name used only by the helper
// tables below, so they cannot disturb real configuration.
const testEnvKey = "WEBHOOKER_TEST_VALUE"
// Real configuration variables exercised by the config.New tests.
const (
envKeyPort = "PORT"
envKeyDebug = "DEBUG"
envKeyMaintenanceMode = "MAINTENANCE_MODE"
)
// envBoolCase is one row of the envBool table.
type envBoolCase struct {
name string
set bool
value string
defaultValue bool
expectError bool
expected bool
}
// envBoolCases is the envBool table, kept out of the test body so
// the test itself stays readable.
func envBoolCases() []envBoolCase {
return []envBoolCase{
{
name: "unset uses default false",
defaultValue: false,
expected: false,
},
{
name: "unset uses default true",
defaultValue: true,
expected: true,
},
{
name: "empty uses default true",
set: true,
value: "",
defaultValue: true,
expected: true,
},
{
name: "true is parsed",
set: true,
value: "true",
expected: true,
},
{
name: "one is parsed",
set: true,
value: "1",
expected: true,
},
{
name: "False is parsed",
set: true,
value: "False",
defaultValue: true,
expected: false,
},
{
name: "zero is parsed",
set: true,
value: "0",
defaultValue: true,
expected: false,
},
{
name: "yes is rejected",
set: true,
value: "yes",
expectError: true,
},
{
name: "on is rejected",
set: true,
value: "on",
expectError: true,
},
{
name: "typo is rejected",
set: true,
value: "ture",
expectError: true,
},
}
}
func TestEnvBool(t *testing.T) {
for _, tt := range envBoolCases() {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvBoolForTest(
testEnvKey, tt.defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
assert.Contains(t, err.Error(), tt.value)
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
func TestEnvPositiveInt(t *testing.T) {
const defaultValue = 7
tests := []struct {
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{
name: "unset returns the default integer",
expected: defaultValue,
},
{
name: "empty returns the default integer",
set: true,
value: "",
expected: defaultValue,
},
{
name: "positive value is parsed",
set: true,
value: "42",
expected: 42,
},
{
name: "unparseable value is rejected",
set: true,
value: "not-a-number",
expectError: true,
},
{
name: "zero is rejected",
set: true,
value: "0",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
{
name: "negative is rejected",
set: true,
value: "-5",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvPositiveIntForTest(
testEnvKey, defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
assert.Contains(t, err.Error(), tt.value)
if tt.errIs != nil {
require.ErrorIs(t, err, tt.errIs)
}
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
func TestEnvPort(t *testing.T) {
const defaultValue = 8080
tests := []struct {
name string
set bool
value string
expectError bool
errIs error
expected int
}{
{
name: "unset returns the default port",
expected: defaultValue,
},
{
name: "valid port is parsed",
set: true,
value: "9000",
expected: 9000,
},
{
name: "highest port is accepted",
set: true,
value: "65535",
expected: 65535,
},
{
name: "unparseable value is rejected",
set: true,
value: "not-a-port",
expectError: true,
},
{
name: "zero is rejected",
set: true,
value: "0",
expectError: true,
errIs: config.ErrNonPositiveValue,
},
{
name: "above the port range is rejected",
set: true,
value: "65536",
expectError: true,
errIs: config.ErrInvalidPort,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
if tt.set {
t.Setenv(testEnvKey, tt.value)
} else {
require.NoError(t, os.Unsetenv(testEnvKey))
}
got, err := config.EnvPortForTest(
testEnvKey, defaultValue,
)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), testEnvKey)
if tt.errIs != nil {
require.ErrorIs(t, err, tt.errIs)
}
return
}
require.NoError(t, err)
assert.Equal(t, tt.expected, got)
})
}
}
// buildConfig constructs a Config through fx exactly as the
// application does, returning the config and any construction error.
func buildConfig(t *testing.T) (*config.Config, error) {
t.Helper()
var cfg *config.Config
app := fx.New(
fx.NopLogger,
fx.Provide(
globals.New,
logger.New,
config.New,
),
fx.Populate(&cfg),
)
return cfg, app.Err()
}
func TestNewRejectsBadEnvValues(t *testing.T) {
tests := []struct {
name string
key string
value string
expectError bool
check func(t *testing.T, cfg *config.Config)
}{
{
name: "valid PORT is used",
key: envKeyPort,
value: "9001",
check: func(t *testing.T, cfg *config.Config) {
t.Helper()
assert.Equal(t, 9001, cfg.Port)
},
},
{
name: "unparseable PORT aborts startup",
key: envKeyPort,
value: "eighty-eighty",
expectError: true,
},
{
name: "out-of-range PORT aborts startup",
key: envKeyPort,
value: "70000",
expectError: true,
},
{
name: "valid DEBUG is used",
key: envKeyDebug,
value: "true",
check: func(t *testing.T, cfg *config.Config) {
t.Helper()
assert.True(t, cfg.Debug)
},
},
{
name: "unparseable DEBUG aborts startup",
key: envKeyDebug,
value: "ture",
expectError: true,
},
{
name: "unparseable MAINTENANCE_MODE aborts startup",
key: envKeyMaintenanceMode,
value: "sometimes",
expectError: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Cannot use t.Parallel() here because t.Setenv
// is incompatible with parallel subtests.
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
t.Setenv(tt.key, tt.value)
cfg, err := buildConfig(t)
if tt.expectError {
require.Error(t, err)
assert.Contains(t, err.Error(), tt.key)
assert.Contains(t, err.Error(), tt.value)
return
}
require.NoError(t, err)
require.NotNil(t, cfg)
tt.check(t, cfg)
})
}
}
// TestNewUsesDefaultsWhenUnset proves the fail-loud behaviour did not
// break the legitimate unset case: absent variables still get their
// documented defaults.
func TestNewUsesDefaultsWhenUnset(t *testing.T) {
t.Setenv("WEBHOOKER_ENVIRONMENT", "dev")
for _, key := range []string{
envKeyPort, envKeyDebug, envKeyMaintenanceMode,
} {
require.NoError(t, os.Unsetenv(key))
}
cfg, err := buildConfig(t)
require.NoError(t, err)
require.NotNil(t, cfg)
assert.Equal(t, 8080, cfg.Port)
assert.False(t, cfg.Debug)
assert.False(t, cfg.MaintenanceMode)
}

View File

@@ -1,20 +0,0 @@
package config
// This file exposes the unexported environment parsing helpers to
// the external config_test package so each helper can be covered by
// its own table-driven test without weakening the package API.
// EnvBoolForTest exposes envBool.
func EnvBoolForTest(key string, defaultValue bool) (bool, error) {
return envBool(key, defaultValue)
}
// EnvPositiveIntForTest exposes envPositiveInt.
func EnvPositiveIntForTest(key string, defaultValue int) (int, error) {
return envPositiveInt(key, defaultValue)
}
// EnvPortForTest exposes envPort.
func EnvPortForTest(key string, defaultValue int) (int, error) {
return envPort(key, defaultValue)
}

View File

@@ -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(

View File

@@ -748,6 +748,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.

View File

@@ -188,6 +188,13 @@ func (e *Engine) ExportRecoverInFlight(
e.recoverInFlight(ctx)
}
// ExportSweepWebhookRetries exposes sweepWebhookRetries.
func (e *Engine) ExportSweepWebhookRetries(
ctx context.Context, webhookID string,
) {
e.sweepWebhookRetries(ctx, webhookID)
}
// ExportStart exposes start for testing.
func (e *Engine) ExportStart(ctx context.Context) {
e.start(ctx)