Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
468db3c814 | ||
|
|
573f7f986a | ||
|
|
373a2f828d | ||
|
|
267d5b12f2 |
+15
-9
@@ -214,16 +214,22 @@ RUN version="$(make version VERSION="$VERSION")"; \
|
||||
# (Alpine.js, extracted from its tarball in 3p/) first.
|
||||
RUN make build VERSION="$VERSION"
|
||||
|
||||
# Rebuild with static linking for Alpine runtime.
|
||||
# make build already verified compilation.
|
||||
# The CGO binary from `make build` is dynamically linked against glibc,
|
||||
# which doesn't exist on Alpine (musl). Rebuild with static linking so
|
||||
# the binary runs on Alpine without glibc.
|
||||
# Rebuild with static linking for the Alpine runtime. The cgo binary the
|
||||
# step above builds is dynamically linked against glibc, which Alpine
|
||||
# (musl) does not have.
|
||||
#
|
||||
# The static flags go in through GO_LDFLAGS rather than a -ldflags of
|
||||
# their own: the build target composes them with the -X that stamps the
|
||||
# version, so this relink cannot silently drop the stamp.
|
||||
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'
|
||||
# A statically linked glibc still loads shared libraries at run time for
|
||||
# getaddrinfo and dlopen, and Alpine has none to load, so the build tags
|
||||
# keep both out of the binary: netgo and osusergo select Go's own DNS
|
||||
# resolver and user lookup instead of glibc's, and
|
||||
# sqlite_omit_load_extension leaves out SQLite extension loading, which
|
||||
# webhooker never uses.
|
||||
#
|
||||
# The static flags and tags go in through GO_LDFLAGS and GO_TAGS rather
|
||||
# than a -ldflags of their own: the build target composes them with the
|
||||
# -X that stamps the version, so this relink cannot silently drop the
|
||||
# stamp.
|
||||
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_TAGS=netgo,osusergo,sqlite_omit_load_extension GO_LDFLAGS='-extldflags "-static"'
|
||||
|
||||
# Runtime stage
|
||||
# alpine:3.21, 2026-03-17
|
||||
|
||||
@@ -16,14 +16,16 @@ VERSION ?= $(shell script/version)
|
||||
# exists to correct.
|
||||
override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
|
||||
|
||||
# Extra linker flags for the build target. The static relink in the
|
||||
# Dockerfile adds -extldflags here rather than passing its own -ldflags,
|
||||
# so composing flags cannot drop the version stamp.
|
||||
# Extra linker flags and build tags for the build target. The static
|
||||
# relink in the Dockerfile passes -extldflags and its build tags through
|
||||
# these rather than a -ldflags of its own, so composing flags cannot drop
|
||||
# the version stamp.
|
||||
#
|
||||
# The build target itself always passes -trimpath and -s -w, as the Go
|
||||
# Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or
|
||||
# debug information in the binary.
|
||||
GO_LDFLAGS ?=
|
||||
GO_TAGS ?=
|
||||
|
||||
bootstrap:
|
||||
@script/bootstrap
|
||||
@@ -53,7 +55,7 @@ check:
|
||||
@script/check
|
||||
|
||||
build: assets
|
||||
go build -trimpath -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
|
||||
go build -trimpath -tags '$(GO_TAGS)' -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
|
||||
|
||||
run: build
|
||||
./bin/webhooker
|
||||
|
||||
@@ -1325,9 +1325,11 @@ webhooker solves this by acting as a durable intermediary:
|
||||
deliberately left in a recoverable state rather than marked done — losing a
|
||||
delivery is the worse failure — so the pending sweep picks it up about
|
||||
fifteen minutes later, or the next restart does, and the target receives a
|
||||
payload it already got. webhooker adds no delivery identifier of its own to
|
||||
an outbound request, so **make your receiver idempotent** against whatever
|
||||
the payload itself carries.
|
||||
payload it already got. A send still in flight when the service stops is
|
||||
left the same way: its attempt is not recorded, and the next start sends it
|
||||
again (see Shutdown). webhooker adds no delivery identifier of its own to an
|
||||
outbound request, so **make your receiver idempotent** against whatever the
|
||||
payload itself carries.
|
||||
|
||||
3. **Observability** — Full request/response logging for every webhook received
|
||||
and every delivery attempted. Prometheus metrics expose volume, latency, and
|
||||
@@ -1768,7 +1770,7 @@ is what bounds it.
|
||||
#### DeliveryResult
|
||||
|
||||
The result of a single delivery attempt. Every attempt (including retries) is
|
||||
individually logged for full observability.
|
||||
individually logged, except one cut off by a stop (see Shutdown).
|
||||
|
||||
| Field | Type | Description |
|
||||
| --------------- | ------- | ----------------------------------------- |
|
||||
@@ -2206,12 +2208,14 @@ This means:
|
||||
buffer rather than spawning more goroutines.
|
||||
- **Independent results** — each worker records its own delivery result in the
|
||||
per-webhook database without coordination.
|
||||
- **Graceful shutdown** — cancel the context, workers finish their current task
|
||||
and exit. The stop hook waits for the pool via `lifecycle.WaitForShutdown`,
|
||||
which bounds that wait by fx's stop timeout rather than blocking forever on a
|
||||
wedged worker. On timeout it logs at `ERROR` and returns an error, and the
|
||||
goroutines that did not finish are still running — an unclean shutdown is
|
||||
reported rather than hidden.
|
||||
- **Graceful shutdown** — the stop hook cancels the workers' context, and each
|
||||
worker exits once its current task returns. Cancelling cuts off an `http` or
|
||||
`slack` request still in flight; that attempt is not recorded, and the next
|
||||
start sends the delivery again (see Shutdown). The stop hook waits for the
|
||||
pool via `lifecycle.WaitForShutdown`, which bounds that wait by fx's stop
|
||||
timeout rather than blocking forever on a wedged worker. On timeout it logs at
|
||||
`ERROR` and returns an error, and the goroutines that did not finish are still
|
||||
running — an unclean shutdown is reported rather than hidden.
|
||||
|
||||
**Recovery paths:**
|
||||
|
||||
@@ -3284,7 +3288,8 @@ stop-hook log:
|
||||
3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
|
||||
seconds**) and by what the hooks before it left, then a Sentry flush if
|
||||
`SENTRY_DSN` is set
|
||||
4. `delivery.Engine` — waits for its workers, then closes the archive databases
|
||||
4. `delivery.Engine` — cuts off any `http` or `slack` request in flight, waits
|
||||
for its workers, then closes the archive databases
|
||||
5. `healthcheck`
|
||||
6. `WebhookDBManager`
|
||||
7. the database close
|
||||
@@ -3295,6 +3300,12 @@ by the stop context, so a wedge there consumes the 5 seconds before the HTTP
|
||||
server hook is ever entered. The hooks after the server are microsecond-scale in
|
||||
normal operation.
|
||||
|
||||
The delivery engine does not wait for a delivery request in flight: it cancels
|
||||
it. That attempt is not recorded and the delivery keeps its `pending` or
|
||||
`retrying` status, so startup recovery sends it again on the next start (see
|
||||
Recovery paths under Bounded Worker Pool). If the receiver already had it, it
|
||||
receives it twice: the at-least-once duplicate described under Rationale.
|
||||
|
||||
The HTTP drain budget is deliberately **shorter** than the sequence budget. Were
|
||||
the two equal, a drain that used its whole budget would exhaust the sequence
|
||||
budget at the instant it finished, and every later hook — the delivery engine,
|
||||
@@ -3419,12 +3430,14 @@ fixed independently of the compiler's:
|
||||
phases and the `css-check` and `markdown-check` stages passing (it copies a
|
||||
file from each), runs `make build` (which extracts Alpine.js from `3p/`
|
||||
first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking
|
||||
so it runs on musl. Both builds go through `make build`, the relink adding
|
||||
its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that stamps
|
||||
the version. The version is the `VERSION` build arg if one is given,
|
||||
otherwise derived from the `.git` in the context, and the stage fails if a
|
||||
context with `.git` would stamp an empty version, `dev` or `unknown` (see
|
||||
[Version stamping](#version-stamping)).
|
||||
so it runs on musl, with the build tags `netgo`, `osusergo` and
|
||||
`sqlite_omit_load_extension` so it needs none of glibc's shared libraries at
|
||||
run time. Both builds go through `make build`, the relink adding its
|
||||
`-extldflags` via `GO_LDFLAGS` and its tags via `GO_TAGS`, so neither can
|
||||
drop the `-X` that stamps the version. The version is the `VERSION` build arg
|
||||
if one is given, otherwise derived from the `.git` in the context, and the
|
||||
stage fails if a context with `.git` would stamp an empty version, `dev` or
|
||||
`unknown` (see [Version stamping](#version-stamping)).
|
||||
7. **Runtime stage** (`alpine:3.21`) — copies the static binary and
|
||||
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
|
||||
all SQLite databases, exposes port 8080, and includes a health check against
|
||||
|
||||
@@ -45,8 +45,7 @@ type Event struct {
|
||||
|
||||
// ResubmittedFromID names the event this one was copied from by
|
||||
// an operator resubmit. It is nil for an event that arrived on
|
||||
// the receiver, which is every event created before the column
|
||||
// existed. It is not a foreign key: the source event can be
|
||||
// the receiver. It is not a foreign key: the source event can be
|
||||
// reaped by retention while its copies remain, and the id is
|
||||
// kept as the record of where the copy came from either way.
|
||||
ResubmittedFromID *string `gorm:"type:uuid;index:idx_events_resubmitted_from_id,priority:1;index:idx_events_entrypoint_id,priority:3" json:"resubmittedFromId,omitempty"`
|
||||
|
||||
@@ -82,7 +82,7 @@ type Webhook struct {
|
||||
// value while building the insert statement, which runs after
|
||||
// BeforeSave; rewriting any later than this loses that race and the
|
||||
// row lands at 30 days. Living on the model also means a future call
|
||||
// site — a REST API, a fixture, a migration — cannot bypass it.
|
||||
// site, such as a REST API or a fixture, cannot bypass it.
|
||||
func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
||||
if w.RetentionDays <= 0 {
|
||||
w.RetentionDays = RetentionForeverDays
|
||||
@@ -98,8 +98,8 @@ func (w *Webhook) BeforeSave(_ *gorm.DB) error {
|
||||
// are exempt from reaping.
|
||||
//
|
||||
// It accepts the RetentionForeverDays sentinel written by BeforeSave
|
||||
// and, defensively, the non-positive values that rows written before
|
||||
// the sentinel existed may still carry.
|
||||
// and, defensively, any non-positive value, which BeforeSave treats
|
||||
// as a request to retain forever.
|
||||
func retainsForever(retentionDays int) bool {
|
||||
return retentionDays <= 0 ||
|
||||
retentionDays >= RetentionForeverDays
|
||||
|
||||
@@ -253,7 +253,7 @@ func (r *RetentionReaper) reapWebhook(
|
||||
// row in the database: the sweep then deletes every event, delivery,
|
||||
// and delivery result, including ones created seconds ago. Rejecting
|
||||
// out-of-range input at the form is the primary guard; saturating here
|
||||
// means an old row, a migration, or a future call site cannot turn a
|
||||
// means a value that reaches the row by any other path cannot turn a
|
||||
// too-large retention into total data loss.
|
||||
func retentionCutoff(
|
||||
now time.Time,
|
||||
|
||||
@@ -118,9 +118,8 @@ const SQLiteFilePerm fs.FileMode = 0o600
|
||||
// SQLite still reports the absence rather than this function
|
||||
// materializing an empty database the caller asked not to create.
|
||||
//
|
||||
// Chmod of a file that already exists is what tightens a data
|
||||
// directory an earlier build left at 0644 — including a developer's
|
||||
// own scratch directory — without any migration machinery.
|
||||
// The mode is set on every open, whoever created the file, so a
|
||||
// database or sidecar found at a wider mode is tightened too.
|
||||
func reserveSQLiteFile(path string, create bool) error {
|
||||
if create {
|
||||
// gosec G304: the path is the database file the caller asked
|
||||
@@ -242,7 +241,19 @@ func SQLiteDSN(path, mode string) string {
|
||||
)
|
||||
q.Add("_pragma", "journal_mode(WAL)")
|
||||
|
||||
return "file:" + path + "?" + q.Encode()
|
||||
// SQLite reads a file: URI's path only up to the first ? or #, and
|
||||
// decodes %XX escapes in it, so url.URL escapes the path. OmitHost
|
||||
// keeps a relative path as "file:data/x.db"; written
|
||||
// "file://data/x.db", SQLite would reject "data" as the URI's
|
||||
// authority.
|
||||
dsn := url.URL{
|
||||
Scheme: "file",
|
||||
Path: path,
|
||||
OmitHost: true,
|
||||
RawQuery: q.Encode(),
|
||||
}
|
||||
|
||||
return dsn.String()
|
||||
}
|
||||
|
||||
// OpenSQLite opens the SQLite file at path with the service's
|
||||
|
||||
@@ -2,6 +2,8 @@ package database_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"database/sql"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -112,6 +114,79 @@ func TestMainDBAppliesPragmasOnALiveHandle(t *testing.T) {
|
||||
assert.Equal(t, "10000", busy)
|
||||
}
|
||||
|
||||
// TestOpenSQLiteOpensThePathAskedFor uses a data directory whose name
|
||||
// holds ?, # and %. SQLite reads a file: URI's path only up to a ? or
|
||||
// #, and decodes %41 to A, so an unescaped path sent every database in
|
||||
// that directory to one file outside it.
|
||||
func TestOpenSQLiteOpensThePathAskedFor(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
dir := filepath.Join(t.TempDir(), "hooks#1?x%41")
|
||||
require.NoError(t, os.Mkdir(dir, 0o700))
|
||||
|
||||
for _, name := range []string{
|
||||
database.MainDBFileName,
|
||||
"events-" + uuid.New().String() + ".db",
|
||||
} {
|
||||
path := filepath.Join(dir, name)
|
||||
|
||||
sqlDB, err := database.OpenSQLite(
|
||||
path, database.SQLiteModeCreate,
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
t.Cleanup(func() { assert.NoError(t, sqlDB.Close()) })
|
||||
|
||||
assertOpenedFile(t, sqlDB, path)
|
||||
}
|
||||
}
|
||||
|
||||
// TestOpenSQLiteOpensARelativePath opens a database at a path relative
|
||||
// to the working directory, which is what a relative DATA_DIR such as
|
||||
// the README's DATA_DIR=../webhooker-data gives OpenSQLite.
|
||||
//
|
||||
//nolint:paralleltest // t.Chdir moves the whole process.
|
||||
func TestOpenSQLiteOpensARelativePath(t *testing.T) {
|
||||
t.Chdir(t.TempDir())
|
||||
require.NoError(t, os.Mkdir("data", 0o700))
|
||||
|
||||
path := filepath.Join("data", database.MainDBFileName)
|
||||
|
||||
sqlDB, err := database.OpenSQLite(path, database.SQLiteModeCreate)
|
||||
require.NoError(t, err)
|
||||
|
||||
defer func() { require.NoError(t, sqlDB.Close()) }()
|
||||
|
||||
assertOpenedFile(t, sqlDB, path)
|
||||
}
|
||||
|
||||
// assertOpenedFile checks that sqlDB is the SQLite file at path.
|
||||
// database_list names the file SQLite actually opened, as an absolute
|
||||
// path with symlinks resolved, so the files are compared, not the names.
|
||||
func assertOpenedFile(t *testing.T, sqlDB *sql.DB, path string) {
|
||||
t.Helper()
|
||||
|
||||
var (
|
||||
seq int
|
||||
schema, file string
|
||||
)
|
||||
|
||||
require.NoError(t, sqlDB.
|
||||
QueryRowContext(t.Context(), "pragma database_list").
|
||||
Scan(&seq, &schema, &file))
|
||||
|
||||
asked, err := os.Stat(path)
|
||||
require.NoError(t, err)
|
||||
|
||||
opened, err := os.Stat(file)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.True(
|
||||
t, os.SameFile(asked, opened),
|
||||
"asked for %s, SQLite opened %s", path, file,
|
||||
)
|
||||
}
|
||||
|
||||
// TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the
|
||||
// reproduction in
|
||||
// https://git.eeqj.de/sneak/webhooker/issues/256: an operator's
|
||||
|
||||
@@ -201,8 +201,7 @@ func (s *ArchiveSweeper) sweep(ctx context.Context) {
|
||||
|
||||
// sweepTarget prunes the archive of a single database target.
|
||||
// A missing, empty, or "never" expiry parses as a zero duration
|
||||
// and is skipped entirely, so those archives keep exactly the
|
||||
// behaviour they had before the sweep existed.
|
||||
// and is skipped entirely, so those archives are never pruned.
|
||||
func (s *ArchiveSweeper) sweepTarget(target *database.Target) {
|
||||
expiry, err := parseArchiveExpiry(target.Config)
|
||||
if err != nil {
|
||||
|
||||
@@ -2,11 +2,16 @@ package delivery_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"path/filepath"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"go.uber.org/fx"
|
||||
"gorm.io/gorm/clause"
|
||||
@@ -272,6 +277,151 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
||||
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
|
||||
}
|
||||
|
||||
// seedHeldDelivery gives the setup's webhook an http target with
|
||||
// maxRetries, pointed at a receiver that keeps every request open until
|
||||
// the client gives up on it, and seeds one delivery to that target at
|
||||
// status. It returns the task that sends attempt attemptNum of the
|
||||
// delivery, and a channel that is closed when a request reaches the
|
||||
// receiver.
|
||||
func seedHeldDelivery(
|
||||
t *testing.T,
|
||||
s iSetup,
|
||||
maxRetries int,
|
||||
status database.DeliveryStatus,
|
||||
attemptNum int,
|
||||
) (delivery.Task, <-chan struct{}) {
|
||||
t.Helper()
|
||||
|
||||
arrived := make(chan struct{})
|
||||
|
||||
var once sync.Once
|
||||
|
||||
ts := httptest.NewServer(http.HandlerFunc(
|
||||
func(_ http.ResponseWriter, r *http.Request) {
|
||||
// The server notices the client hanging up only once the
|
||||
// body has been read; until then r.Context() never ends.
|
||||
_, _ = io.Copy(io.Discard, r.Body)
|
||||
|
||||
once.Do(func() { close(arrived) })
|
||||
|
||||
<-r.Context().Done()
|
||||
},
|
||||
))
|
||||
t.Cleanup(ts.Close)
|
||||
|
||||
cfg := iHTTPConfig(ts.URL)
|
||||
targetID := uuid.New().String()
|
||||
|
||||
iCreateWebhook(t, s.MainDB, s.WebhookID, "hook")
|
||||
iCreateTarget(
|
||||
t, s.MainDB, targetID, s.WebhookID, "held",
|
||||
database.TargetTypeHTTP, cfg, maxRetries,
|
||||
)
|
||||
|
||||
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"held":true}`)
|
||||
d := iSeedDelivery(t, s.WebhookDB, event.ID, targetID, status)
|
||||
|
||||
bodyStr := event.Body
|
||||
|
||||
return iTask(
|
||||
d, event, s.WebhookID, targetID, "held", cfg,
|
||||
maxRetries, attemptNum, &bodyStr,
|
||||
), arrived
|
||||
}
|
||||
|
||||
// stopDuringAttempt waits until the receiver holds an attempt open, then
|
||||
// stops the engine through its real stop hook.
|
||||
func stopDuringAttempt(
|
||||
t *testing.T, lc *recordingLifecycle, arrived <-chan struct{},
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
select {
|
||||
case <-arrived:
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("the delivery attempt never reached the receiver")
|
||||
}
|
||||
|
||||
require.NoError(t, lc.hooks[0].OnStop(context.Background()))
|
||||
}
|
||||
|
||||
// requireRequeued runs restart recovery on the stopped engine and
|
||||
// requires it to queue task's attempt again on ch.
|
||||
func requireRequeued(
|
||||
t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task,
|
||||
) {
|
||||
t.Helper()
|
||||
|
||||
s.Engine.ExportRecoverWebhookDeliveries(
|
||||
context.Background(), s.WebhookID,
|
||||
)
|
||||
|
||||
select {
|
||||
case got := <-ch:
|
||||
assert.Equal(t, task.DeliveryID, got.DeliveryID)
|
||||
assert.Equal(t, task.AttemptNum, got.AttemptNum)
|
||||
case <-time.After(5 * time.Second):
|
||||
t.Fatal("restart recovery did not queue the delivery again")
|
||||
}
|
||||
}
|
||||
|
||||
// TestEngine_StopLeavesFireAndForgetAttemptPending is the regression
|
||||
// test for a stop that turned an attempt in flight into a terminal
|
||||
// failure. Stopping the engine cancels the request; for a target with
|
||||
// no retries that was the delivery's only attempt, recorded as failed
|
||||
// with "context canceled", and nothing ever sent it again. The
|
||||
// delivery must instead stay pending, with no result, for restart
|
||||
// recovery to send.
|
||||
func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
s := newISetup(t)
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, 0, database.DeliveryStatusPending, 1,
|
||||
)
|
||||
|
||||
s.Engine.Notify([]delivery.Task{task})
|
||||
stopDuringAttempt(t, lc, arrived)
|
||||
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusPending,
|
||||
)
|
||||
assert.Empty(t, iResults(t, s.WebhookDB, task.DeliveryID))
|
||||
|
||||
requireRequeued(t, s, s.Engine.ExportDeliveryCh(), task)
|
||||
}
|
||||
|
||||
// TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression
|
||||
// for a target with retries, on its last attempt: the cancelled attempt
|
||||
// used to exhaust the retries and end the delivery failed. It must stay
|
||||
// retrying with only its earlier attempt recorded, and restart recovery
|
||||
// must send that last attempt again.
|
||||
func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const maxRetries = 2
|
||||
|
||||
s := newISetup(t)
|
||||
lc := startEngineViaHook(t, s.Engine)
|
||||
|
||||
task, arrived := seedHeldDelivery(
|
||||
t, s, maxRetries, database.DeliveryStatusRetrying, maxRetries,
|
||||
)
|
||||
iSeedFailedResult(t, s.WebhookDB, task.DeliveryID)
|
||||
|
||||
s.Engine.ExportScheduleRetry(task, 0)
|
||||
stopDuringAttempt(t, lc, arrived)
|
||||
|
||||
iAssertStatus(
|
||||
t, s.WebhookDB, task.DeliveryID, database.DeliveryStatusRetrying,
|
||||
)
|
||||
assert.Len(t, iResults(t, s.WebhookDB, task.DeliveryID), 1)
|
||||
|
||||
requireRequeued(t, s, s.Engine.ExportRetryCh(), task)
|
||||
}
|
||||
|
||||
// deliverToArchive gives the setup's webhook a database target,
|
||||
// runs one delivery to it through the running engine, and returns
|
||||
// the target's ID and archive file path. The archive writer holds
|
||||
|
||||
@@ -82,8 +82,8 @@ func isReservedTargetHeader(name string) bool {
|
||||
// one pair silently overwriting the other.
|
||||
//
|
||||
// An input with no pairs yields an empty map, which omitempty drops
|
||||
// from the stored config: a target configured with no headers keeps
|
||||
// the same config JSON it had before this field existed.
|
||||
// from the stored config, so a target configured with no headers
|
||||
// stores no headers key at all.
|
||||
func ParseTargetHeaders(raw string) (map[string]string, error) {
|
||||
headers := make(map[string]string)
|
||||
|
||||
|
||||
@@ -57,7 +57,11 @@ type httpCore struct {
|
||||
// recorded and no circuit breaker is consulted. A positive
|
||||
// maxRetries gates the attempt on the circuit breaker and
|
||||
// schedules a backed-off retry on failure.
|
||||
//
|
||||
// ctx is the context attempt sends its request with. Stopping the
|
||||
// engine cancels it; see interruptedByStop.
|
||||
func (c *httpCore) deliver(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
@@ -66,23 +70,57 @@ func (c *httpCore) deliver(
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
if maxRetries == 0 {
|
||||
c.fireAndForget(webhookDB, d, attempt())
|
||||
c.fireAndForget(ctx, webhookDB, d, attempt())
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.withRetry(
|
||||
webhookDB, d, task, sched, maxRetries, attempt,
|
||||
ctx, webhookDB, d, task, sched, maxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
// interruptedByStop reports whether an attempt failed because the
|
||||
// engine is stopping, which cancels the context its request was sent
|
||||
// with. Such an attempt is not recorded: no DeliveryResult, no circuit
|
||||
// breaker outcome, no status change. The delivery stays pending or
|
||||
// retrying, and startup recovery sends it again on the next start.
|
||||
// The request did go out, so the attempt metrics still count it.
|
||||
//
|
||||
// A target's own timeout leaves that context alone, so its attempt is
|
||||
// still recorded as the failure it is.
|
||||
func (c *httpCore) interruptedByStop(
|
||||
ctx context.Context,
|
||||
d *database.Delivery,
|
||||
res attemptResult,
|
||||
) bool {
|
||||
if res.success || ctx.Err() == nil {
|
||||
return false
|
||||
}
|
||||
|
||||
c.eng.log.Warn(
|
||||
"delivery attempt interrupted by shutdown; not recorded, "+
|
||||
"it is sent again on the next start",
|
||||
"delivery_id", d.ID,
|
||||
"event_id", d.EventID,
|
||||
"target_id", d.TargetID,
|
||||
)
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func (c *httpCore) fireAndForget(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
res attemptResult,
|
||||
) {
|
||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||
|
||||
if c.interruptedByStop(ctx, d, res) {
|
||||
return
|
||||
}
|
||||
|
||||
err := c.eng.recordResult(
|
||||
webhookDB, d, 1, res.success,
|
||||
res.statusCode, res.respBody, res.errMsg,
|
||||
@@ -110,6 +148,7 @@ func (c *httpCore) fireAndForget(
|
||||
}
|
||||
|
||||
func (c *httpCore) withRetry(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
@@ -133,6 +172,10 @@ func (c *httpCore) withRetry(
|
||||
|
||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||
|
||||
if c.interruptedByStop(ctx, d, res) {
|
||||
return
|
||||
}
|
||||
|
||||
err := c.eng.recordResult(
|
||||
webhookDB, d, attemptNum, res.success,
|
||||
res.statusCode, res.respBody, res.errMsg,
|
||||
@@ -395,7 +438,7 @@ func (t *httpTarget) Deliver(
|
||||
}
|
||||
|
||||
t.deliver(
|
||||
webhookDB, d, task, sched,
|
||||
ctx, webhookDB, d, task, sched,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -13,9 +13,9 @@ import (
|
||||
const RedactionMarker = "(redacted)"
|
||||
|
||||
// Redactor removes one target's own credential material from
|
||||
// text that target's remote peer chose: a delivery response
|
||||
// body, or a delivery error stored before the delivery path
|
||||
// learned to mask the URLs it embeds.
|
||||
// a delivery response body, whose text the target's remote
|
||||
// peer chose, and from a stored delivery error as a second
|
||||
// line behind the delivery path's own URL masking.
|
||||
//
|
||||
// It removes byte-identical echoes of strings taken from the
|
||||
// target's stored configuration, and nothing else. Anything
|
||||
|
||||
@@ -82,7 +82,7 @@ func (t *slackTarget) Deliver(
|
||||
}
|
||||
|
||||
t.deliver(
|
||||
webhookDB, d, task, sched,
|
||||
ctx, webhookDB, d, task, sched,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -42,9 +42,6 @@ func TestDeliveryAttempts_ReadInTheTargetTypesOwnTerms(t *testing.T) {
|
||||
{database.TargetTypeLog, true, 0, "", "written to the log", ""},
|
||||
}
|
||||
|
||||
// Named for type and outcome, so that no two cases share a name:
|
||||
// Go adds "#01" to a repeated one, and a "#" in the temporary
|
||||
// directory's path cuts the path SQLiteDSN gives SQLite short.
|
||||
for _, tc := range cases {
|
||||
t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -12,13 +12,12 @@ import (
|
||||
//
|
||||
// The bound is enforced here and in SQL, because this page's
|
||||
// memory profile must not depend on a constant in another
|
||||
// package staying where it is, and because rows predating the
|
||||
// delivery engine's own cap or restored from an archive are
|
||||
// not covered by it at all.
|
||||
// package staying where it is, or on every stored row having
|
||||
// been written by the delivery engine under its own cap.
|
||||
//
|
||||
// It happens to equal that engine cap, so a row written by the
|
||||
// current engine reaches this one exactly and is never cut
|
||||
// twice. Nothing here may assume the two differ: see view.
|
||||
// engine reaches this one exactly and is never cut twice.
|
||||
// Nothing here may assume the two differ: see view.
|
||||
const maxRenderedResponseBytes = 4096
|
||||
|
||||
// deliveryResultColumns is the delivery attempt projection.
|
||||
@@ -40,9 +39,8 @@ const deliveryResultColumns = "delivery_id, attempt_num, created_at, " +
|
||||
//
|
||||
// Both Error and ResponseBody have been through the target's
|
||||
// Redactor. The engine already masks the URL out of the
|
||||
// errors it stores, so for errors this is a second line
|
||||
// covering rows written before it did; for response bodies it
|
||||
// is the only line, and its reach is what
|
||||
// errors it stores, so for errors this is a second line; for
|
||||
// response bodies it is the only line, and its reach is what
|
||||
// delivery.Redactor documents.
|
||||
type DeliveryResultView struct {
|
||||
AttemptNum int
|
||||
|
||||
@@ -49,13 +49,6 @@ var (
|
||||
// working retry configuration over a typo. A default answers a
|
||||
// question that was not asked; it never answers one that was asked
|
||||
// badly.
|
||||
//
|
||||
// A target stored with a count above the ceiling before this
|
||||
// validation existed keeps rendering and keeps delivering — nothing
|
||||
// clamps the row. Re-saving it from the edit form does have to bring
|
||||
// it into range, because the form submits the pre-filled value back
|
||||
// and accepting it would be the ceiling not applying to the edit
|
||||
// path. The 400 names the ceiling, so the fix is one field.
|
||||
func parseMaxRetries(raw string, fallback int) (int, error) {
|
||||
raw = strings.TrimSpace(raw)
|
||||
if raw == "" {
|
||||
|
||||
@@ -441,10 +441,9 @@ func (s *Session) Regenerate(
|
||||
//
|
||||
// Whichever comes first ends the session.
|
||||
//
|
||||
// A session that claims to be authenticated but carries no
|
||||
// timestamps predates this check; it is treated as expired so the
|
||||
// user re-authenticates rather than being granted an unbounded
|
||||
// session.
|
||||
// A session that claims to be authenticated but lacks the timestamp
|
||||
// a deadline is measured from is treated as expired, so the user
|
||||
// re-authenticates rather than being granted an unbounded session.
|
||||
func (s *Session) expired(sess *sessions.Session) bool {
|
||||
now := s.now()
|
||||
|
||||
|
||||
@@ -156,7 +156,8 @@ func TestVersion_IsStableAcrossInvocations(t *testing.T) {
|
||||
// The build target is the only place composing linker flags. If a
|
||||
// future edit drops either half, the binary silently reports "dev"
|
||||
// again (the defect this package exists for) or fails to link on
|
||||
// Alpine.
|
||||
// Alpine. If it stops passing GO_TAGS, the image's static binary links
|
||||
// glibc's getaddrinfo and dlopen again.
|
||||
func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -164,6 +165,7 @@ func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
||||
|
||||
require.Contains(t, makefile, "-X main.version=$(VERSION)")
|
||||
require.Contains(t, makefile, "$(GO_LDFLAGS)")
|
||||
require.Contains(t, makefile, "-tags '$(GO_TAGS)'")
|
||||
require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
|
||||
}
|
||||
|
||||
@@ -233,7 +235,9 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
|
||||
"a raw go build bypasses the Makefile's -X flag")
|
||||
require.Contains(t, dockerfile, "ARG VERSION")
|
||||
require.Contains(t, dockerfile,
|
||||
`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`)
|
||||
`make build VERSION="$VERSION"`+
|
||||
` GO_TAGS=netgo,osusergo,sqlite_omit_load_extension`+
|
||||
` GO_LDFLAGS='-extldflags "-static"'`)
|
||||
}
|
||||
|
||||
func read(t *testing.T, path string) string {
|
||||
|
||||
Reference in New Issue
Block a user