Compare commits
1
Commits
next
..
7e9e441d42
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7e9e441d42 |
+9
-15
@@ -214,22 +214,16 @@ RUN version="$(make version VERSION="$VERSION")"; \
|
|||||||
# (Alpine.js, extracted from its tarball in 3p/) first.
|
# (Alpine.js, extracted from its tarball in 3p/) first.
|
||||||
RUN make build VERSION="$VERSION"
|
RUN make build VERSION="$VERSION"
|
||||||
|
|
||||||
# Rebuild with static linking for the Alpine runtime. The cgo binary the
|
# Rebuild with static linking for Alpine runtime.
|
||||||
# step above builds is dynamically linked against glibc, which Alpine
|
# make build already verified compilation.
|
||||||
# (musl) does not have.
|
# 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.
|
||||||
#
|
#
|
||||||
# A statically linked glibc still loads shared libraries at run time for
|
# The static flags go in through GO_LDFLAGS rather than a -ldflags of
|
||||||
# getaddrinfo and dlopen, and Alpine has none to load, so the build tags
|
# their own: the build target composes them with the -X that stamps the
|
||||||
# keep both out of the binary: netgo and osusergo select Go's own DNS
|
# version, so this relink cannot silently drop the stamp.
|
||||||
# resolver and user lookup instead of glibc's, and
|
RUN CGO_ENABLED=1 make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'
|
||||||
# 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
|
# Runtime stage
|
||||||
# alpine:3.21, 2026-03-17
|
# alpine:3.21, 2026-03-17
|
||||||
|
|||||||
@@ -16,16 +16,14 @@ VERSION ?= $(shell script/version)
|
|||||||
# exists to correct.
|
# exists to correct.
|
||||||
override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
|
override VERSION := $(or $(strip $(VERSION)),$(shell script/version))
|
||||||
|
|
||||||
# Extra linker flags and build tags for the build target. The static
|
# Extra linker flags for the build target. The static relink in the
|
||||||
# relink in the Dockerfile passes -extldflags and its build tags through
|
# Dockerfile adds -extldflags here rather than passing its own -ldflags,
|
||||||
# these rather than a -ldflags of its own, so composing flags cannot drop
|
# so composing flags cannot drop the version stamp.
|
||||||
# the version stamp.
|
|
||||||
#
|
#
|
||||||
# The build target itself always passes -trimpath and -s -w, as the Go
|
# 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
|
# Dockerfile in REPO_POLICIES.md does: no build paths, symbol table or
|
||||||
# debug information in the binary.
|
# debug information in the binary.
|
||||||
GO_LDFLAGS ?=
|
GO_LDFLAGS ?=
|
||||||
GO_TAGS ?=
|
|
||||||
|
|
||||||
bootstrap:
|
bootstrap:
|
||||||
@script/bootstrap
|
@script/bootstrap
|
||||||
@@ -55,7 +53,7 @@ check:
|
|||||||
@script/check
|
@script/check
|
||||||
|
|
||||||
build: assets
|
build: assets
|
||||||
go build -trimpath -tags '$(GO_TAGS)' -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
|
go build -trimpath -ldflags '$(strip -s -w -X main.version=$(VERSION) $(GO_LDFLAGS))' -o bin/webhooker ./cmd/webhooker
|
||||||
|
|
||||||
run: build
|
run: build
|
||||||
./bin/webhooker
|
./bin/webhooker
|
||||||
|
|||||||
@@ -1317,22 +1317,20 @@ webhooker solves this by acting as a durable intermediary:
|
|||||||
nothing is lost if downstream targets are unavailable.
|
nothing is lost if downstream targets are unavailable.
|
||||||
|
|
||||||
2. **Guaranteed delivery** — Events are queued for delivery to each configured
|
2. **Guaranteed delivery** — Events are queued for delivery to each configured
|
||||||
target. Failed deliveries are retried with configurable backoff. Delivery
|
target. Failed deliveries are retried with configurable backoff. Every
|
||||||
attempts are logged with status codes, response bodies, and timing.
|
delivery attempt is logged with status codes, response bodies, and timing.
|
||||||
|
|
||||||
**That guarantee is at-least-once, not exactly-once.** When a send reaches
|
**That guarantee is at-least-once, not exactly-once.** When a send reaches
|
||||||
its target but the write recording that outcome fails, the delivery is
|
its target but the write recording that outcome fails, the delivery is
|
||||||
deliberately left in a recoverable state rather than marked done — losing a
|
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
|
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
|
fifteen minutes later, or the next restart does, and the target receives a
|
||||||
payload it already got. A send still in flight when the service stops is
|
payload it already got. webhooker adds no delivery identifier of its own to
|
||||||
left the same way: its attempt is not recorded, and the next start sends it
|
an outbound request, so **make your receiver idempotent** against whatever
|
||||||
again (see Shutdown). webhooker adds no delivery identifier of its own to an
|
the payload itself carries.
|
||||||
outbound request, so **make your receiver idempotent** against whatever the
|
|
||||||
payload itself carries.
|
|
||||||
|
|
||||||
3. **Observability** — Full request/response logging for every webhook received
|
3. **Observability** — Full request/response logging for every webhook received
|
||||||
and for delivery attempts. Prometheus metrics expose volume, latency, and
|
and every delivery attempted. Prometheus metrics expose volume, latency, and
|
||||||
error rates. The web UI provides real-time visibility into event flow.
|
error rates. The web UI provides real-time visibility into event flow.
|
||||||
|
|
||||||
4. **Fan-out** — A single incoming webhook can be delivered to multiple targets
|
4. **Fan-out** — A single incoming webhook can be delivered to multiple targets
|
||||||
@@ -1770,7 +1768,7 @@ is what bounds it.
|
|||||||
#### DeliveryResult
|
#### DeliveryResult
|
||||||
|
|
||||||
The result of a single delivery attempt. Every attempt (including retries) is
|
The result of a single delivery attempt. Every attempt (including retries) is
|
||||||
individually logged, except one cut off by a stop (see Shutdown).
|
individually logged for full observability.
|
||||||
|
|
||||||
| Field | Type | Description |
|
| Field | Type | Description |
|
||||||
| --------------- | ------- | ----------------------------------------- |
|
| --------------- | ------- | ----------------------------------------- |
|
||||||
@@ -2208,16 +2206,12 @@ This means:
|
|||||||
buffer rather than spawning more goroutines.
|
buffer rather than spawning more goroutines.
|
||||||
- **Independent results** — each worker records its own delivery result in the
|
- **Independent results** — each worker records its own delivery result in the
|
||||||
per-webhook database without coordination.
|
per-webhook database without coordination.
|
||||||
- **Graceful shutdown** — the stop hook cancels the workers' context. A worker
|
- **Graceful shutdown** — cancel the context, workers finish their current task
|
||||||
may still take tasks already queued before it exits. An `http` or `slack`
|
and exit. The stop hook waits for the pool via `lifecycle.WaitForShutdown`,
|
||||||
request in flight is cut off, and one started after the cancel fails at once.
|
which bounds that wait by fx's stop timeout rather than blocking forever on a
|
||||||
Neither is recorded or counted in the metrics, and the next start sends the
|
wedged worker. On timeout it logs at `ERROR` and returns an error, and the
|
||||||
delivery again (see Shutdown). A `database` or `log` task runs as normal. The
|
goroutines that did not finish are still running — an unclean shutdown is
|
||||||
stop hook waits for the pool via `lifecycle.WaitForShutdown`, which bounds
|
reported rather than hidden.
|
||||||
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:**
|
**Recovery paths:**
|
||||||
|
|
||||||
@@ -2323,7 +2317,7 @@ getting anywhere.
|
|||||||
| Metric | Type | Meaning |
|
| Metric | Type | Meaning |
|
||||||
| -------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
| -------------------------------------- | --------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||||
| `webhooker_events_received_total` | counter | Events received and durably stored. Compare against the delivery counters on one dashboard |
|
| `webhooker_events_received_total` | counter | Events received and durably stored. Compare against the delivery counters on one dashboard |
|
||||||
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying`. An attempt cut off by a stop is not counted (see Shutdown) |
|
| `webhooker_delivery_attempts_total` | counter | Delivery attempts actually dispatched to a target. A delivery a circuit breaker refused is not one: it is counted as a retry instead, but only when the refusal moves it into `retrying` |
|
||||||
| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
|
| `webhooker_deliveries_succeeded_total` | counter | Deliveries that reached `delivered` |
|
||||||
| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried |
|
| `webhooker_deliveries_failed_total` | counter | Deliveries that failed terminally and will not be retried |
|
||||||
| `webhooker_delivery_retries_total` | counter | Deliveries put back into `retrying` |
|
| `webhooker_delivery_retries_total` | counter | Deliveries put back into `retrying` |
|
||||||
@@ -3290,8 +3284,7 @@ stop-hook log:
|
|||||||
3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
|
3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
|
||||||
seconds**) and by what the hooks before it left, then a Sentry flush if
|
seconds**) and by what the hooks before it left, then a Sentry flush if
|
||||||
`SENTRY_DSN` is set
|
`SENTRY_DSN` is set
|
||||||
4. `delivery.Engine` — cuts off any `http` or `slack` request in flight, waits
|
4. `delivery.Engine` — waits for its workers, then closes the archive databases
|
||||||
for its workers, then closes the archive databases
|
|
||||||
5. `healthcheck`
|
5. `healthcheck`
|
||||||
6. `WebhookDBManager`
|
6. `WebhookDBManager`
|
||||||
7. the database close
|
7. the database close
|
||||||
@@ -3302,12 +3295,6 @@ 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
|
server hook is ever entered. The hooks after the server are microsecond-scale in
|
||||||
normal operation.
|
normal operation.
|
||||||
|
|
||||||
The delivery engine cancels a delivery request in flight. That attempt is not
|
|
||||||
recorded or counted in the metrics, 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 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
|
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,
|
budget at the instant it finished, and every later hook — the delivery engine,
|
||||||
@@ -3432,14 +3419,12 @@ fixed independently of the compiler's:
|
|||||||
phases and the `css-check` and `markdown-check` stages passing (it copies a
|
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/`
|
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
|
first), and then rebuilds the binary with `CGO_ENABLED=1` and static linking
|
||||||
so it runs on musl, with the build tags `netgo`, `osusergo` and
|
so it runs on musl. Both builds go through `make build`, the relink adding
|
||||||
`sqlite_omit_load_extension` so it needs none of glibc's shared libraries at
|
its `-extldflags` via `GO_LDFLAGS`, so neither can drop the `-X` that stamps
|
||||||
run time. Both builds go through `make build`, the relink adding its
|
the version. The version is the `VERSION` build arg if one is given,
|
||||||
`-extldflags` via `GO_LDFLAGS` and its tags via `GO_TAGS`, so neither can
|
otherwise derived from the `.git` in the context, and the stage fails if a
|
||||||
drop the `-X` that stamps the version. The version is the `VERSION` build arg
|
context with `.git` would stamp an empty version, `dev` or `unknown` (see
|
||||||
if one is given, otherwise derived from the `.git` in the context, and the
|
[Version stamping](#version-stamping)).
|
||||||
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
|
7. **Runtime stage** (`alpine:3.21`) — copies the static binary and
|
||||||
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
|
`deploy/docker-entrypoint.sh`, creates the `/var/lib/webhooker` directory for
|
||||||
all SQLite databases, exposes port 8080, and includes a health check against
|
all SQLite databases, exposes port 8080, and includes a health check against
|
||||||
|
|||||||
@@ -241,19 +241,7 @@ func SQLiteDSN(path, mode string) string {
|
|||||||
)
|
)
|
||||||
q.Add("_pragma", "journal_mode(WAL)")
|
q.Add("_pragma", "journal_mode(WAL)")
|
||||||
|
|
||||||
// SQLite reads a file: URI's path only up to the first ? or #, and
|
return "file:" + path + "?" + q.Encode()
|
||||||
// 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
|
// OpenSQLite opens the SQLite file at path with the service's
|
||||||
|
|||||||
@@ -2,8 +2,6 @@ package database_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"database/sql"
|
|
||||||
"os"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
@@ -114,79 +112,6 @@ func TestMainDBAppliesPragmasOnALiveHandle(t *testing.T) {
|
|||||||
assert.Equal(t, "10000", busy)
|
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
|
// TestConcurrentReaderDoesNotBlockWrites is the unit-scale form of the
|
||||||
// reproduction in
|
// reproduction in
|
||||||
// https://git.eeqj.de/sneak/webhooker/issues/256: an operator's
|
// https://git.eeqj.de/sneak/webhooker/issues/256: an operator's
|
||||||
|
|||||||
@@ -32,9 +32,8 @@ func targetTotals(
|
|||||||
|
|
||||||
// TestUpdateDeliveryStatus_FinishTimeAndTargetTotals pins what a status
|
// TestUpdateDeliveryStatus_FinishTimeAndTargetTotals pins what a status
|
||||||
// write records for the webhook page's statistics: the time a delivery
|
// write records for the webhook page's statistics: the time a delivery
|
||||||
// finished, set only when it becomes delivered or failed and stored in
|
// finished, set only when it becomes delivered or failed, and one more
|
||||||
// the same form as created_at, and one more on its target's delivered
|
// on its target's delivered or failed total.
|
||||||
// or failed total.
|
|
||||||
func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) {
|
func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
@@ -76,16 +75,6 @@ func TestUpdateDeliveryStatus_FinishTimeAndTargetTotals(t *testing.T) {
|
|||||||
if tt.finished {
|
if tt.finished {
|
||||||
require.NotNil(t, stored.FinishedAt)
|
require.NotNil(t, stored.FinishedAt)
|
||||||
assert.False(t, stored.FinishedAt.Before(before))
|
assert.False(t, stored.FinishedAt.Before(before))
|
||||||
|
|
||||||
var createdAt, finishedAt string
|
|
||||||
|
|
||||||
require.NoError(t, db.Raw(
|
|
||||||
`SELECT CAST(created_at AS TEXT),
|
|
||||||
CAST(finished_at AS TEXT)
|
|
||||||
FROM deliveries WHERE id = ?`, d.ID,
|
|
||||||
).Row().Scan(&createdAt, &finishedAt))
|
|
||||||
assertStoredTime(t, createdAt)
|
|
||||||
assertStoredTime(t, finishedAt)
|
|
||||||
} else {
|
} else {
|
||||||
assert.Nil(t, stored.FinishedAt)
|
assert.Nil(t, stored.FinishedAt)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1702,10 +1702,7 @@ func writeDeliveryStatus(
|
|||||||
}).
|
}).
|
||||||
Updates(map[string]any{
|
Updates(map[string]any{
|
||||||
"status": status,
|
"status": status,
|
||||||
// GORM's clock, the one that stamps created_at. A bare
|
"finished_at": time.Now(),
|
||||||
// time.Now() carries a monotonic clock reading, which the
|
|
||||||
// driver would store as an " m=+…" suffix.
|
|
||||||
"finished_at": tx.NowFunc(),
|
|
||||||
})
|
})
|
||||||
if res.Error != nil || res.RowsAffected == 0 {
|
if res.Error != nil || res.RowsAffected == 0 {
|
||||||
return res.Error
|
return res.Error
|
||||||
@@ -1909,13 +1906,12 @@ func (e *Engine) takeForRedispatch(
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
// GORM's clock, as writeDeliveryStatus explains.
|
|
||||||
res := webhookDB.
|
res := webhookDB.
|
||||||
Model(&database.Delivery{}).
|
Model(&database.Delivery{}).
|
||||||
Where(
|
Where(
|
||||||
"id = ? AND status = ?", deliveryID, eligible,
|
"id = ? AND status = ?", deliveryID, eligible,
|
||||||
).
|
).
|
||||||
UpdateColumn("updated_at", webhookDB.NowFunc())
|
UpdateColumn("updated_at", time.Now())
|
||||||
|
|
||||||
if res.Error != nil {
|
if res.Error != nil {
|
||||||
e.log.Error(
|
e.log.Error(
|
||||||
|
|||||||
@@ -2,16 +2,11 @@ package delivery_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"io"
|
|
||||||
"net/http"
|
|
||||||
"net/http/httptest"
|
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sync"
|
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/google/uuid"
|
"github.com/google/uuid"
|
||||||
"github.com/stretchr/testify/assert"
|
|
||||||
"github.com/stretchr/testify/require"
|
"github.com/stretchr/testify/require"
|
||||||
"go.uber.org/fx"
|
"go.uber.org/fx"
|
||||||
"gorm.io/gorm/clause"
|
"gorm.io/gorm/clause"
|
||||||
@@ -277,154 +272,6 @@ func TestEngine_StopHookHonoursStopTimeout(t *testing.T) {
|
|||||||
requireStopHookExpires(t, lc.hooks[0], "delivery engine")
|
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 and no count in
|
|
||||||
// the attempt metrics, for restart recovery to send.
|
|
||||||
func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
|
|
||||||
t.Parallel()
|
|
||||||
|
|
||||||
s := newISetup(t)
|
|
||||||
reg := mIsolate(t, s)
|
|
||||||
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))
|
|
||||||
assert.Zero(t, mCounter(t, reg, mAttempts, mTypeHTTP))
|
|
||||||
assert.Zero(t, mHTTPDurations(t, reg))
|
|
||||||
|
|
||||||
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,
|
// deliverToArchive gives the setup's webhook a database target,
|
||||||
// runs one delivery to it through the running engine, and returns
|
// runs one delivery to it through the running engine, and returns
|
||||||
// the target's ID and archive file path. The archive writer holds
|
// the target's ID and archive file path. The archive writer holds
|
||||||
|
|||||||
@@ -117,18 +117,6 @@ func seedDelivery(
|
|||||||
return d
|
return d
|
||||||
}
|
}
|
||||||
|
|
||||||
// assertStoredTime asserts that text, a timestamp as SQLite holds it,
|
|
||||||
// is in the form GORM stores created_at in: the layout of
|
|
||||||
// time.Time.String, with no " m=+…" monotonic clock reading after it.
|
|
||||||
func assertStoredTime(t *testing.T, text string) {
|
|
||||||
t.Helper()
|
|
||||||
|
|
||||||
_, err := time.Parse(
|
|
||||||
"2006-01-02 15:04:05.999999999 -0700 MST", text,
|
|
||||||
)
|
|
||||||
assert.NoError(t, err, "stored as %q", text)
|
|
||||||
}
|
|
||||||
|
|
||||||
// httpDeliveryFixture holds the task and delivery needed
|
// httpDeliveryFixture holds the task and delivery needed
|
||||||
// for HTTP delivery tests.
|
// for HTTP delivery tests.
|
||||||
type httpDeliveryFixture struct {
|
type httpDeliveryFixture struct {
|
||||||
|
|||||||
@@ -462,8 +462,7 @@ func TestScheduledRetryIsNotSweptDuringBackoff(t *testing.T) {
|
|||||||
|
|
||||||
// TestRedispatchStampsTheRow pins the cadence control: a stranded
|
// TestRedispatchStampsTheRow pins the cadence control: a stranded
|
||||||
// delivery that has just been handed out is not selected again by the
|
// delivery that has just been handed out is not selected again by the
|
||||||
// next tick a minute later. The stamp is stored in the same form as
|
// next tick a minute later.
|
||||||
// created_at.
|
|
||||||
func TestRedispatchStampsTheRow(t *testing.T) {
|
func TestRedispatchStampsTheRow(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
@@ -494,13 +493,4 @@ func TestRedispatchStampsTheRow(t *testing.T) {
|
|||||||
"a re-dispatched delivery must be stamped so the "+
|
"a re-dispatched delivery must be stamped so the "+
|
||||||
"next tick does not select it again",
|
"next tick does not select it again",
|
||||||
)
|
)
|
||||||
|
|
||||||
var createdAt, updatedAt string
|
|
||||||
|
|
||||||
require.NoError(t, s.WebhookDB.Raw(
|
|
||||||
`SELECT CAST(created_at AS TEXT), CAST(updated_at AS TEXT)
|
|
||||||
FROM deliveries WHERE id = ?`, d.ID,
|
|
||||||
).Row().Scan(&createdAt, &updatedAt))
|
|
||||||
assertStoredTime(t, createdAt)
|
|
||||||
assertStoredTime(t, updatedAt)
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -32,8 +32,7 @@ type Scheduler interface {
|
|||||||
// Engine.observeAttempt, alongside the DeliveryResult it records for
|
// Engine.observeAttempt, alongside the DeliveryResult it records for
|
||||||
// it. Deliver is also entered for attempts that never happen — an
|
// it. Deliver is also entered for attempts that never happen — an
|
||||||
// open circuit breaker refuses one — so the count cannot be taken
|
// open circuit breaker refuses one — so the count cannot be taken
|
||||||
// from around this call. An attempt cut off by a stop is neither
|
// from around this call.
|
||||||
// counted nor recorded.
|
|
||||||
type Target interface {
|
type Target interface {
|
||||||
Deliver(
|
Deliver(
|
||||||
ctx context.Context,
|
ctx context.Context,
|
||||||
|
|||||||
@@ -277,11 +277,7 @@ func (w *archiveWriter) write(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// GORM's clock, the one that stamps created_at in the other
|
row.ArchivedAt = time.Now()
|
||||||
// databases, so this is stored in the same form. A bare time.Now()
|
|
||||||
// carries a monotonic clock reading, which the driver would store
|
|
||||||
// as an " m=+…" suffix.
|
|
||||||
row.ArchivedAt = w.db.NowFunc()
|
|
||||||
|
|
||||||
err := w.db.Create(&row).Error
|
err := w.db.Create(&row).Error
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -150,13 +150,6 @@ func TestArchiveWriter_WritesRow(t *testing.T) {
|
|||||||
assert.Equal(t, row.ContentType, got[0].ContentType)
|
assert.Equal(t, row.ContentType, got[0].ContentType)
|
||||||
assert.JSONEq(t, `{"hello":"world"}`, got[0].Body)
|
assert.JSONEq(t, `{"hello":"world"}`, got[0].Body)
|
||||||
assert.False(t, got[0].ArchivedAt.IsZero())
|
assert.False(t, got[0].ArchivedAt.IsZero())
|
||||||
|
|
||||||
var archivedAt string
|
|
||||||
|
|
||||||
require.NoError(t, w.DB().Raw(
|
|
||||||
"SELECT CAST(archived_at AS TEXT) FROM archived_events",
|
|
||||||
).Row().Scan(&archivedAt))
|
|
||||||
assertStoredTime(t, archivedAt)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestArchiveWriter_RecreatesAfterRemoval(
|
func TestArchiveWriter_RecreatesAfterRemoval(
|
||||||
|
|||||||
@@ -57,11 +57,7 @@ type httpCore struct {
|
|||||||
// recorded and no circuit breaker is consulted. A positive
|
// recorded and no circuit breaker is consulted. A positive
|
||||||
// maxRetries gates the attempt on the circuit breaker and
|
// maxRetries gates the attempt on the circuit breaker and
|
||||||
// schedules a backed-off retry on failure.
|
// 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(
|
func (c *httpCore) deliver(
|
||||||
ctx context.Context,
|
|
||||||
webhookDB *gorm.DB,
|
webhookDB *gorm.DB,
|
||||||
d *database.Delivery,
|
d *database.Delivery,
|
||||||
task *Task,
|
task *Task,
|
||||||
@@ -70,55 +66,21 @@ func (c *httpCore) deliver(
|
|||||||
attempt func() attemptResult,
|
attempt func() attemptResult,
|
||||||
) {
|
) {
|
||||||
if maxRetries == 0 {
|
if maxRetries == 0 {
|
||||||
c.fireAndForget(ctx, webhookDB, d, attempt())
|
c.fireAndForget(webhookDB, d, attempt())
|
||||||
|
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
c.withRetry(
|
c.withRetry(
|
||||||
ctx, webhookDB, d, task, sched, maxRetries, attempt,
|
webhookDB, d, task, sched, maxRetries, attempt,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
// interruptedByStop reports whether an attempt failed because the
|
|
||||||
// engine is stopping, which cancels the context its request is made
|
|
||||||
// with. Such an attempt is not recorded: no DeliveryResult, no circuit
|
|
||||||
// breaker outcome, no status change, no count in the attempt metrics.
|
|
||||||
// The delivery stays pending or retrying, and startup recovery sends
|
|
||||||
// it again on the next start.
|
|
||||||
//
|
|
||||||
// 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(
|
func (c *httpCore) fireAndForget(
|
||||||
ctx context.Context,
|
|
||||||
webhookDB *gorm.DB,
|
webhookDB *gorm.DB,
|
||||||
d *database.Delivery,
|
d *database.Delivery,
|
||||||
res attemptResult,
|
res attemptResult,
|
||||||
) {
|
) {
|
||||||
if c.interruptedByStop(ctx, d, res) {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||||
|
|
||||||
err := c.eng.recordResult(
|
err := c.eng.recordResult(
|
||||||
@@ -148,7 +110,6 @@ func (c *httpCore) fireAndForget(
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (c *httpCore) withRetry(
|
func (c *httpCore) withRetry(
|
||||||
ctx context.Context,
|
|
||||||
webhookDB *gorm.DB,
|
webhookDB *gorm.DB,
|
||||||
d *database.Delivery,
|
d *database.Delivery,
|
||||||
task *Task,
|
task *Task,
|
||||||
@@ -170,10 +131,6 @@ func (c *httpCore) withRetry(
|
|||||||
|
|
||||||
res := attempt()
|
res := attempt()
|
||||||
|
|
||||||
if c.interruptedByStop(ctx, d, res) {
|
|
||||||
return
|
|
||||||
}
|
|
||||||
|
|
||||||
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
c.eng.observeAttempt(d.Target.Type, res.elapsed())
|
||||||
|
|
||||||
err := c.eng.recordResult(
|
err := c.eng.recordResult(
|
||||||
@@ -438,7 +395,7 @@ func (t *httpTarget) Deliver(
|
|||||||
}
|
}
|
||||||
|
|
||||||
t.deliver(
|
t.deliver(
|
||||||
ctx, webhookDB, d, task, sched,
|
webhookDB, d, task, sched,
|
||||||
d.Target.MaxRetries, attempt,
|
d.Target.MaxRetries, attempt,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -82,7 +82,7 @@ func (t *slackTarget) Deliver(
|
|||||||
}
|
}
|
||||||
|
|
||||||
t.deliver(
|
t.deliver(
|
||||||
ctx, webhookDB, d, task, sched,
|
webhookDB, d, task, sched,
|
||||||
d.Target.MaxRetries, attempt,
|
d.Target.MaxRetries, attempt,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -42,6 +42,9 @@ func TestDeliveryAttempts_ReadInTheTargetTypesOwnTerms(t *testing.T) {
|
|||||||
{database.TargetTypeLog, true, 0, "", "written to the log", ""},
|
{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 {
|
for _, tc := range cases {
|
||||||
t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) {
|
t.Run(string(tc.targetType)+" "+tc.outcome, func(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|||||||
@@ -156,8 +156,7 @@ func TestVersion_IsStableAcrossInvocations(t *testing.T) {
|
|||||||
// The build target is the only place composing linker flags. If a
|
// The build target is the only place composing linker flags. If a
|
||||||
// future edit drops either half, the binary silently reports "dev"
|
// future edit drops either half, the binary silently reports "dev"
|
||||||
// again (the defect this package exists for) or fails to link on
|
// again (the defect this package exists for) or fails to link on
|
||||||
// Alpine. If it stops passing GO_TAGS, the image's static binary links
|
// Alpine.
|
||||||
// glibc's getaddrinfo and dlopen again.
|
|
||||||
func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
@@ -165,7 +164,6 @@ func TestMakefile_BuildComposesVersionAndExtraFlags(t *testing.T) {
|
|||||||
|
|
||||||
require.Contains(t, makefile, "-X main.version=$(VERSION)")
|
require.Contains(t, makefile, "-X main.version=$(VERSION)")
|
||||||
require.Contains(t, makefile, "$(GO_LDFLAGS)")
|
require.Contains(t, makefile, "$(GO_LDFLAGS)")
|
||||||
require.Contains(t, makefile, "-tags '$(GO_TAGS)'")
|
|
||||||
require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
|
require.Contains(t, makefile, "VERSION ?= $(shell script/version)")
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -235,9 +233,7 @@ func TestDockerfile_BuildsThroughTheMakeTarget(t *testing.T) {
|
|||||||
"a raw go build bypasses the Makefile's -X flag")
|
"a raw go build bypasses the Makefile's -X flag")
|
||||||
require.Contains(t, dockerfile, "ARG VERSION")
|
require.Contains(t, dockerfile, "ARG VERSION")
|
||||||
require.Contains(t, dockerfile,
|
require.Contains(t, dockerfile,
|
||||||
`make build VERSION="$VERSION"`+
|
`make build VERSION="$VERSION" GO_LDFLAGS='-extldflags "-static"'`)
|
||||||
` GO_TAGS=netgo,osusergo,sqlite_omit_load_extension`+
|
|
||||||
` GO_LDFLAGS='-extldflags "-static"'`)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
func read(t *testing.T, path string) string {
|
func read(t *testing.T, path string) string {
|
||||||
|
|||||||
Reference in New Issue
Block a user