Files
webhooker/internal/delivery/event_db_isolation_test.go
sneak 3f429f9a4f
All checks were successful
check / check (push) Successful in 3m11s
Close the two remaining delivery terminal-state gaps (closes #107)
A delivery could reach a bad end without the engine recording why, and
a retrying delivery could fail to reach an end at all.

An unknown target type marked the delivery failed and wrote no
DeliveryResult, so the event log showed "failed" with no attempts and
the only account of why was one line in the server log. It now records
a result naming the type before failing the delivery.

A deleted target left its retrying deliveries stranded. Both recovery
and the sweep began with a scoped loadTarget, which cannot see a soft
deleted row, so both logged and returned: the delivery stayed retrying
for the life of the database while the sweep repeated the same error
every minute. Both now terminalise it with a recorded reason.

Deleting a target also did not stop deliveries to it. A scheduled
retry is a time.AfterFunc holding the target's configuration from when
the chain began, and nothing on that path read the target row, so the
timer kept firing and kept sending to the destination the operator had
removed for the rest of the backoff chain; terminalising in recovery
and the sweep alone would only have caught it after a restart.
processRetryTask now confirms the target still exists before it
attempts, and abandons the chain when it does not.

Only a target confirmed gone stops anything. A lookup that fails for
any other reason is the main database being unreadable, which is
transient, and every path leaves the delivery exactly as it was rather
than failing it.

The reason text comes from one Unscoped lookup confined to these
terminal paths, because a soft deleted row is what distinguishes a
target the operator deleted from an id that never named one. The
engine's normal target loading stays scoped, or deleting a target
would stop nothing.

Terminal writes reached from recovery keep going through the existing
retainIdle ownership gate; the retry path writes directly, as a
target's own Deliver does, because the worker already holds that
delivery. Nothing was added to either sweep dispatch arm.

Retry fixtures that drove processRetryTask for a target id with no row
in the main database now create one. That state is not reachable in
service: the handler reads the target to build the task.
2026-08-24 02:33:49 +00:00

166 lines
3.8 KiB
Go

package delivery_test
import (
"context"
"database/sql"
"fmt"
"net/http"
"net/http/httptest"
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
_ "modernc.org/sqlite"
"sneak.berlin/go/webhooker/internal/database"
)
// assertNoTargetRows opens the per-webhook database file directly,
// outside GORM, and fails if its targets table holds any rows.
// Target config is the credential for slack and http targets, and
// event databases are the files that get backed up and handed
// around.
func assertNoTargetRows(t *testing.T, dbPath string) {
t.Helper()
sqlDB, err := sql.Open(
"sqlite", fmt.Sprintf("file:%s?mode=ro", dbPath),
)
require.NoError(t, err)
defer func() { _ = sqlDB.Close() }()
var tables int
require.NoError(t, sqlDB.QueryRowContext(
t.Context(),
"SELECT count(*) FROM sqlite_master "+
"WHERE type = 'table' AND name = 'targets'",
).Scan(&tables))
if tables == 0 {
return
}
var rows int
require.NoError(t, sqlDB.QueryRowContext(
t.Context(), "SELECT count(*) FROM targets",
).Scan(&rows))
assert.Zero(
t, rows,
"per-webhook event database must hold no target rows",
)
}
// TestEventDBHoldsNoTargetRows drives a delivery and then a retry
// through the real engine write paths and asserts neither leaves a
// target row behind in events-*.db.
func TestEventDBHoldsNoTargetRows(t *testing.T) {
t.Parallel()
s := newISetup(t)
ts := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
},
))
defer ts.Close()
cfg := iHTTPConfig(ts.URL)
targetID := uuid.New().String()
dbPath := s.DBMgr.DBPath(s.WebhookID)
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`,
)
body := event.Body
// A new delivery.
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
task := iTask(
d, event, s.WebhookID, targetID,
"leaky-target", cfg, 5, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusDelivered,
)
assertNoTargetRows(t, dbPath)
// A retry. Its target exists in the main database, because the
// engine confirms a scheduled retry's target has not been
// deleted before running it; see
// https://git.eeqj.de/sneak/webhooker/issues/107.
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "leaky-target",
database.TargetTypeHTTP, cfg, 5,
)
rd := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusRetrying,
)
rTask := iTask(
rd, event, s.WebhookID, targetID,
"leaky-target", cfg, 5, 2, &body,
)
s.Engine.ExportProcessRetryTask(context.TODO(), &rTask)
iAssertStatus(
t, s.WebhookDB, rd.ID,
database.DeliveryStatusDelivered,
)
assertNoTargetRows(t, dbPath)
}
// TestEventDBHoldsNoTargetRowsOnFailedDelivery covers the failure
// write path, which updates the delivery to failed and records a
// result, rather than the success path above.
func TestEventDBHoldsNoTargetRowsOnFailedDelivery(t *testing.T) {
t.Parallel()
s := newISetup(t)
ts := httptest.NewServer(http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusInternalServerError)
},
))
defer ts.Close()
cfg := iHTTPConfig(ts.URL)
targetID := uuid.New().String()
event := iSeedEvent(
t, s.WebhookDB, s.WebhookID, `{"leak":"none"}`,
)
body := event.Body
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
)
task := iTask(
d, event, s.WebhookID, targetID,
"leaky-target", cfg, 0, 1, &body,
)
s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(
t, s.WebhookDB, d.ID,
database.DeliveryStatusFailed,
)
assertNoTargetRows(t, s.DBMgr.DBPath(s.WebhookID))
}