4 Commits
Author SHA1 Message Date
clawbot 55ccb28270 Run the tests with cgo off and without the race detector (closes #538)
check / check (push) Canceled after 0s
sneak ruled the race detector out
(#538 (comment)), and on
Linux it was the only reason the tests needed cgo. The test phase and the
browser test binary now build with CGO_ENABLED=0 and without -race, so no
test binary compiles mattn's C SQLite. The test phase keeps the builder
stage's Debian Go image. Its comments, the README and the code comments that
said the suite runs under -race now say what is true, and the README notes
the exception to the vendored REPO_POLICIES.md.

Model: opus-5-5
2026-10-21 17:28:50 +02:00
clawbot 06be075b70 Store events and create deliveries in the delivery engine (closes #544)
check / check (push) Canceled after 0s
The receiver, resubmit and replay handlers now read and check the
request and make one call. Engine.StoreEvent stores an event with a
pending delivery for each active target and the totals in one
transaction, then queues them; Engine.CreateReplay does the same for
one replay delivery. The handlers take *delivery.Engine, so the
Notifier interface and its bridge are gone, and the engine's notify is
no longer exported. Archives and CircuitBreakers stay because the
handler tests fake them. The tests that read a queued task moved to
the engine's tests; the handler tests use an engine that is never
started.

Model: opus-5-5
2026-10-16 00:51:26 +02:00
clawbot 8406c77949 Store a target's settings as columns (closes #543)
check / check (push) Canceled after 0s
A target's settings move from the per-type JSON string in
targets.config to columns: url, headers, timeout_seconds,
forward_query, archive_expiry and archive_rotation. A column a type
does not use stays empty. headers holds the header map as JSON text
through GORM's JSON serializer, and archive_expiry holds a
time.Duration, 0 meaning never. The forms check each setting once,
with the same refusals and messages, and the edit form is filled from
the columns, so the per-type config structs, their parsers and the
"could not be read" branches are gone. A delivery task carries the
six settings in place of the JSON string; tests check that each copy
into a task, and back out of one, keeps all six.

Model: opus-5-5
2026-10-15 23:02:39 +02:00
clawbot ded39aa158 Use one recovery routine at start and on every sweep tick (closes #542)
check / check (push) Canceled after 0s
Start-up recovery and the 60-second sweep were two parallel sets of
functions doing the same job. recoverDeliveries now does it, run once at
start and then on every tick from one goroutine. A retrying delivery is
scheduled after what is left of its backoff, now if nothing is left, so
backoffElapsed is gone. The run at start still takes every pending
delivery and opens a webhook database whose file is missing; a tick
still takes only pending deliveries untouched for 15 minutes, at most
500, and skips a missing file. A retrying delivery is taken before its
event is read. The README describes one routine.

Model: opus-5-5
2026-10-15 21:57:05 +02:00
38 changed files with 1831 additions and 1323 deletions

No files matched your search

+13 -8
View File
@@ -111,11 +111,16 @@ RUN --network=none golangci-lint run --config .golangci.yml --build-tags browser
# Nothing is wanted from js-lint; the copy is what makes this phase run it. # Nothing is wanted from js-lint; the copy is what makes this phase run it.
COPY --from=js-lint /src/yarn.lock /dev/null COPY --from=js-lint /src/yarn.lock /dev/null
# Test phase. -race needs cgo and so a C compiler, which the Debian Go image # Test phase. The tests run with cgo off and so without Go's race detector,
# ships and the alpine one does not. `make test` (script/test) builds this # which needs cgo on Linux; sneak ruled the race detector out in
# stage alone; the build stage below depends on it. # https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273.
# `make test` (script/test) builds this stage alone; the build stage below
# depends on it.
# #
# golang:1.26.1-bookworm (Debian-based), 2026-03-17 # golang:1.26.1-bookworm (Debian-based), 2026-03-17: the builder stage's
# image, which that stage needs for its git and make. Its test step runs
# under the image's bash, and the tests in internal/versionscript use its
# git, make and sh, and skip when they are missing.
FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS test FROM golang:1.26.1-bookworm@sha256:4465644228bc2857a954b092167e12aa59c006a3492282a6c820bf4755fd64a4 AS test
WORKDIR /src WORKDIR /src
@@ -131,8 +136,8 @@ RUN script/assets
# -timeout applies to each package on its own, so 90s has only to clear the # -timeout applies to each package on its own, so 90s has only to clear the
# slowest one. -p 4 -parallel 8 keep the run under 2 GB of memory: at most # slowest one. -p 4 -parallel 8 keep the run under 2 GB of memory: at most
# four test binaries build or run at once, each with at most eight parallel # four test binaries build or run at once, each with at most eight parallel
# tests. Under -race every test binary and every link costs a few hundred MB, # tests. The defaults are one per core, so without these the memory the run
# so the defaults (one per core) add up to several GB on a many-core host. # needs grows with the host's core count.
# #
# The first run has no -v: go test then prints one result line per package, # The first run has no -v: go test then prints one result line per package,
# with its coverage, and for a package that fails, everything its tests # with its coverage, and for a package that fails, everything its tests
@@ -154,12 +159,12 @@ RUN script/assets
SHELL ["/bin/bash", "-o", "pipefail", "-c"] SHELL ["/bin/bash", "-o", "pipefail", "-c"]
RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \ RUN --mount=type=tmpfs,target=/tmp/tests,size=512m \
export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \ export TMPDIR=/tmp/tests GOTMPDIR=/tmp; \
go test -race -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \ CGO_ENABLED=0 go test -cover -p 4 -parallel 8 -timeout 90s ./... 2>&1 | tee /tmp/go-test.log && exit 0; \
tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \ tests="$(awk '/^--- FAIL: / { print $3 }' /tmp/go-test.log | paste -s -d '|' -)"; \
packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \ packages="$(awk '/^FAIL\t/ { print $2 }' /tmp/go-test.log)"; \
if [ -n "$tests" ]; then \ if [ -n "$tests" ]; then \
echo "--- Rerunning the failed tests with -v for details ---"; \ echo "--- Rerunning the failed tests with -v for details ---"; \
go test -race -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \ CGO_ENABLED=0 go test -v -p 4 -parallel 8 -timeout 90s -run "^($tests)\$" $packages; \
fi; \ fi; \
exit 1 exit 1
+3 -3
View File
@@ -15,9 +15,9 @@ RUN go mod download
COPY . . COPY . .
# The test binary embeds the templates and static files, so the browser # The test binary embeds the templates and static files, so the browser
# stage needs nothing else. -p 4 keeps the compile's memory down, as in # stage needs nothing else. It is built with cgo off and -p 4 keeps the
# the test phase of Dockerfile. # compile's memory down, both as in the test phase of Dockerfile.
RUN make assets && go test -c -p 4 -tags browser -o /browser.test ./internal/server RUN make assets && CGO_ENABLED=0 go test -c -p 4 -tags browser -o /browser.test ./internal/server
# chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The # chromedp/headless-shell:151.0.7922.109 (Debian trixie), 2026-08-11. The
# browser is on PATH as headless-shell, where the test's browser library # browser is on PATH as headless-shell, where the test's browser library
+79 -47
View File
@@ -60,7 +60,7 @@ make assets # Extract Alpine.js from 3p/ (build and dev run it)
make fmt # Format Go (gofmt + goimports) and Markdown (prettier, in Docker) make fmt # Format Go (gofmt + goimports) and Markdown (prettier, in Docker)
make fmt-check # Fail if gofmt or prettier would change anything (writes nothing) make fmt-check # Fail if gofmt or prettier would change anything (writes nothing)
make lint # Run golangci-lint and ESLint in Docker make lint # Run golangci-lint and ESLint in Docker
make test # Run tests with race detection, in Docker make test # Run tests with cgo off, in Docker
make test-browser # Run the browser test in Docker (Dockerfile.browser) make test-browser # Run the browser test in Docker (Dockerfile.browser)
make check # test + lint + fmt-check + css-check (CI gate) make check # test + lint + fmt-check + css-check (CI gate)
make build # Build binary to bin/webhooker (version-stamped) make build # Build binary to bin/webhooker (version-stamped)
@@ -75,6 +75,11 @@ make css-check # Fail if static/css/tailwind.css is stale (writes nothing)
make clean # Remove bin/ make clean # Remove bin/
``` ```
The tests run with cgo off and so without Go's race detector, which needs cgo on
Linux: an exception to the `-race` in the test phase of the vendored
`REPO_POLICIES.md`, ruled by @sneak in
https://git.eeqj.de/sneak/webhooker/issues/538#issuecomment-153273.
### Configuration ### Configuration
All configuration is via environment variables. For local development, you can All configuration is via environment variables. For local development, you can
@@ -1346,11 +1351,11 @@ webhooker solves this by acting as a durable intermediary:
**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 sweep picks it up about fifteen
fifteen minutes later, or the next restart does, and the target receives a minutes later, or the next restart does, and the target receives a payload
payload it already got. A send still in flight when the service stops is it already got. A send still in flight when the service stops is left the
left the same way: its attempt is not recorded, and the next start sends it same way: its attempt is not recorded, and the next start sends it again
again (see Shutdown). webhooker adds no delivery identifier of its own to an (see Shutdown). webhooker adds no delivery identifier of its own to an
outbound request, so **make your receiver idempotent** against whatever the outbound request, so **make your receiver idempotent** against whatever the
payload itself carries. payload itself carries.
@@ -1626,8 +1631,9 @@ Each type uses only its own settings, and the ones it does not use stay empty:
`http` uses `url`, `headers`, `timeout_seconds` and `forward_query`; `slack` `http` uses `url`, `headers`, `timeout_seconds` and `forward_query`; `slack`
uses `url`; `database` uses `archive_expiry` and `archive_rotation`; `log` uses uses `url`; `database` uses `archive_expiry` and `archive_rotation`; `log` uses
none. The add target form, the new webhook form and the target edit form check none. The add target form, the new webhook form and the target edit form check
every setting before it is saved, so nothing that reads a target checks it every setting before it is saved, and each setting is a column of its own, so
again. nothing that reads a target parses its settings or has to handle one it cannot
read.
An `http` target POSTs the event to its `url`. Its `headers` are applied last, An `http` target POSTs the event to its `url`. Its `headers` are applied last,
so they win over the event's own forwarded headers. so they win over the event's own forwarded headers.
@@ -1869,8 +1875,8 @@ These indexes on the per-webhook event databases are created by
to match: to match:
| Table | Columns | Serves | | Table | Columns | Serves |
| ------------------ | ------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ------------------ | ------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Startup recovery, the retry and pending sweeps every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, the webhook page's statistics and target list and the webhook list, which count each target's deliveries by status and when they finished, and the event log, which lists and counts the events with a failed delivery or one pending or retrying | | `deliveries` | `status`, `deleted_at`, `finished_at`, `target_id` | Delivery recovery at start and every 60 seconds and the queue-depth sampler every 30 seconds, which select deliveries by status, the webhook page's statistics and target list and the webhook list, which count each target's deliveries by status and when they finished, and the event log, which lists and counts the events with a failed delivery or one pending or retrying |
| `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events | | `deliveries` | `event_id`, `deleted_at` | The event log, which loads each event's deliveries, and retention, which counts and deletes the deliveries of expired events |
| `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events | | `delivery_results` | `delivery_id`, `deleted_at` | The event log, which loads the attempts of a page's deliveries, and retention, which deletes the attempts of expired events |
| `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events | | `events` | `deleted_at`, `created_at` | The webhook page's statistics, which count recent events |
@@ -2152,12 +2158,8 @@ External Service
1. Look up Entrypoint by UUID — 404 if unknown, 1. Look up Entrypoint by UUID — 404 if unknown,
410 if inactive 410 if inactive
2. Read the body under the 1 MB cap 2. Read the body under the 1 MB cap
3. Capture full request as Event 3. Hand the full request, as an Event, to
4. Create Delivery records for each active Target Engine.StoreEvent
5. Build self-contained delivery.Task structs
(target settings + event data inline for
bodies < 16 KiB)
6. Notify Engine via channel (no DB read needed)
│ │
▼ ▼
┌──────────────┐ ┌──────────────┐
@@ -2166,6 +2168,13 @@ External Service
│ (worker │ │ (worker │
│ pool) │ │ pool) │
└──────┬───────┘ └──────┬───────┘
4. Store the Event, a pending Delivery for each
active Target, and the totals, in one
transaction
5. Build a delivery.Task for each Delivery
(target settings + event data, the body
inline when it is under 16 KiB)
6. Queue the tasks on the delivery channel
│ │
┌── bounded worker pool (N workers) ──┐ ┌── bounded worker pool (N workers) ──┐
▼ ▼ ▼ ▼ ▼ ▼
@@ -2179,6 +2188,12 @@ External Service
└────────────┘ └────────────┘
``` ```
A **Resubmit** stores its copy of an event through the same `Engine.StoreEvent`,
and a **Replay** adds one delivery of a stored event through
`Engine.CreateReplay`, which writes the delivery and its target's totals in one
transaction and queues it. Those two methods are the only code that creates a
delivery; the handlers read and check the request, then make the one call.
### Bounded Worker Pool ### Bounded Worker Pool
The delivery engine uses a **fixed-size worker pool** (default: 10 workers) to The delivery engine uses a **fixed-size worker pool** (default: 10 workers) to
@@ -2188,8 +2203,9 @@ preventing goroutine explosions regardless of queue depth.
**Architecture:** **Architecture:**
- **Channels as queues:** Two buffered channels serve as bounded queues: a - **Channels as queues:** Two buffered channels serve as bounded queues: a
delivery channel (new tasks from the webhook handler) and a retry channel delivery channel (the tasks of newly stored deliveries and of the `pending`
(tasks from backoff timers). Both are buffered to 10,000. deliveries the recovery routine hands back) and a retry channel (tasks from
backoff timers). Both are buffered to 10,000.
- **Fan-out via channel, not goroutines:** When an event arrives with multiple - **Fan-out via channel, not goroutines:** When an event arrives with multiple
targets, each `delivery.Task` is sent to the delivery channel. Workers pick targets, each `delivery.Task` is sent to the delivery channel. Workers pick
them up and process them — no goroutine-per-target. them up and process them — no goroutine-per-target.
@@ -2198,7 +2214,7 @@ preventing goroutine explosions regardless of queue depth.
Workers are the ONLY goroutines doing actual HTTP delivery. Workers are the ONLY goroutines doing actual HTTP delivery.
- **Retry backpressure with DB fallback:** When a retry timer fires and the - **Retry backpressure with DB fallback:** When a retry timer fires and the
retry channel is full, the timer is dropped — the delivery stays in `retrying` retry channel is full, the timer is dropped — the delivery stays in `retrying`
status in the database. A periodic sweep (every 60s) scans for these status in the database. The sweep (every 60s, see Recovery below) finds these
"orphaned" retries and re-queues them. No blocked goroutines, no unbounded "orphaned" retries and re-queues them. No blocked goroutines, no unbounded
timer chains. timer chains.
- **Bounded concurrency:** At most N deliveries (N = number of workers) are - **Bounded concurrency:** At most N deliveries (N = number of workers) are
@@ -2225,24 +2241,36 @@ This means:
that did not finish are still running — an unclean shutdown is reported rather that did not finish are still running — an unclean shutdown is reported rather
than hidden. than hidden.
**Recovery paths:** **Recovery:** one routine finds the deliveries the engine is not working on and
hands them back. It runs once when the engine starts, and then every 60 seconds;
that periodic run is the sweep. For every webhook it reads the `pending` and
`retrying` deliveries. One that already has a successful attempt is marked
`delivered` and not sent again. A `retrying` delivery gets a backoff timer for
what is left of its backoff, and is queued at once if nothing is left. A
`pending` delivery is queued on the delivery channel. A delivery the engine has
queued, is attempting, or holds a backoff timer for is left alone, at start as
on every sweep. This is what catches "orphaned" retries, whose in-memory timer
was dropped because the retry channel was full, and deliveries a stop or a crash
cut off: the database is the durable fallback that ensures no delivery is
permanently lost, even under extreme backpressure.
1. **Startup recovery:** When the engine starts, it scans all per-webhook The run at start and the sweep differ in two ways:
databases for `pending` and `retrying` deliveries. Pending deliveries are
sent to the delivery channel; retrying deliveries get backoff timers - At start every `pending` delivery is taken, however recent, so one a stop cut
scheduled. off moments ago is sent again at once (see Shutdown). The sweep takes only
2. **Periodic retry sweep (DB-mediated fallback):** Every 60 seconds the engine `pending` deliveries untouched for 15 minutes, at most 500 per webhook each
scans for `retrying` deliveries whose backoff period has elapsed. This time, so a delivery whose outcome the database will not record is sent again
catches "orphaned" retries — ones whose in-memory timer was dropped because every 15 minutes rather than every minute.
the retry channel was full. The database is the durable fallback that ensures - At start every webhook's database is opened, so that a missing one is reported
no retry is permanently lost, even under extreme backpressure. then (see Per-Webhook Event Databases). The sweep skips a webhook whose
database file is missing rather than create it.
**Changing a target's type does not migrate in-flight deliveries.** Only `http` **Changing a target's type does not migrate in-flight deliveries.** Only `http`
and `slack` targets own durable retries; `database` and `log` targets are 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 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 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 deliveries is still `retrying`, the recovery routine above terminally marks that
delivery `failed` and record a `DeliveryResult` naming the current target type delivery `failed` and records a `DeliveryResult` naming the current target type
as the reason, logging it at warn level. The delivery is not re-dispatched under 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 the new type — the operator never asked for that delivery — and the event itself
remains stored in the per-webhook event database. Nothing is lost: the delivery remains stored in the per-webhook event database. Nothing is lost: the delivery
@@ -2255,8 +2283,8 @@ re-inject it to every currently active target.
`http` and `slack` targets with `max_attempts` > 1 are protected by a `http` and `slack` targets with `max_attempts` > 1 are protected by a
**per-target circuit breaker** that prevents hammering a down target with **per-target circuit breaker** that prevents hammering a down target with
repeated failed delivery attempts. The circuit breaker is in-memory only and repeated failed delivery attempts. The circuit breaker is in-memory only and
resets on restart (which is fine — startup recovery rescans the database resets on restart (which is fine — the recovery routine rescans the database at
anyway). start anyway).
**States:** **States:**
@@ -3041,6 +3069,7 @@ webhooker/
│ │ └── logfield.go # Encoded-byte budget for client-supplied log values │ │ └── logfield.go # Encoded-byte budget for client-supplied log values
│ ├── delivery/ │ ├── delivery/
│ │ ├── engine.go # Event-driven delivery engine (channel + timer based) │ │ ├── engine.go # Event-driven delivery engine (channel + timer based)
│ │ ├── store.go # Storing an event with its deliveries, and a replay, then queueing them
│ │ ├── circuit_breaker.go # Per-target circuit breaker for http/slack targets with retries │ │ ├── circuit_breaker.go # Per-target circuit breaker for http/slack targets with retries
│ │ ├── target.go # Target interface, Task, Scheduler │ │ ├── target.go # Target interface, Task, Scheduler
│ │ ├── target_http.go # HTTP target (retries, circuit breaker) │ │ ├── target_http.go # HTTP target (retries, circuit breaker)
@@ -3137,23 +3166,26 @@ Components are wired via Uber fx in this order:
10. `metrics.NewRegistry` — The registry `/metrics` serves 10. `metrics.NewRegistry` — The registry `/metrics` serves
11. `metrics.New` — The delivery collectors, registered on that registry 11. `metrics.New` — The delivery collectors, registered on that registry
12. `middleware.New` — HTTP middleware 12. `middleware.New` — HTTP middleware
13. `delivery.New` — Event-driven delivery engine 13. `delivery.New` — Event-driven delivery engine, which also stores each event
with its deliveries and creates replay deliveries
14. `archive.NewWriters` — The database targets' archive writers, and the lock 14. `archive.NewWriters` — The database targets' archive writers, and the lock
every rename of their files runs under every rename of their files runs under
15. `archive.NewSweeper` — Periodic pruning of idle archives 15. `archive.NewSweeper` — Periodic pruning of idle archives
16. `delivery.Engine` → `delivery.Notifier` — interface bridge 16. `archive.Writers` → `archive.Archives` — interface bridge so deleting or
17. `archive.Writers` → `archive.Archives` — interface bridge so deleting or
renaming a webhook or target reaches its archive files renaming a webhook or target reaches its archive files
17. `delivery.Engine` → `delivery.CircuitBreakers` — interface bridge so the
pages can show a target whose deliveries are paused
18. `server.New` — HTTP server and router 18. `server.New` — HTTP server and router
The server starts via The server starts via
`fx.Invoke(func(*server.Server, *delivery.Engine, *database.RetentionReaper, *archive.Sweeper) {})`, `fx.Invoke(func(*server.Server, *delivery.Engine, *database.RetentionReaper, *archive.Sweeper) {})`,
which triggers the fx lifecycle hooks in dependency order. The which triggers the fx lifecycle hooks in dependency order. The handlers take
`delivery.Notifier` interface allows the webhook handler to send self-contained `*delivery.Engine` itself: the receiver and resubmit handlers store an event
`delivery.Task` slices to the engine without a direct package dependency. Each through `StoreEvent`, and the replay handler adds a delivery through
task carries all target settings and event data inline (for bodies under 16 KiB, `CreateReplay`. Each task the engine queues carries the target's settings and
`delivery.MaxInlineBodySize`), so the engine can deliver without reading from the event's data, and the body too when it is under 16 KiB
any database — it only writes to record results. (`delivery.MaxInlineBodySize`); a larger body is read from the event row when
the attempt runs.
### Middleware Stack ### Middleware Stack
@@ -3335,9 +3367,9 @@ normal operation.
The delivery engine cancels a delivery request in flight. That attempt is not 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 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 `retrying` status, so recovery at the next start sends it again (see Recovery
Recovery paths under Bounded Worker Pool). If the receiver already had it, it under Bounded Worker Pool). If the receiver already had it, it receives it
receives it twice: the at-least-once duplicate described under Rationale. 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
@@ -3453,8 +3485,8 @@ fixed independently of the compiler's:
`golangci-lint config verify` and `golangci-lint run`, both with `golangci-lint config verify` and `golangci-lint run`, both with
`--network=none`, and depends on `js-lint` (it copies a file from it). `--network=none`, and depends on `js-lint` (it copies a file from it).
`make lint` builds this stage alone. `make lint` builds this stage alone.
5. **Test phase** (`test`, `golang:1.26.1-bookworm`, whose C compiler `-race` 5. **Test phase** (`test`, `golang:1.26.1-bookworm`, the builder stage's image)
needs) — extracts Alpine.js, then runs `go test -race -cover` at most four — extracts Alpine.js, then runs `go test -cover` with cgo off, at most four
packages and eight tests at a time, with a 90-second timeout per package. On packages and eight tests at a time, with a 90-second timeout per package. On
a failure it runs only the failed tests again with `-v`, since verbose output a failure it runs only the failed tests again with `-v`, since verbose output
from the whole suite would pass the 2 MiB at which the Docker build cuts off from the whole suite would pass the 2 MiB at which the Docker build cuts off
-3
View File
@@ -211,9 +211,6 @@ func newApp() *fx.App {
delivery.New, delivery.New,
archive.NewWriters, archive.NewWriters,
archive.NewSweeper, archive.NewSweeper,
// Wire *delivery.Engine as delivery.Notifier so the
// webhook handler can notify the engine of new deliveries.
func(e *delivery.Engine) delivery.Notifier { return e },
// Wire *archive.Writers as archive.Archives so deleting // Wire *archive.Writers as archive.Archives so deleting
// or renaming a webhook or target reaches its archive // or renaming a webhook or target reaches its archive
// files. // files.
+1 -1
View File
@@ -303,7 +303,7 @@ func (r *racingWrites) wait() (bool, error) {
// contending for the writer's mutex when the eviction takes it. // contending for the writer's mutex when the eviction takes it.
// The write that wins the mutex after the eviction must abandon // The write that wins the mutex after the eviction must abandon
// its work rather than reopen the archive, leaving the writer // its work rather than reopen the archive, leaving the writer
// permanently handle-free. Run under -race. // permanently handle-free.
func TestEvictWebhook_RacingWriteDoesNotReopenHandle( func TestEvictWebhook_RacingWriteDoesNotReopenHandle(
t *testing.T, t *testing.T,
) { ) {
+5 -3
View File
@@ -959,9 +959,11 @@ func TestArchiveSweep_SkipsDeletedWebhookTargets(t *testing.T) {
) )
} }
// TestArchiveSweep_ConcurrentWrites proves the sweep serialises // TestArchiveSweep_ConcurrentWrites runs the sweep while writes to
// against writes through the target's writer mutex. Run under // the same target are under way. The sweep serialises against them
// -race, an unsynchronised sweep would be caught here. // through the target's writer mutex; the suite runs without Go's race
// detector, so a sweep that did not would fail this test only by
// panicking, hanging or leaving no archive file.
func TestArchiveSweep_ConcurrentWrites(t *testing.T) { func TestArchiveSweep_ConcurrentWrites(t *testing.T) {
t.Parallel() t.Parallel()
+3 -2
View File
@@ -79,8 +79,9 @@ func TestEventTierQueriesUseTheirIndexes(t *testing.T) {
byStatus := "idx_deliveries_status (status=? AND deleted_at=?)" byStatus := "idx_deliveries_status (status=? AND deleted_at=?)"
byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)" byEvent := "idx_deliveries_event_id (event_id=? AND deleted_at=?)"
// The delivery engine: recovery and the retry sweep, the sweep for // The delivery engine: the recovery routine, which reads the
// stranded pending deliveries, and the queue depth count. // retrying deliveries and, on a tick of the sweep, the pending ones
// untouched for a while; and the queue depth count.
assertPlanUses(t, db, dry.Where( assertPlanUses(t, db, dry.Where(
"status = ?", database.DeliveryStatusRetrying, "status = ?", database.DeliveryStatusRetrying,
).Find(&deliveries), byStatus) ).Find(&deliveries), byStatus)
+4 -3
View File
@@ -67,9 +67,10 @@ func DefaultPasswordConfig() *PasswordConfig {
// testArgon2Memory is the Argon2id memory cost, in KiB, that a test // testArgon2Memory is the Argon2id memory cost, in KiB, that a test
// binary hashes with: 1 MB instead of the shipped 64 MB. Every test // binary hashes with: 1 MB instead of the shipped 64 MB. Every test
// that starts a database hashes the bootstrap admin password, dozens // that starts a database hashes the bootstrap admin password, dozens
// of them run in parallel, and under the race detector each 64 MB hash // of them run in parallel, and each 64 MB hash holds at least 64 MB
// holds about 150 MB. VerifyPassword reads the cost from the hash it // while it runs (about 150 MB when the suite ran under the race
// checks, so verification follows. // detector). VerifyPassword reads the cost from the hash it checks, so
// verification follows.
const testArgon2Memory = 1024 const testArgon2Memory = 1024
// hashAtShippedCostInTest makes a test binary hash at the shipped // hashAtShippedCostInTest makes a test binary hash at the shipped
+3 -2
View File
@@ -184,8 +184,9 @@ func (r *RetentionReaper) sweep(ctx context.Context) {
wh := webhooks[i] wh := webhooks[i]
// A missing database has nothing to reap. Restart recovery // A missing database has nothing to reap. The delivery
// reports a lost one (see WebhookDBManager.GetDB). // engine's recovery routine reports a lost one at start (see
// WebhookDBManager.GetDB).
if !r.dbManager.DBExists(wh.ID) { if !r.dbManager.DBExists(wh.ID) {
continue continue
} }
+214 -430
View File
@@ -1,5 +1,5 @@
// Package delivery manages asynchronous event delivery // Package delivery stores each event with its deliveries and delivers
// to configured targets. // them, asynchronously, to configured targets.
package delivery package delivery
import ( import (
@@ -23,10 +23,11 @@ import (
const ( const (
// deliveryChannelSize is the buffer size for the delivery // deliveryChannelSize is the buffer size for the delivery
// channel. New Tasks from the webhook handler are sent // channel. The Tasks of newly stored deliveries, and of the
// here. Workers drain this channel. Sized large enough // pending deliveries the recovery routine hands back, are
// that the webhook handler should never block under // sent here. Workers drain this channel. Sized large enough
// normal load. // that no task should be dropped under normal load: one that
// does not fit is left for the sweep.
deliveryChannelSize = 10000 deliveryChannelSize = 10000
// retryChannelSize is the buffer size for the retry // retryChannelSize is the buffer size for the retry
@@ -40,9 +41,10 @@ const (
// explosions regardless of queue depth. // explosions regardless of queue depth.
defaultWorkers = 10 defaultWorkers = 10
// retrySweepInterval is how often the periodic retry // sweepInterval is how often the recovery routine runs
// sweep runs. // once it has run at start. Each of those runs is a tick of
retrySweepInterval = 60 * time.Second // the sweep.
sweepInterval = 60 * time.Second
// pendingSweepMinAge is how long a delivery must have sat // pendingSweepMinAge is how long a delivery must have sat
// untouched at pending before the sweep will look at it. // untouched at pending before the sweep will look at it.
@@ -63,10 +65,10 @@ const (
pendingSweepMinAge = 15 * time.Minute pendingSweepMinAge = 15 * time.Minute
// pendingSweepBatch bounds how many stranded pending deliveries // pendingSweepBatch bounds how many stranded pending deliveries
// one sweep of one webhook re-dispatches. The sweep runs every // one tick of the sweep re-dispatches for one webhook. The sweep
// retrySweepInterval, so a larger backlog drains across // ticks every sweepInterval, so a larger backlog drains
// successive sweeps instead of arriving as one burst against a // across successive ticks instead of arriving as one burst against
// database that was already struggling to accept writes. // a database that was already struggling to accept writes.
pendingSweepBatch = 500 pendingSweepBatch = 500
// MaxInlineBodySize is the maximum event body size that // MaxInlineBodySize is the maximum event body size that
@@ -126,17 +128,11 @@ type Task struct {
AttemptNum int AttemptNum int
} }
// Notifier is the interface for notifying the delivery
// engine about new deliveries.
type Notifier interface {
Notify(tasks []Task)
}
// CircuitBreakers is how the handlers read a target's circuit // CircuitBreakers is how the handlers read a target's circuit
// breaker, so the webhook page and the event log can say that // breaker, so the webhook page and the event log can say that
// deliveries to the target are paused and until when. It keeps the // deliveries to the target are paused and until when. The handlers
// handlers free of the engine's internals and is trivially faked in // hold the engine itself as well; they read breakers through this
// tests. // interface so that a test can make one read as open.
type CircuitBreakers interface { type CircuitBreakers interface {
StateAndCooldown(targetID string) (CircuitState, time.Duration) StateAndCooldown(targetID string) (CircuitState, time.Duration)
} }
@@ -156,12 +152,13 @@ type EngineParams struct {
} }
// Engine processes queued deliveries in the background // Engine processes queued deliveries in the background
// using a bounded worker pool architecture. It owns only // using a bounded worker pool architecture. It also stores each
// the cross-target machinery: the worker pool, the queue // event with its deliveries and creates replay deliveries (see
// and retry channels, restart recovery, and the persistence // store.go). Beyond that it owns only the cross-target machinery:
// helpers and Scheduler that individual targets rely on. // the worker pool, the delivery and retry channels, the recovery
// Each target type owns its own delivery, including retries, // routine, and the persistence helpers and Scheduler that
// backoff, and circuit breaking. // individual targets rely on. Each target type owns its own
// delivery, including retries, backoff, and circuit breaking.
type Engine struct { type Engine struct {
database *database.Database database *database.Database
dbManager *database.WebhookDBManager dbManager *database.WebhookDBManager
@@ -197,7 +194,7 @@ type Engine struct {
archives *archive.Writers archives *archive.Writers
// inflight is the set of deliveries this engine currently owns. // inflight is the set of deliveries this engine currently owns.
// Recovery and the sweeps re-dispatch only what it does not // The recovery routine re-dispatches only what it does not
// hold. Held by value: its zero value works, so no constructor // hold. Held by value: its zero value works, so no constructor
// can leave it out. See inflight.go. // can leave it out. See inflight.go.
inflight inflightSet inflight inflightSet
@@ -231,38 +228,6 @@ func New(
return e return e
} }
// Notify signals the delivery engine that new deliveries
// are ready.
func (e *Engine) Notify(tasks []Task) {
for i := range tasks {
// Owned before it is queued, and until the worker that runs
// it returns. A task can sit in a 10000-deep channel for a
// long time on a healthy system, and nothing may re-send it
// while it waits. See inflight.go.
if !e.inflight.retainIdle(tasks[i].DeliveryID) {
e.log.Warn(
"delivery already in flight, not queued again",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
continue
}
select {
case e.deliveryCh <- tasks[i]:
default:
e.inflight.release(tasks[i].DeliveryID)
e.log.Warn(
"delivery channel full, "+
"task will be recovered by the sweep",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
}
}
}
// StateAndCooldown implements CircuitBreakers. It returns the state of // StateAndCooldown implements CircuitBreakers. It returns the state of
// the target's circuit breaker and, while the breaker is open, what is // the target's circuit breaker and, while the breaker is open, what is
// left of its cooldown; the cooldown is zero once that has passed and // left of its cooldown; the cooldown is zero once that has passed and
@@ -323,6 +288,38 @@ func (e *Engine) ScheduleRetry(
}) })
} }
// notify queues the tasks of deliveries already stored. StoreEvent and
// CreateReplay call it once their transaction has committed.
func (e *Engine) notify(tasks []Task) {
for i := range tasks {
// Owned before it is queued, and until the worker that runs
// it returns. A task can sit in a 10000-deep channel for a
// long time on a healthy system, and nothing may re-send it
// while it waits. See inflight.go.
if !e.inflight.retainIdle(tasks[i].DeliveryID) {
e.log.Warn(
"delivery already in flight, not queued again",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
continue
}
select {
case e.deliveryCh <- tasks[i]:
default:
e.inflight.release(tasks[i].DeliveryID)
e.log.Warn(
"delivery channel full, "+
"task will be recovered by the sweep",
"delivery_id", tasks[i].DeliveryID,
"event_id", tasks[i].EventID,
)
}
}
}
// userAgent is the User-Agent header of every http and slack // userAgent is the User-Agent header of every http and slack
// delivery request: the program name and the running build's // delivery request: the program name and the running build's
// version. // version.
@@ -349,8 +346,8 @@ func (e *Engine) registerHooks(lc fx.Lifecycle) {
}) })
} }
// start launches the worker pool, restart recovery, and the // start launches the worker pool and the recovery routine, which
// periodic retry sweep. // runs now and then on every tick of the sweep (see runRecovery).
// //
// Their context is derived from context.Background(), NOT from // Their context is derived from context.Background(), NOT from
// the fx OnStart hook context. The hook context carries fx's // the fx OnStart hook context. The hook context carries fx's
@@ -373,11 +370,7 @@ func (e *Engine) start() {
e.wg.Add(1) e.wg.Add(1)
go e.recoverPending(ctx) go e.runRecovery(ctx)
e.wg.Add(1)
go e.retrySweep(ctx)
e.wg.Add(1) e.wg.Add(1)
@@ -460,12 +453,6 @@ func (e *Engine) runTask(
run(ctx, task) run(ctx, task)
} }
func (e *Engine) recoverPending(ctx context.Context) {
defer e.wg.Done()
e.recoverInFlight(ctx)
}
func (e *Engine) processNewTask( func (e *Engine) processNewTask(
ctx context.Context, task *Task, ctx context.Context, task *Task,
) { ) {
@@ -480,9 +467,10 @@ func (e *Engine) processNewTask(
return return
} }
// Restart recovery can send and release this delivery before the // The recovery routine's run at start can send and release this
// receiver's Notify queues it. Ownership cannot refuse a delivery // delivery before StoreEvent or CreateReplay queues it. Ownership
// nobody holds, so the row decides whether it still needs sending. // cannot refuse a delivery nobody holds, so the row decides whether
// it still needs sending.
row, err := e.loadDelivery(webhookDB, task.DeliveryID) row, err := e.loadDelivery(webhookDB, task.DeliveryID)
if err != nil { if err != nil {
e.log.Error( e.log.Error(
@@ -612,9 +600,9 @@ func (e *Engine) processRetryTask(
// else on this path reads the target row. Without this check a // else on this path reads the target row. Without this check a
// deletion stops nothing: the timer keeps firing and keeps sending to // deletion stops nothing: the timer keeps firing and keeps sending to
// the destination the operator removed, for the whole remaining // the destination the operator removed, for the whole remaining
// backoff chain. Terminalising in the recovery and sweep paths alone // backoff chain. Terminalising in the recovery routine alone is not
// is not enough, because those only see the delivery once nothing // enough, because it only sees the delivery once nothing holds it in
// holds it in memory — which is to say after a restart. // memory — which is to say after a restart.
// //
// The worker already owns this delivery, so the terminal write happens // The worker already owns this delivery, so the terminal write happens
// here directly, exactly as a target's own Deliver fails one. Claiming // here directly, exactly as a target's own Deliver fails one. Claiming
@@ -662,7 +650,47 @@ func (e *Engine) abandonRetryForMissingTarget(
return true return true
} }
func (e *Engine) recoverInFlight(ctx context.Context) { // runRecovery runs the recovery routine, recoverDeliveries, once when
// the engine starts and then on every tick of the sweep, every
// sweepInterval, until the engine stops.
func (e *Engine) runRecovery(ctx context.Context) {
defer e.wg.Done()
ticker := time.NewTicker(sweepInterval)
defer ticker.Stop()
e.recoverDeliveries(ctx, true)
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
e.recoverDeliveries(ctx, false)
}
}
}
// recoverDeliveries is the engine's one recovery routine: atStart is
// true for its run when the engine starts and false on a tick of the
// sweep.
//
// For every webhook it finds the pending and retrying deliveries the
// engine is not working on, such as those a stop or a crash cut off, a
// full channel dropped, or a failed bookkeeping write left behind. One
// that already has a successful attempt is marked delivered. A retrying
// one is scheduled after what is left of its backoff, or now when
// nothing is left. A pending one goes to a worker now.
//
// Ownership keeps it off a delivery the engine is working on, has
// queued, or holds a retry timer for, the same at start as on a tick:
// every delivery it acts on is taken through the inflight set first.
// See inflight.go.
//
// A run at start differs from a tick in which webhooks it opens (see
// recoverWebhookDeliveries) and which pending deliveries it takes (see
// recoverPendingDeliveries).
func (e *Engine) recoverDeliveries(ctx context.Context, atStart bool) {
var webhookIDs []string var webhookIDs []string
err := e.database.DB(). err := e.database.DB().
@@ -684,18 +712,27 @@ func (e *Engine) recoverInFlight(ctx context.Context) {
default: default:
} }
// Opened even when its file is missing, so that a lost e.recoverWebhookDeliveries(ctx, webhookID, atStart)
// database is reported at start, not when the webhook next
// receives an event, which for a quiet webhook may be never.
e.recoverWebhookDeliveries(ctx, webhookID)
} }
} }
// recoverWebhookDeliveries runs the recovery routine over one webhook.
//
// At start the webhook's database is opened even when its file is
// missing, so that a lost database is reported at start, not when the
// webhook next receives an event, which for a quiet webhook may be
// never. A tick leaves a missing file alone rather than create one: a
// webhook being created has its row saved a moment before its file,
// and a tick in between would report that file as lost.
func (e *Engine) recoverWebhookDeliveries( func (e *Engine) recoverWebhookDeliveries(
ctx context.Context, webhookID string, ctx context.Context, webhookID string, atStart bool,
) { ) {
// The web interface is already serving, so the webhook may have if !atStart && !e.dbManager.DBExists(webhookID) {
// been deleted since the list was read. Opening its database then return
}
// The web interface is serving, so the webhook may have been
// deleted since the list was read. Opening its database then
// would create the file again after the delete removed it. // would create the file again after the delete removed it.
stillExists := func() (bool, error) { stillExists := func() (bool, error) {
var count int64 var count int64
@@ -727,16 +764,18 @@ func (e *Engine) recoverWebhookDeliveries(
} }
e.recoverPendingDeliveries( e.recoverPendingDeliveries(
ctx, webhookDB, webhookID, ctx, webhookDB, webhookID, atStart,
) )
e.recoverRetryingDeliveries( e.recoverRetryingDeliveries(
webhookDB, webhookID, ctx, webhookDB, webhookID,
) )
} }
// recoverRetryingDeliveries recovers every retrying delivery of one
// webhook, at start and on a tick alike.
func (e *Engine) recoverRetryingDeliveries( func (e *Engine) recoverRetryingDeliveries(
webhookDB *gorm.DB, webhookID string, ctx context.Context, webhookDB *gorm.DB, webhookID string,
) { ) {
var retrying []database.Delivery var retrying []database.Delivery
@@ -763,6 +802,12 @@ func (e *Engine) recoverRetryingDeliveries(
) )
for i := range retrying { for i := range retrying {
select {
case <-ctx.Done():
return
default:
}
if _, ok := settled[retrying[i].ID]; ok { if _, ok := settled[retrying[i].ID]; ok {
continue continue
} }
@@ -774,8 +819,9 @@ func (e *Engine) recoverRetryingDeliveries(
} }
// recoverSingleRetry hands an orphaned retrying delivery back // recoverSingleRetry hands an orphaned retrying delivery back
// to its target to recompute the remaining backoff, then // to its target to work out what is left of its backoff, then
// reschedules it. Targets that do not own durable retries // schedules its next attempt after that, or now when nothing is
// left. Targets that do not own durable retries
// (fire-and-forget) never produce retrying deliveries, so a // (fire-and-forget) never produce retrying deliveries, so a
// delivery found in that state has had its target's type // delivery found in that state has had its target's type
// changed underneath it and is terminally failed. // changed underneath it and is terminally failed.
@@ -823,6 +869,19 @@ func (e *Engine) recoverSingleRetry(
webhookDB, d.ID, attemptNum, webhookDB, d.ID, attemptNum,
) )
// Taken before the event is read, so that a delivery the engine
// already holds, most often one waiting out its backoff on a
// timer, costs a tick no read of its body.
if !e.takeForRedispatch(
webhookDB, d.ID, database.DeliveryStatusRetrying,
) {
return
}
// ScheduleRetry takes a reference of its own before it returns,
// and that one keeps the delivery owned through the backoff.
defer e.inflight.release(d.ID)
event, err := e.loadEvent(webhookDB, d.EventID) event, err := e.loadEvent(webhookDB, d.EventID)
if err != nil { if err != nil {
e.log.Error( e.log.Error(
@@ -836,13 +895,9 @@ func (e *Engine) recoverSingleRetry(
return return
} }
task := buildRecoveryTask( e.ScheduleRetry(buildTask(
d, webhookID, &event, &target, attemptNum+1, d, webhookID, &event, &target, attemptNum+1,
) ), remaining)
if !e.rescheduleRecovered(webhookDB, task, remaining) {
return
}
e.log.Info( e.log.Info(
"recovering retrying delivery", "recovering retrying delivery",
@@ -853,28 +908,54 @@ func (e *Engine) recoverSingleRetry(
) )
} }
// recoverPendingDeliveries recovers the pending deliveries of one
// webhook: it marks delivered those that already have a successful
// attempt and sends the rest.
//
// A delivery is created pending and leaves that state only when its
// outcome is written, so a pending row the engine does not own is one
// it stopped working on before then: for example one whose bookkeeping
// write failed, whose attempt a stop cut off, that was still queued
// when the engine last stopped, whose task was dropped because the
// delivery channel was full (by notify or by queueRecovered), or whose
// worker could not open the webhook's database.
//
// At start every pending delivery is taken, however recent, so one a
// stop cut off moments ago is sent again now. A tick takes only those
// untouched for pendingSweepMinAge, at most pendingSweepBatch of them.
// The age bound is not what keeps a tick off live work: a delivery
// waiting in deliveryCh is pending and arbitrarily old. Ownership is,
// checked per delivery in takeForRedispatch. See inflight.go.
func (e *Engine) recoverPendingDeliveries( func (e *Engine) recoverPendingDeliveries(
ctx context.Context, ctx context.Context,
webhookDB *gorm.DB, webhookDB *gorm.DB,
webhookID string, webhookID string,
atStart bool,
) { ) {
query := webhookDB.Where(
"status = ?", database.DeliveryStatusPending,
)
if !atStart {
query = query.
Where(
"updated_at < ?",
time.Now().Add(-pendingSweepMinAge),
).
Limit(pendingSweepBatch)
}
var deliveries []database.Delivery var deliveries []database.Delivery
// No Preload: event bodies are read one at a time in // No Preload: event bodies are read one at a time in
// sendRecoveredDeliveries, and only for the deliveries actually // sendRecoveredDeliveries, and only for the deliveries actually
// being sent. // being sent.
result := webhookDB. err := query.Find(&deliveries).Error
Where( if err != nil {
"status = ?",
database.DeliveryStatusPending,
).
Find(&deliveries)
if result.Error != nil {
e.log.Error( e.log.Error(
"failed to query pending deliveries", "failed to query pending deliveries for recovery",
"webhook_id", webhookID, "webhook_id", webhookID,
"error", result.Error, "error", err,
) )
return return
@@ -890,21 +971,6 @@ func (e *Engine) recoverPendingDeliveries(
"count", len(deliveries), "count", len(deliveries),
) )
e.recoverPendingBatch(
ctx, webhookDB, webhookID, deliveries,
)
}
// recoverPendingBatch settles every delivery in the batch that was
// already delivered, and re-dispatches only the rest. Both the
// restart-time recovery and the periodic sweep go through it, so a
// pending delivery is treated the same however it was found.
func (e *Engine) recoverPendingBatch(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
deliveries []database.Delivery,
) {
targetMap := e.loadTargetMap(deliveries) targetMap := e.loadTargetMap(deliveries)
settled := e.reconcileDelivered( settled := e.reconcileDelivered(
@@ -932,11 +998,11 @@ func (e *Engine) recoverPendingBatch(
// wire I/O happened and was recorded, and all that is missing is the // wire I/O happened and was recorded, and all that is missing is the
// status. // status.
// //
// Every recovery path runs this, not only the pending one. A delivery // The recovery routine runs this for retrying deliveries as well as
// abandoned at retrying can hold a successful result just as a pending // pending ones. A delivery abandoned at retrying can hold a successful
// one can — a second attempt that reached the receiver and whose status // result just as a pending one can — a second attempt that reached the
// write then failed sits at retrying with success recorded — and // receiver and whose status write then failed sits at retrying with
// re-sending it is the same duplicate. // success recorded — and re-sending it is the same duplicate.
// //
// Deliveries whose result row itself never landed are not in the // Deliveries whose result row itself never landed are not in the
// returned set and are re-sent, recorded as the further attempt they // returned set and are re-sent, recorded as the further attempt they
@@ -1023,247 +1089,8 @@ func (e *Engine) reconcileDelivered(
return settled return settled
} }
func (e *Engine) retrySweep(ctx context.Context) {
defer e.wg.Done()
ticker := time.NewTicker(retrySweepInterval)
defer ticker.Stop()
for {
select {
case <-ctx.Done():
return
case <-ticker.C:
e.sweepOrphanedRetries(ctx)
}
}
}
func (e *Engine) sweepOrphanedRetries(
ctx context.Context,
) {
var webhookIDs []string
err := e.database.DB().
Model(&database.Webhook{}).
Pluck("id", &webhookIDs).Error
if err != nil {
e.log.Error(
"retry sweep: failed to query webhook IDs",
"error", err,
)
return
}
for _, webhookID := range webhookIDs {
select {
case <-ctx.Done():
return
default:
}
if !e.dbManager.DBExists(webhookID) {
continue
}
e.sweepWebhookRetries(ctx, webhookID)
}
}
func (e *Engine) sweepWebhookRetries(
ctx context.Context, webhookID string,
) {
webhookDB, err := e.dbManager.GetDB(webhookID)
if err != nil {
e.log.Error(
"retry sweep: failed to get webhook database",
"webhook_id", webhookID,
"error", err,
)
return
}
var retrying []database.Delivery
err = webhookDB.
Where(
"status = ?",
database.DeliveryStatusRetrying,
).
Find(&retrying).Error
if err != nil {
e.log.Error(
"retry sweep: "+
"failed to query retrying deliveries",
"webhook_id", webhookID,
"error", err,
)
return
}
settled := e.reconcileDelivered(
webhookDB, webhookID, retrying,
e.loadTargetMap(retrying),
)
for i := range retrying {
select {
case <-ctx.Done():
return
default:
}
if _, ok := settled[retrying[i].ID]; ok {
continue
}
e.sweepSingleRetry(
webhookDB, webhookID, &retrying[i],
)
}
e.sweepWebhookPending(ctx, webhookDB, webhookID)
}
// sweepWebhookPending recovers deliveries stranded at pending.
//
// A delivery is created pending and leaves that state only when its
// outcome is written, so a pending row the engine does not own is one
// whose bookkeeping write failed — the state that used to sit there
// until a restart, and then produce a duplicate at the receiver. The
// sweep gives it the same reconcile-then-dispatch treatment restart
// recovery gets, so it costs a minute rather than an operator
// noticing.
//
// What keeps the sweep off live work is ownership, checked per
// delivery in takeForRedispatch, not the age bound in this query.
// A delivery waiting in deliveryCh is pending and arbitrarily old —
// the channel holds 10000 tasks and 10 workers drain it — so
// reasoning from the row's age alone re-sends it. See inflight.go.
func (e *Engine) sweepWebhookPending(
ctx context.Context,
webhookDB *gorm.DB,
webhookID string,
) {
var pending []database.Delivery
err := webhookDB.
Where(
"status = ? AND updated_at < ?",
database.DeliveryStatusPending,
time.Now().Add(-pendingSweepMinAge),
).
Limit(pendingSweepBatch).
Find(&pending).Error
if err != nil {
e.log.Error(
"retry sweep: "+
"failed to query pending deliveries",
"webhook_id", webhookID,
"error", err,
)
return
}
if len(pending) == 0 {
return
}
e.log.Info(
"retry sweep: recovering stranded pending deliveries",
"webhook_id", webhookID,
"count", len(pending),
)
e.recoverPendingBatch(ctx, webhookDB, webhookID, pending)
}
// sweepSingleRetry re-enqueues an orphaned retrying delivery
// whose backoff window has elapsed, delegating the backoff
// 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,
d *database.Delivery,
) {
target, err := e.loadTarget(d.TargetID)
if err != nil {
// Deleted is terminal, unreadable is not; see
// recoverSingleRetry.
if errors.Is(err, gorm.ErrRecordNotFound) {
e.failMissingTarget(webhookDB, webhookID, d)
return
}
e.log.Error(
"retry sweep: failed to load target",
"delivery_id", d.ID,
"target_id", d.TargetID,
"error", err,
)
return
}
rs, ok := e.targets[target.Type].(rescheduler)
if !ok {
e.failUnretryableRetry(
webhookDB, webhookID, d, &target,
)
return
}
attemptNum := e.countAttempts(webhookDB, d.ID)
if !rs.backoffElapsed(
webhookDB, d.ID, attemptNum,
) {
return
}
event, err := e.loadEvent(webhookDB, d.EventID)
if err != nil {
e.log.Error(
"retry sweep: failed to load event",
"delivery_id", d.ID,
"event_id", d.EventID,
"error", err,
)
return
}
task := buildRecoveryTask(
d, webhookID, &event, &target, attemptNum+1,
)
if !e.redispatch(
e.retryCh, webhookDB, task,
database.DeliveryStatusRetrying,
) {
return
}
e.log.Info(
"retry sweep: "+
"recovered orphaned retrying delivery",
"delivery_id", d.ID,
"webhook_id", webhookID,
"attempt", attemptNum+1,
)
}
// failUnretryableRetry terminally fails an orphaned retrying // failUnretryableRetry terminally fails an orphaned retrying
// delivery whose target type no longer supports retries. Both // delivery whose target type no longer supports retries.
// 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 // This is only reachable when a target's type has been changed
// out from under an in-flight retrying delivery (or the type is // out from under an in-flight retrying delivery (or the type is
@@ -1311,8 +1138,8 @@ func (e *Engine) failUnretryableRetry(
} }
// failMissingTarget terminally fails a recovered delivery, pending or // failMissingTarget terminally fails a recovered delivery, pending or
// retrying, whose target row is gone. Restart recovery and the periodic // retrying, whose target row is gone. The recovery routine calls it for
// sweep call it for both statuses, so the transition exists once. // both statuses.
// //
// Until it existed those paths logged the failed lookup and moved on, // Until it existed those paths logged the failed lookup and moved on,
// which left the delivery where it was for the life of the database and // which left the delivery where it was for the life of the database and
@@ -1842,11 +1669,11 @@ func (e *Engine) countAttempts(
// answer cannot go stale between asking and acting. // answer cannot go stale between asking and acting.
// //
// Stamping updated_at is the same statement, and it is a cadence // Stamping updated_at is the same statement, and it is a cadence
// control rather than a claim: the pending sweep selects on that // control rather than a claim: a tick of the sweep selects pending
// column, so a delivery handed out now is not selected again on the // deliveries on that column, so one handed out now is not selected
// next tick a minute later but after pendingSweepMinAge. A delivery // again on the next tick a minute later but after pendingSweepMinAge.
// the database refuses to settle is therefore retried on that // A delivery the database refuses to settle is therefore retried on
// interval instead of every tick. // that interval instead of every tick.
// //
// A failed write is a refusal. It means the database is not accepting // A failed write is a refusal. It means the database is not accepting
// writes, which is the condition that stranded the delivery in the // writes, which is the condition that stranded the delivery in the
@@ -1894,14 +1721,11 @@ func (e *Engine) takeForRedispatch(
return true return true
} }
// queueRecovered puts an owned delivery's task on a worker channel, // queueRecovered puts an owned pending delivery's task on the delivery
// dropping the ownership the gate took if it does not fit. // channel, dropping the ownership the gate took if it does not fit.
func (e *Engine) queueRecovered( func (e *Engine) queueRecovered(task Task) {
ch chan<- Task, task Task,
) bool {
select { select {
case ch <- task: case e.deliveryCh <- task:
return true
default: default:
e.inflight.release(task.DeliveryID) e.inflight.release(task.DeliveryID)
e.log.Warn( e.log.Warn(
@@ -1910,53 +1734,9 @@ func (e *Engine) queueRecovered(
"delivery_id", task.DeliveryID, "delivery_id", task.DeliveryID,
"webhook_id", task.WebhookID, "webhook_id", task.WebhookID,
) )
return false
} }
} }
// redispatch hands a recovered delivery to a worker channel through
// takeForRedispatch, and reports whether the task was queued.
func (e *Engine) redispatch(
ch chan<- Task,
webhookDB *gorm.DB,
task Task,
eligible database.DeliveryStatus,
) bool {
if !e.takeForRedispatch(
webhookDB, task.DeliveryID, eligible,
) {
return false
}
return e.queueRecovered(ch, task)
}
// rescheduleRecovered hands an orphaned retrying delivery back to the
// retry timer, through the same gate. It reports whether the delivery
// was rescheduled.
//
// The reference taken by the gate is dropped as soon as ScheduleRetry
// has taken its own, which it does before returning: what keeps the
// delivery owned through the backoff window is ScheduleRetry's
// reference, not this one.
func (e *Engine) rescheduleRecovered(
webhookDB *gorm.DB, task Task, delay time.Duration,
) bool {
if !e.takeForRedispatch(
webhookDB, task.DeliveryID,
database.DeliveryStatusRetrying,
) {
return false
}
defer e.inflight.release(task.DeliveryID)
e.ScheduleRetry(task, delay)
return true
}
// countAttemptsBatch counts the recorded attempts of every delivery // countAttemptsBatch counts the recorded attempts of every delivery
// in a batch with one grouped query, keyed by delivery id. Deliveries // in a batch with one grouped query, keyed by delivery id. Deliveries
// with no attempts are simply absent from the result, which reads back // with no attempts are simply absent from the result, which reads back
@@ -2038,7 +1818,11 @@ func (e *Engine) loadTarget(
return target, nil return target, nil
} }
func buildRecoveryTask( // buildTask builds the task that carries delivery d of event to target
// through the engine's channels, as attempt attemptNum. The body is
// carried only when it is under MaxInlineBodySize; a larger one is read
// from the event row when the attempt runs (see hydrateEvent).
func buildTask(
d *database.Delivery, d *database.Delivery,
webhookID string, webhookID string,
event *database.Event, event *database.Event,
@@ -2214,11 +1998,11 @@ func (e *Engine) sendRecoveredDeliveries(
continue continue
} }
task := buildRecoveryTask( task := buildTask(
&deliveries[i], webhookID, &event, &target, &deliveries[i], webhookID, &event, &target,
attempts[deliveries[i].ID]+1, attempts[deliveries[i].ID]+1,
) )
e.queueRecovered(e.deliveryCh, task) e.queueRecovered(task)
} }
} }
+54 -44
View File
@@ -492,7 +492,7 @@ func TestWorkerLifecycle_StartStop(t *testing.T) {
) )
task.TargetType = database.TargetTypeLog task.TargetType = database.TargetTypeLog
s.Engine.Notify([]delivery.Task{task}) s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, d.ID) iWaitForDelivered(t, s.WebhookDB, d.ID)
@@ -640,7 +640,7 @@ func TestRecoverPendingDeliveries(t *testing.T) {
s.Engine.ExportRecoverPendingDeliveries( s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, context.Background(), s.WebhookDB,
s.WebhookID, s.WebhookID, atStart,
) )
for i := range 3 { for i := range 3 {
@@ -737,7 +737,7 @@ func TestRecoverWebhookDeliveries_RetryingDeliveries(
) )
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
select { select {
@@ -861,7 +861,7 @@ func TestRecoverSingleRetry_TypeNoLongerRetries(
) )
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
iAssertTerminallyFailed( iAssertTerminallyFailed(
@@ -869,7 +869,7 @@ func TestRecoverSingleRetry_TypeNoLongerRetries(
) )
} }
func TestSweepSingleRetry_TypeNoLongerRetries( func TestRecoverSingleRetry_OnTick_TypeNoLongerRetries(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -884,8 +884,8 @@ func TestSweepSingleRetry_TypeNoLongerRetries(
t, s, database.TargetTypeDatabase, t, s, database.TargetTypeDatabase,
) )
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertTerminallyFailed( iAssertTerminallyFailed(
@@ -936,8 +936,8 @@ func TestFailUnretryableRetry_WritesNoTargetRow(
iSeedFailedResult(t, s.WebhookDB, d.ID) iSeedFailedResult(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertStatus(t, s.WebhookDB, d.ID, iAssertStatus(t, s.WebhookDB, d.ID,
@@ -968,13 +968,13 @@ func TestRecoverSingleRetry_UnknownTargetType(
deliveryID := iSeedRetryingWithType(t, s, unknown) deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
iAssertTerminallyFailed(t, s, deliveryID, unknown) iAssertTerminallyFailed(t, s, deliveryID, unknown)
} }
func TestSweepSingleRetry_UnknownTargetType( func TestRecoverSingleRetry_OnTick_UnknownTargetType(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -989,8 +989,8 @@ func TestSweepSingleRetry_UnknownTargetType(
deliveryID := iSeedRetryingWithType(t, s, unknown) deliveryID := iSeedRetryingWithType(t, s, unknown)
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertTerminallyFailed(t, s, deliveryID, unknown) iAssertTerminallyFailed(t, s, deliveryID, unknown)
@@ -1032,19 +1032,19 @@ func iCreateWebhook(
require.NoError(t, mainDB.Create(&webhook).Error) require.NoError(t, mainDB.Create(&webhook).Error)
} }
// --- recoverInFlight Tests --- // --- recoverDeliveries Tests ---
func TestRecoverInFlight_NoWebhooks(t *testing.T) { func TestRecoverDeliveries_NoWebhooks(t *testing.T) {
t.Parallel() t.Parallel()
s := newISetup(t) s := newISetup(t)
s.Engine.ExportRecoverInFlight( s.Engine.ExportRecoverDeliveries(
context.Background(), context.Background(), atStart,
) )
} }
func TestRecoverInFlight_WithPendingDeliveries( func TestRecoverDeliveries_WithPendingDeliveries(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -1071,8 +1071,8 @@ func TestRecoverInFlight_WithPendingDeliveries(
database.DeliveryStatusPending, database.DeliveryStatusPending,
) )
s.Engine.ExportRecoverInFlight( s.Engine.ExportRecoverDeliveries(
context.Background(), context.Background(), atStart,
) )
select { select {
@@ -1083,16 +1083,16 @@ func TestRecoverInFlight_WithPendingDeliveries(
database.TargetTypeLog, task.TargetType, database.TargetTypeLog, task.TargetType,
) )
case <-time.After(2 * time.Second): case <-time.After(2 * time.Second):
t.Fatal("expected task from recoverInFlight") t.Fatal("expected task from recovery at start")
} }
} }
// TestRecoverInFlight_ReportsAMissingWebhookDatabase covers a webhook // TestRecoverDeliveries_ReportsAMissingWebhookDatabase covers a webhook
// whose database file is gone, after a partial restore say. Restart // whose database file is gone, after a partial restore say. Recovery at
// recovery opens every webhook's database, so the empty one made in its // start opens every webhook's database, so the empty one made in its
// place is reported at start, naming the file // place is reported at start, naming the file
// (https://git.eeqj.de/sneak/webhooker/issues/290). // (https://git.eeqj.de/sneak/webhooker/issues/290).
func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) { func TestRecoverDeliveries_ReportsAMissingWebhookDatabase(t *testing.T) {
t.Parallel() t.Parallel()
mainDB := iMainDB(t) mainDB := iMainDB(t)
@@ -1112,7 +1112,7 @@ func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
&http.Client{Timeout: 5 * time.Second}, 1, &http.Client{Timeout: 5 * time.Second}, 1,
) )
engine.ExportRecoverInFlight(context.Background()) engine.ExportRecoverDeliveries(context.Background(), atStart)
assert.Contains( assert.Contains(
t, logs.String(), t, logs.String(),
@@ -1121,10 +1121,11 @@ func TestRecoverInFlight_ReportsAMissingWebhookDatabase(t *testing.T) {
) )
} }
// TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead covers a // TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead covers a
// webhook deleted from the web interface while restart recovery runs. // webhook deleted from the web interface while the recovery routine
// Its database file is gone, and recovery must not create it again. // runs. Its database file is gone, and recovery must not create it
func TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead( // again.
func TestRecoverDeliveries_SkipsAWebhookDeletedAfterTheListIsRead(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -1160,7 +1161,7 @@ func TestRecoverInFlight_SkipsAWebhookDeletedAfterTheListIsRead(
&http.Client{Timeout: 5 * time.Second}, 1, &http.Client{Timeout: 5 * time.Second}, 1,
) )
engine.ExportRecoverInFlight(context.Background()) engine.ExportRecoverDeliveries(context.Background(), atStart)
require.True(t, deleted) require.True(t, deleted)
assert.False(t, dbMgr.DBExists(webhookID)) assert.False(t, dbMgr.DBExists(webhookID))
@@ -1220,11 +1221,14 @@ func TestDeliverHTTP_CustomTargetHeaders(t *testing.T) {
// --- HTTP delivery with custom timeout --- // --- HTTP delivery with custom timeout ---
// TestDeliverHTTP_TargetTimeout proves a task's TimeoutSeconds reaches
// the delivery through the engine. The destination answers after two
// seconds, inside the engine's own five-second client timeout, so only
// the task's one second can fail the delivery.
func TestDeliverHTTP_TargetTimeout(t *testing.T) { func TestDeliverHTTP_TargetTimeout(t *testing.T) {
t.Parallel() t.Parallel()
db := testWebhookDB(t) s := newISetup(t)
e := iEngine(t, 1)
ts := httptest.NewServer( ts := httptest.NewServer(
http.HandlerFunc( http.HandlerFunc(
@@ -1236,24 +1240,30 @@ func TestDeliverHTTP_TargetTimeout(t *testing.T) {
) )
defer ts.Close() defer ts.Close()
event, del := iSeedEventAndDelivery( event := iSeedEvent(
t, db, `{"timeout":"test"}`, t, s.WebhookDB, s.WebhookID, `{"timeout":"test"}`,
)
targetID := uuid.New().String()
d := iSeedDelivery(
t, s.WebhookDB, event.ID, targetID,
database.DeliveryStatusPending,
) )
task, d := iHTTPTaskAndDelivery( bodyStr := event.Body
event, del, "timeout-target", task := iTask(
ts.URL, 1, 1, d, event, s.WebhookID, targetID,
"timeout-target", ts.URL, 1, 1, &bodyStr,
) )
task.TimeoutSeconds = 1 task.TimeoutSeconds = 1
d.Target.TimeoutSeconds = 1
e.ExportDeliverHTTP(context.TODO(), db, d, task) s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(t, db, del.ID, iAssertStatus(t, s.WebhookDB, d.ID,
database.DeliveryStatusFailed, database.DeliveryStatusFailed,
) )
iAssertResultFailed(t, db, del.ID) iAssertResultFailed(t, s.WebhookDB, d.ID)
} }
// TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size // TestDeliverHTTP_CutsStoredResponseAtMaxBodyLog pins the size
@@ -1442,7 +1452,7 @@ func TestDeliverHTTP_NoURL(t *testing.T) {
iAssertResultFailed(t, db, del.ID) iAssertResultFailed(t, db, del.ID)
} }
// --- Notify batching --- // --- notify batching ---
func TestNotify_MultipleTasks(t *testing.T) { func TestNotify_MultipleTasks(t *testing.T) {
t.Parallel() t.Parallel()
@@ -1457,7 +1467,7 @@ func TestNotify_MultipleTasks(t *testing.T) {
} }
} }
e.Notify(tasks) e.ExportNotify(tasks)
for i := range 5 { for i := range 5 {
select { select {
+17 -16
View File
@@ -29,7 +29,7 @@ const (
// OnStart before the caller may enqueue work. A worker pool // OnStart before the caller may enqueue work. A worker pool
// wrongly rooted in the already-done hook context has nothing // wrongly rooted in the already-done hook context has nothing
// but ctx.Done() ready in its select, so it is deterministically // but ctx.Done() ready in its select, so it is deterministically
// gone by the end of this window. Without the wait, Notify would // gone by the end of this window. Without the wait, notify would
// race the pool's very first select, in which a ready ctx.Done() // race the pool's very first select, in which a ready ctx.Done()
// and a ready deliveryCh are chosen between at random and a // and a ready deliveryCh are chosen between at random and a
// doomed pool still delivers. // doomed pool still delivers.
@@ -99,7 +99,7 @@ func requireStopHookExpires(
// the caller can drive OnStop too. // the caller can drive OnStop too.
// //
// Callers must not seed pending or retrying deliveries before // Callers must not seed pending or retrying deliveries before
// calling this: restart recovery enqueues those during startup, // calling this: the recovery routine's run at start enqueues those,
// which would put work in the queue while the pool is still // which would put work in the queue while the pool is still
// racing its first select. // racing its first select.
func startEngineViaHook( func startEngineViaHook(
@@ -178,11 +178,11 @@ func TestEngine_WorkersOutliveStartHookContext(t *testing.T) {
_ = lc.hooks[0].OnStop(context.Background()) _ = lc.hooks[0].OnStop(context.Background())
}) })
// Seeded only after the pool has settled, so restart recovery // Seeded only after the pool has settled, so the recovery
// cannot enqueue it during startup. // routine's run at start cannot enqueue it.
deliveryID, task := seedLogTask(t, s) deliveryID, task := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{task}) s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID) iWaitForDelivered(t, s.WebhookDB, deliveryID)
} }
@@ -202,7 +202,7 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
// Let the pool prove it is running before stopping it, so a // Let the pool prove it is running before stopping it, so a
// fast OnStop cannot pass by stopping something already dead. // fast OnStop cannot pass by stopping something already dead.
firstID, firstTask := seedLogTask(t, s) firstID, firstTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{firstTask}) s.Engine.ExportNotify([]delivery.Task{firstTask})
iWaitForDelivered(t, s.WebhookDB, firstID) iWaitForDelivered(t, s.WebhookDB, firstID)
var stopErr error var stopErr error
@@ -232,7 +232,7 @@ func TestEngine_StopHookStopsWorkers(t *testing.T) {
// With every worker gone, a freshly notified task must sit // With every worker gone, a freshly notified task must sit
// untouched in the queue rather than being delivered. // untouched in the queue rather than being delivered.
secondID, secondTask := seedLogTask(t, s) secondID, secondTask := seedLogTask(t, s)
s.Engine.Notify([]delivery.Task{secondTask}) s.Engine.ExportNotify([]delivery.Task{secondTask})
time.Sleep(200 * time.Millisecond) time.Sleep(200 * time.Millisecond)
@@ -344,15 +344,15 @@ func stopDuringAttempt(
require.NoError(t, lc.hooks[0].OnStop(context.Background())) require.NoError(t, lc.hooks[0].OnStop(context.Background()))
} }
// requireRequeued runs restart recovery on the stopped engine and // requireRequeued runs the recovery routine as at start on the stopped
// requires it to queue task's attempt again on ch. // engine and requires it to queue task's attempt again on ch.
func requireRequeued( func requireRequeued(
t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task, t *testing.T, s iSetup, ch <-chan delivery.Task, task delivery.Task,
) { ) {
t.Helper() t.Helper()
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
select { select {
@@ -360,7 +360,7 @@ func requireRequeued(
assert.Equal(t, task.DeliveryID, got.DeliveryID) assert.Equal(t, task.DeliveryID, got.DeliveryID)
assert.Equal(t, task.AttemptNum, got.AttemptNum) assert.Equal(t, task.AttemptNum, got.AttemptNum)
case <-time.After(5 * time.Second): case <-time.After(5 * time.Second):
t.Fatal("restart recovery did not queue the delivery again") t.Fatal("recovery at start did not queue the delivery again")
} }
} }
@@ -370,7 +370,8 @@ func requireRequeued(
// no retries that was the delivery's only attempt, recorded as failed // no retries that was the delivery's only attempt, recorded as failed
// with "context canceled", and nothing ever sent it again. The // with "context canceled", and nothing ever sent it again. The
// delivery must instead stay pending, with no result and no count in // delivery must instead stay pending, with no result and no count in
// the attempt metrics, for restart recovery to send. // the attempt metrics, for the recovery routine to send at the next
// start.
func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) { func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
t.Parallel() t.Parallel()
@@ -382,7 +383,7 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
t, s, 1, database.DeliveryStatusPending, 1, t, s, 1, database.DeliveryStatusPending, 1,
) )
s.Engine.Notify([]delivery.Task{task}) s.Engine.ExportNotify([]delivery.Task{task})
stopDuringAttempt(t, lc, arrived) stopDuringAttempt(t, lc, arrived)
iAssertStatus( iAssertStatus(
@@ -398,8 +399,8 @@ func TestEngine_StopLeavesFireAndForgetAttemptPending(t *testing.T) {
// TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression // TestEngine_StopLeavesLastRetryAttemptRetrying is the same regression
// for a target with retries, on its last attempt: the cancelled attempt // 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 // used to exhaust the retries and end the delivery failed. It must stay
// retrying with only its earlier attempt recorded, and restart recovery // retrying with only its earlier attempt recorded, and the recovery
// must send that last attempt again. // routine at the next start must send that last attempt again.
func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) { func TestEngine_StopLeavesLastRetryAttemptRetrying(t *testing.T) {
t.Parallel() t.Parallel()
@@ -447,7 +448,7 @@ func deliverToArchive(t *testing.T, s iSetup) string {
task.TargetID = tgt.ID task.TargetID = tgt.ID
task.TargetType = database.TargetTypeDatabase task.TargetType = database.TargetTypeDatabase
s.Engine.Notify([]delivery.Task{task}) s.Engine.ExportNotify([]delivery.Task{task})
iWaitForDelivered(t, s.WebhookDB, deliveryID) iWaitForDelivered(t, s.WebhookDB, deliveryID)
+2 -2
View File
@@ -218,7 +218,7 @@ func TestNotify_NonBlocking(t *testing.T) {
done := make(chan struct{}) done := make(chan struct{})
go func() { go func() {
e.Notify([]delivery.Task{ e.ExportNotify([]delivery.Task{
{DeliveryID: "overflow-1"}, {DeliveryID: "overflow-1"},
{DeliveryID: "overflow-2"}, {DeliveryID: "overflow-2"},
}) })
@@ -229,7 +229,7 @@ func TestNotify_NonBlocking(t *testing.T) {
case <-done: case <-done:
case <-time.After(2 * time.Second): case <-time.After(2 * time.Second):
t.Fatal( t.Fatal(
"Notify blocked when delivery channel was full", "notify blocked when delivery channel was full",
) )
} }
} }
+16 -16
View File
@@ -158,6 +158,11 @@ func (e *Engine) ExportUpdateDeliveryStatus(
return e.updateDeliveryStatus(webhookDB, d, "", status) return e.updateDeliveryStatus(webhookDB, d, "", status)
} }
// ExportNotify exposes notify.
func (e *Engine) ExportNotify(tasks []Task) {
e.notify(tasks)
}
// ExportProcessNewTask exposes processNewTask. // ExportProcessNewTask exposes processNewTask.
func (e *Engine) ExportProcessNewTask( func (e *Engine) ExportProcessNewTask(
ctx context.Context, task *Task, ctx context.Context, task *Task,
@@ -267,32 +272,27 @@ func (e *Engine) ExportRecoverPendingDeliveries(
ctx context.Context, ctx context.Context,
webhookDB *gorm.DB, webhookDB *gorm.DB,
webhookID string, webhookID string,
atStart bool,
) { ) {
e.recoverPendingDeliveries( e.recoverPendingDeliveries(
ctx, webhookDB, webhookID, ctx, webhookDB, webhookID, atStart,
) )
} }
// ExportRecoverWebhookDeliveries exposes // ExportRecoverWebhookDeliveries exposes
// recoverWebhookDeliveries. // recoverWebhookDeliveries.
func (e *Engine) ExportRecoverWebhookDeliveries( func (e *Engine) ExportRecoverWebhookDeliveries(
ctx context.Context, webhookID string, ctx context.Context, webhookID string, atStart bool,
) { ) {
e.recoverWebhookDeliveries(ctx, webhookID) e.recoverWebhookDeliveries(ctx, webhookID, atStart)
} }
// ExportRecoverInFlight exposes recoverInFlight. // ExportRecoverDeliveries exposes recoverDeliveries, the recovery
func (e *Engine) ExportRecoverInFlight( // routine.
ctx context.Context, func (e *Engine) ExportRecoverDeliveries(
ctx context.Context, atStart bool,
) { ) {
e.recoverInFlight(ctx) e.recoverDeliveries(ctx, atStart)
}
// ExportSweepWebhookRetries exposes sweepWebhookRetries.
func (e *Engine) ExportSweepWebhookRetries(
ctx context.Context, webhookID string,
) {
e.sweepWebhookRetries(ctx, webhookID)
} }
// ExportStart exposes start for testing. // ExportStart exposes start for testing.
@@ -339,9 +339,9 @@ func (e *Engine) ExportRetainDelivery(deliveryID string) bool {
// ExportRecoverRetryingDeliveries exposes recoverRetryingDeliveries. // ExportRecoverRetryingDeliveries exposes recoverRetryingDeliveries.
func (e *Engine) ExportRecoverRetryingDeliveries( func (e *Engine) ExportRecoverRetryingDeliveries(
webhookDB *gorm.DB, webhookID string, ctx context.Context, webhookDB *gorm.DB, webhookID string,
) { ) {
e.recoverRetryingDeliveries(webhookDB, webhookID) e.recoverRetryingDeliveries(ctx, webhookDB, webhookID)
} }
// ExportFailMissingTarget exposes failMissingTarget, so a test can hand // ExportFailMissingTarget exposes failMissingTarget, so a test can hand
+9 -9
View File
@@ -6,10 +6,10 @@ import "sync"
// //
// A delivery is owned from the moment a task for it is handed to a // A delivery is owned from the moment a task for it is handed to a
// channel or to a retry timer until the engine has no further plan for // channel or to a retry timer until the engine has no further plan for
// it in memory. Restart recovery and both arms of the periodic sweep // it in memory. The recovery routine, at start and on every tick of the
// re-dispatch only deliveries the set does not hold, which is what // sweep, re-dispatches only deliveries the set does not hold, which is
// makes them exact rather than a guess about how long a row has sat at // what makes it exact rather than a guess about how long a row has sat
// pending. // at pending.
// //
// This replaces reasoning from timestamps. A delivery's row says // This replaces reasoning from timestamps. A delivery's row says
// pending from creation until its outcome is written, which covers // pending from creation until its outcome is written, which covers
@@ -25,8 +25,8 @@ import "sync"
// In-memory state is sufficient because a data directory admits one // In-memory state is sufficient because a data directory admits one
// process: internal/datadir takes an flock on it at startup and a // process: internal/datadir takes an flock on it at startup and a
// second instance refuses to run. Deliveries owned by a process that // second instance refuses to run. Deliveries owned by a process that
// died are not in any successor's set, and restart recovery is what // died are not in any successor's set, and the recovery routine's run
// picks those up. // at start is what picks those up.
// //
// References are counted rather than held as a plain set because // References are counted rather than held as a plain set because
// ownership outlives the worker that took it. A target that schedules // ownership outlives the worker that took it. A target that schedules
@@ -61,9 +61,9 @@ func (s *inflightSet) retain(deliveryID string) {
// retainIdle takes the first reference on a delivery, and reports // retainIdle takes the first reference on a delivery, and reports
// whether it got it. It fails when the engine already owns the // whether it got it. It fails when the engine already owns the
// delivery, which is what makes two claimants — restart recovery and // delivery, which is what makes two claimants — notify and the
// the sweep run concurrently, or two sweep arms — mutually exclusive // recovery routine, say — mutually exclusive rather than merely
// rather than merely atomic. // atomic.
func (s *inflightSet) retainIdle(deliveryID string) bool { func (s *inflightSet) retainIdle(deliveryID string) bool {
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock() defer s.mu.Unlock()
+30 -26
View File
@@ -84,9 +84,9 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
) )
rAgePending(t, s.WebhookDB, d.ID) rAgePending(t, s.WebhookDB, d.ID)
// Queued exactly as the receiver queues it, and never dequeued: // Queued exactly as StoreEvent queues it, and never dequeued:
// no workers are running in this engine. // no workers are running in this engine.
s.Engine.Notify([]delivery.Task{{ s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID, DeliveryID: d.ID,
EventID: event.ID, EventID: event.ID,
WebhookID: s.WebhookID, WebhookID: s.WebhookID,
@@ -95,8 +95,8 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
require.Equal(t, 1, s.Engine.ExportInflightHeld()) require.Equal(t, 1, s.Engine.ExportInflightHeld())
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
tasks := fDrain(s.Engine) tasks := fDrain(s.Engine)
@@ -107,9 +107,11 @@ func TestSweepLeavesAQueuedDeliveryAlone(t *testing.T) {
) )
} }
// TestRecoveryAndSweepDoNotDoubleDispatch drives the two entry points // TestRecoveryAndSweepDoNotDoubleDispatch drives the recovery routine's
// the engine starts concurrently against one aged pending row. Before // run at start and a tick of the sweep at once against one aged pending
// ownership they both dispatched it. // row. The engine runs them one after the other, but notify can race
// either, and ownership is what keeps any two claimants from both
// sending. Before ownership the two runs both dispatched the row.
func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) { func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) {
t.Parallel() t.Parallel()
@@ -133,12 +135,12 @@ func TestRecoveryAndSweepDoNotDoubleDispatch(t *testing.T) {
wg.Go(func() { wg.Go(func() {
s.Engine.ExportRecoverPendingDeliveries( s.Engine.ExportRecoverPendingDeliveries(
ctx, s.WebhookDB, s.WebhookID, ctx, s.WebhookDB, s.WebhookID, atStart,
) )
}) })
wg.Go(func() { wg.Go(func() {
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
ctx, s.WebhookID, ctx, s.WebhookID, onTick,
) )
}) })
wg.Wait() wg.Wait()
@@ -222,7 +224,7 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
body := `{"released":true}` body := `{"released":true}`
s.Engine.Notify([]delivery.Task{{ s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID, DeliveryID: d.ID,
EventID: event.ID, EventID: event.ID,
WebhookID: s.WebhookID, WebhookID: s.WebhookID,
@@ -246,10 +248,11 @@ func TestOwnershipIsReleasedAfterDelivery(t *testing.T) {
} }
// TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of // TestNotifyAfterRecoveryDoesNotSendAgain is the startup race of
// https://git.eeqj.de/sneak/webhooker/issues/299. The receiver has // https://git.eeqj.de/sneak/webhooker/issues/299. StoreEvent has
// written a delivery, restart recovery finds it pending, sends it and // written a delivery, the recovery routine's run at start finds it
// releases it, and only then does the receiver's Notify for it arrive. // pending, sends it and releases it, and only then does StoreEvent's
// Nothing owns the delivery by then, so Notify takes it. // notify for it arrive. Nothing owns the delivery by then, so notify
// takes it.
func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) { func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
t.Parallel() t.Parallel()
@@ -273,7 +276,8 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
) )
}() }()
// Restart recovery sends the delivery and lets it go. // The recovery routine's run at start sends the delivery and lets
// it go.
iWaitForDelivered(t, s.WebhookDB, d.ID) iWaitForDelivered(t, s.WebhookDB, d.ID)
require.Eventually( require.Eventually(
t, t,
@@ -285,7 +289,7 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
body := event.Body body := event.Body
s.Engine.Notify([]delivery.Task{{ s.Engine.ExportNotify([]delivery.Task{{
DeliveryID: d.ID, DeliveryID: d.ID,
EventID: event.ID, EventID: event.ID,
WebhookID: s.WebhookID, WebhookID: s.WebhookID,
@@ -296,7 +300,7 @@ func TestNotifyAfterRecoveryDoesNotSendAgain(t *testing.T) {
EntrypointID: event.EntrypointID, EntrypointID: event.EntrypointID,
}}) }})
// Notify took the delivery, and a worker releases it once it has // notify took the delivery, and a worker releases it once it has
// run the task. // run the task.
require.Eventually( require.Eventually(
t, t,
@@ -334,7 +338,7 @@ func TestRetryingRecoverySkipsASuccessfulResult(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 2, true) rSeedResult(t, s.WebhookDB, d.ID, 2, true)
s.Engine.ExportRecoverRetryingDeliveries( s.Engine.ExportRecoverRetryingDeliveries(
s.WebhookDB, s.WebhookID, context.Background(), s.WebhookDB, s.WebhookID,
) )
assert.Empty( assert.Empty(
@@ -349,8 +353,8 @@ func TestRetryingRecoverySkipsASuccessfulResult(t *testing.T) {
) )
} }
// TestRetryingSweepSkipsASuccessfulResult is the same rule on the // TestRetryingSweepSkipsASuccessfulResult is the same rule on a tick of
// periodic sweep's retrying arm. // the sweep.
func TestRetryingSweepSkipsASuccessfulResult(t *testing.T) { func TestRetryingSweepSkipsASuccessfulResult(t *testing.T) {
t.Parallel() t.Parallel()
@@ -368,8 +372,8 @@ func TestRetryingSweepSkipsASuccessfulResult(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 1, false) rSeedResult(t, s.WebhookDB, d.ID, 1, false)
rSeedResult(t, s.WebhookDB, d.ID, 2, true) rSeedResult(t, s.WebhookDB, d.ID, 2, true)
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
assert.Empty(t, fDrain(s.Engine)) assert.Empty(t, fDrain(s.Engine))
@@ -420,8 +424,8 @@ func TestScheduledRetryIsNotSweptDuringBackoff(t *testing.T) {
require.Equal(t, 1, s.Engine.ExportInflightHeld()) require.Equal(t, 1, s.Engine.ExportInflightHeld())
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
assert.Empty( assert.Empty(
@@ -453,7 +457,7 @@ func TestRedispatchStampsTheRow(t *testing.T) {
ctx := context.Background() ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID) s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
require.Len(t, fDrain(s.Engine), 1) require.Len(t, fDrain(s.Engine), 1)
var row database.Delivery var row database.Delivery
+2 -2
View File
@@ -441,8 +441,8 @@ func TestDeliveryMetrics_OrphanedRetryFailureLabelled(
t, s, database.TargetTypeLog, t, s, database.TargetTypeLog,
) )
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertStatus(t, s.WebhookDB, deliveryID, iAssertStatus(t, s.WebhookDB, deliveryID,
+2 -1
View File
@@ -20,7 +20,8 @@ const queueDepthSampleInterval = 30 * time.Second
// to be seeded correctly at startup from rows written by a previous // to be seeded correctly at startup from rows written by a previous
// process, and would drift permanently on any transition that failed // process, and would drift permanently on any transition that failed
// to persist. Counting is the measurement that cannot go wrong, and // to persist. Counting is the measurement that cannot go wrong, and
// it is the same whole-database walk the retry sweep already makes. // it is the same whole-database walk each tick of the sweep already
// makes.
func (e *Engine) queueDepthSampler(ctx context.Context) { func (e *Engine) queueDepthSampler(ctx context.Context) {
defer e.wg.Done() defer e.wg.Done()
+14 -13
View File
@@ -88,7 +88,7 @@ func TestRecoverySkipsPendingWithSuccessfulResult(
) )
s.Engine.ExportRecoverPendingDeliveries( s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID, context.Background(), s.WebhookDB, s.WebhookID, atStart,
) )
select { select {
@@ -145,7 +145,7 @@ func TestRecoveryContinuesTheAttemptNumbering(t *testing.T) {
rSeedResult(t, s.WebhookDB, d.ID, 2, false) rSeedResult(t, s.WebhookDB, d.ID, 2, false)
s.Engine.ExportRecoverPendingDeliveries( s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID, context.Background(), s.WebhookDB, s.WebhookID, atStart,
) )
select { select {
@@ -183,8 +183,8 @@ func TestSweepRecoversStrandedPending(t *testing.T) {
database.DeliveryStatusPending, database.DeliveryStatusPending,
) )
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
select { select {
@@ -230,7 +230,7 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
ctx := context.Background() ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID) s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
select { select {
case task := <-s.Engine.ExportDeliveryCh(): case task := <-s.Engine.ExportDeliveryCh():
@@ -246,7 +246,7 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
database.DeliveryStatusPending, database.DeliveryStatusPending,
) )
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID) s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
select { select {
case task := <-s.Engine.ExportDeliveryCh(): case task := <-s.Engine.ExportDeliveryCh():
@@ -258,9 +258,9 @@ func TestSweepClaimsAStrandedDeliveryOnlyOnce(t *testing.T) {
} }
} }
// TestSweepSettlesStrandedPendingWithoutResending is the sweep's own // TestSweepSettlesStrandedPendingWithoutResending is the reconcile on a
// version of the reconcile: a stranded delivery holding a successful // tick of the sweep: a stranded delivery holding a successful result is
// result is settled where it stands, and the receiver hears nothing. // settled where it stands, and the receiver hears nothing.
func TestSweepSettlesStrandedPendingWithoutResending( func TestSweepSettlesStrandedPendingWithoutResending(
t *testing.T, t *testing.T,
) { ) {
@@ -280,8 +280,8 @@ func TestSweepSettlesStrandedPendingWithoutResending(
rSeedResult(t, s.WebhookDB, d.ID, 1, true) rSeedResult(t, s.WebhookDB, d.ID, 1, true)
rAgePending(t, s.WebhookDB, d.ID) rAgePending(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
select { select {
@@ -312,8 +312,9 @@ func TestSweepSettlesStrandedPendingWithoutResending(
// TestFailedResultWriteLeavesDeliveryRecoverable is the rule the // TestFailedResultWriteLeavesDeliveryRecoverable is the rule the
// targets now follow: a bookkeeping write that fails must not advance // targets now follow: a bookkeeping write that fails must not advance
// the status, because pending and retrying are the states the sweeps // the status, because pending and retrying are the states the recovery
// recover and delivered is a claim the database refused to record. // routine recovers and delivered is a claim the database refused to
// record.
func TestFailedResultWriteLeavesDeliveryRecoverable( func TestFailedResultWriteLeavesDeliveryRecoverable(
t *testing.T, t *testing.T,
) { ) {
+389
View File
@@ -0,0 +1,389 @@
package delivery_test
import (
"context"
"log/slog"
"net/http"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/database/databasetest"
"sneak.berlin/go/webhooker/internal/delivery"
)
// These tests cover the recovery routine, which the engine runs once
// when it starts and then on every tick of the sweep
// (https://git.eeqj.de/sneak/webhooker/issues/542). Every case runs it
// both ways, each on a webhook of its own.
// atStart and onTick are the two values of the recovery routine's
// atStart parameter: its run when the engine starts, and a tick of the
// sweep.
const (
atStart = true
onTick = false
)
// recoveryRun names one of the two runs of the recovery routine.
type recoveryRun struct {
name string
atStart bool
}
// recoveryRuns returns the routine's run at start and a tick.
func recoveryRuns() []recoveryRun {
return []recoveryRun{
{"at start", atStart},
{"on a tick", onTick},
}
}
// recoverySetup gives a fresh webhook its main-database row and an http
// target with retries, and returns the setup and the target's ID. No
// worker runs, so whatever the routine queues stays in the channels for
// the test to read, and nothing is sent to the target.
func recoverySetup(t *testing.T) (iSetup, string) {
t.Helper()
s := newISetup(t)
targetID := uuid.New().String()
iCreateWebhook(t, s.MainDB, s.WebhookID, "recovery")
iCreateTarget(
t, s.MainDB, targetID, s.WebhookID, "recovery",
database.TargetTypeHTTP,
"http://example.com/hook", 20,
)
return s, targetID
}
// seedRecoveryDelivery seeds a delivery at status with the given
// recorded attempts, failed but for the last when lastSucceeded is set,
// all recorded now.
func seedRecoveryDelivery(
t *testing.T,
s iSetup,
targetID string,
status database.DeliveryStatus,
attempts int,
lastSucceeded bool,
) string {
t.Helper()
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"recovery":true}`)
d := iSeedDelivery(t, s.WebhookDB, event.ID, targetID, status)
for n := 1; n <= attempts; n++ {
rSeedResult(
t, s.WebhookDB, d.ID, n, lastSucceeded && n == attempts,
)
}
return d.ID
}
// backdateAttempts moves every recorded attempt of a delivery an hour
// into the past, long past the backoff after its first attempt.
func backdateAttempts(t *testing.T, s iSetup, deliveryID string) {
t.Helper()
require.NoError(t, s.WebhookDB.Model(&database.DeliveryResult{}).
Where("delivery_id = ?", deliveryID).
UpdateColumn("created_at", s.WebhookDB.NowFunc().Add(-time.Hour)).
Error)
}
// requireQueued waits for a task on ch. A retrying delivery reaches the
// retry channel through a timer, even one due now, so it is waited for.
func requireQueued(
t *testing.T, ch <-chan delivery.Task,
) delivery.Task {
t.Helper()
select {
case task := <-ch:
return task
case <-time.After(5 * time.Second):
t.Fatal("the recovery routine queued nothing")
return delivery.Task{}
}
}
// assertNothingQueued asserts the engine queues nothing, giving a retry
// timer that is due now the moment it needs to fire.
func assertNothingQueued(t *testing.T, e *delivery.Engine) {
t.Helper()
select {
case task := <-e.ExportDeliveryCh():
t.Errorf("queued pending delivery %s", task.DeliveryID)
case task := <-e.ExportRetryCh():
t.Errorf("queued retrying delivery %s", task.DeliveryID)
case <-time.After(200 * time.Millisecond):
}
}
// TestRecovery_RetryingBackoffPassed: a retrying delivery whose backoff
// is over is queued for its next attempt at once.
func TestRecovery_RetryingBackoffPassed(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 1, false,
)
backdateAttempts(t, s, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportRetryCh())
assert.Equal(t, id, task.DeliveryID)
assert.Equal(t, 2, task.AttemptNum)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusRetrying)
})
}
}
// TestRecovery_RetryingBackoffNotPassed: a retrying delivery whose last
// attempt was just now waits out the rest of its backoff, ten attempts
// in, eight and a half minutes. It is not sent now, and the engine holds
// it, so a later tick leaves it to the timer.
func TestRecovery_RetryingBackoffNotPassed(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 10, false,
)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusRetrying)
assert.Equal(t, 1, s.Engine.ExportInflightHeld(),
"no retry timer holds the delivery",
)
})
}
}
// TestRecovery_PendingOlderThanTheAgeBound: a pending delivery untouched
// for longer than the sweep's age bound is sent by either run.
func TestRecovery_PendingOlderThanTheAgeBound(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, id, task.DeliveryID)
assert.Equal(t, 1, task.AttemptNum)
})
}
}
// TestRecovery_TaskCarriesEveryTargetSetting: the task the recovery
// routine queues carries every one of the target's settings, all six
// set here.
// They are copied into the task one field at a time, and a field left
// out is sent empty without any error.
func TestRecovery_TaskCarriesEveryTargetSetting(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s := newISetup(t)
iCreateWebhook(t, s.MainDB, s.WebhookID, "recovery")
target := database.Target{
WebhookID: s.WebhookID,
Name: "recovery",
Type: database.TargetTypeHTTP,
Active: true,
URL: "http://example.com/hook",
Headers: map[string]string{headerAuthorization: "Bearer t"},
TimeoutSeconds: 7,
ForwardQuery: true,
ArchiveExpiry: time.Hour,
ArchiveRotation: "daily",
MaxAttempts: 1,
}
require.NoError(t, s.MainDB.Create(&target).Error)
id := seedRecoveryDelivery(
t, s, target.ID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, id)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.URL, task.TargetURL)
assert.Equal(t, target.Headers, task.TargetHeaders)
assert.Equal(t, target.TimeoutSeconds, task.TimeoutSeconds)
assert.Equal(t, target.ForwardQuery, task.ForwardQuery)
assert.Equal(t, target.ArchiveExpiry, task.ArchiveExpiry)
assert.Equal(t, target.ArchiveRotation, task.ArchiveRotation)
})
}
}
// TestRecovery_PendingYoungerThanTheAgeBound: a pending delivery touched
// a moment ago, as one a stop cut off is, is sent at start, which is how
// it reaches its target on the next start
// (https://git.eeqj.de/sneak/webhooker/issues/527). A tick leaves it.
func TestRecovery_PendingYoungerThanTheAgeBound(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
id := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
if run.atStart {
task := requireQueued(t, s.Engine.ExportDeliveryCh())
assert.Equal(t, id, task.DeliveryID)
return
}
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, id, database.DeliveryStatusPending)
assert.Zero(t, s.Engine.ExportInflightHeld())
})
}
}
// TestRecovery_AlreadySucceeded: a pending or retrying delivery whose
// success is recorded but whose status write was lost is marked
// delivered where it stands, and not sent again.
func TestRecovery_AlreadySucceeded(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
pending := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 1, true,
)
rAgePending(t, s.WebhookDB, pending)
retrying := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 2, true,
)
backdateAttempts(t, s, retrying)
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
for id, attempts := range map[string]int{pending: 1, retrying: 2} {
iAssertStatus(
t, s.WebhookDB, id, database.DeliveryStatusDelivered,
)
assert.Len(t, iResults(t, s.WebhookDB, id), attempts,
"marking the delivery delivered recorded an attempt",
)
}
})
}
}
// TestRecovery_MissingDatabase: a tick leaves a webhook whose database
// file is missing alone, and the run at start opens it, which creates an
// empty one and reports the loss (see
// TestRecoverDeliveries_ReportsAMissingWebhookDatabase).
func TestRecovery_MissingDatabase(t *testing.T) {
t.Parallel()
mainDB := iMainDB(t)
webhookID := uuid.New().String()
iCreateWebhook(t, mainDB.DB(), webhookID, "missing-database")
dbMgr := databasetest.NewWebhookDBManager(t, t.TempDir())
t.Cleanup(func() { _ = dbMgr.CloseAll() })
engine := delivery.NewTestEngineWithDB(
mainDB, dbMgr,
slog.New(slog.DiscardHandler),
&http.Client{Timeout: 5 * time.Second}, 1,
)
engine.ExportRecoverDeliveries(context.Background(), onTick)
assert.False(t, dbMgr.DBExists(webhookID),
"a tick created the missing database",
)
engine.ExportRecoverDeliveries(context.Background(), atStart)
assert.True(t, dbMgr.DBExists(webhookID),
"the run at start did not open the missing database",
)
}
// TestRecovery_LeavesADeliveryTheEngineHolds: a delivery the engine is
// working on, has queued, or holds a retry timer for is left alone by
// either run, pending and retrying alike, however old.
func TestRecovery_LeavesADeliveryTheEngineHolds(t *testing.T) {
t.Parallel()
for _, run := range recoveryRuns() {
t.Run(run.name, func(t *testing.T) {
t.Parallel()
s, targetID := recoverySetup(t)
pending := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusPending, 0, false,
)
rAgePending(t, s.WebhookDB, pending)
retrying := seedRecoveryDelivery(
t, s, targetID, database.DeliveryStatusRetrying, 1, false,
)
backdateAttempts(t, s, retrying)
require.True(t, s.Engine.ExportRetainDelivery(pending))
require.True(t, s.Engine.ExportRetainDelivery(retrying))
s.Engine.ExportRecoverDeliveries(context.Background(), run.atStart)
assertNothingQueued(t, s.Engine)
iAssertStatus(t, s.WebhookDB, pending, database.DeliveryStatusPending)
iAssertStatus(t, s.WebhookDB, retrying, database.DeliveryStatusRetrying)
assert.Equal(t, 2, s.Engine.ExportInflightHeld())
})
}
}
+164
View File
@@ -0,0 +1,164 @@
package delivery
import (
"fmt"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
)
// StoreEvent stores event with one pending delivery for each of its
// webhook's active targets, and adds them to the totals, in one
// transaction, then queues the deliveries. It returns how many
// deliveries it created.
//
// The receiver stores every event it accepts through here, and a
// resubmit stores its copy of a stored event, so every event is
// stored, retried, SSRF-guarded and circuit-broken the same way however
// it entered the system. CreateReplay is the only other place a
// delivery is created.
//
// The targets are the webhook's active ones as they stand now: a
// target created after a resubmitted event first arrived is included,
// and an inactive one is skipped. An event with no active target is
// stored with no deliveries.
func (e *Engine) StoreEvent(event *database.Event) (int, error) {
var targets []database.Target
err := e.database.DB().Where(
"webhook_id = ? AND active = ?", event.WebhookID, true,
).Find(&targets).Error
if err != nil {
return 0, fmt.Errorf("loading active targets: %w", err)
}
webhookDB, err := e.dbManager.GetDB(event.WebhookID)
if err != nil {
return 0, fmt.Errorf("getting webhook database: %w", err)
}
event.BodyBytes = int64(len(event.Body))
tasks := make([]Task, 0, len(targets))
err = webhookDB.Transaction(func(tx *gorm.DB) error {
err := tx.Create(event).Error
if err != nil {
return fmt.Errorf("creating event: %w", err)
}
for i := range targets {
dlv, err := createDelivery(tx, event.ID, targets[i].ID, false)
if err != nil {
return err
}
tasks = append(tasks, buildTask(
dlv, event.WebhookID, event, &targets[i], 1,
))
}
err = database.AddEventTotals(tx, database.EventTotals{
Events: 1, LastEventAt: &event.CreatedAt,
})
if err != nil {
return err
}
// A resubmitted copy did not arrive on its entrypoint's URL, so
// it leaves the entrypoint's last event as it is.
if event.ResubmittedFromID != nil {
return nil
}
return database.AddEntrypointTotals(tx, database.EntrypointTotals{
EntrypointID: event.EntrypointID, LastEventAt: event.CreatedAt,
})
})
if err != nil {
return 0, err
}
// Counted here, after the commit: an event exists once it is
// durably stored, which is what the delivery counters are compared
// against on a dashboard. A resubmitted event counts too, because
// it produces deliveries that the delivery side counts; the
// resubmit counter is what separates the two.
e.mtr.EventReceived()
e.notify(tasks)
return len(tasks), nil
}
// CreateReplay creates one more pending delivery of the stored event
// eventID to target, marked as a replay, adds it to the target's totals
// in the same transaction, and queues it. It returns the new
// delivery's id.
//
// The caller passes the target as it stands now and has already
// decided the replay may go ahead. The delivery being replayed is not
// touched.
func (e *Engine) CreateReplay(
webhookID, eventID string, target *database.Target,
) (string, error) {
webhookDB, err := e.dbManager.GetDB(webhookID)
if err != nil {
return "", fmt.Errorf("getting webhook database: %w", err)
}
event, err := e.loadEvent(webhookDB, eventID)
if err != nil {
return "", err
}
var dlv *database.Delivery
err = webhookDB.Transaction(func(tx *gorm.DB) error {
var err error
dlv, err = createDelivery(tx, event.ID, target.ID, true)
return err
})
if err != nil {
return "", err
}
e.notify([]Task{buildTask(dlv, webhookID, &event, target, 1)})
return dlv.ID, nil
}
// createDelivery writes a pending delivery of the event to the target
// on the caller's transaction, and adds it to the target's totals.
//
// Neither Event nor Target is set on the row, and the per-webhook
// database omits associations on every write in any case (see
// internal/database/event_db_isolation.go), so the target's settings
// are never copied into the event database.
func createDelivery(
tx *gorm.DB, eventID, targetID string, replay bool,
) (*database.Delivery, error) {
dlv := &database.Delivery{
EventID: eventID,
TargetID: targetID,
Status: database.DeliveryStatusPending,
Replay: replay,
}
err := tx.Create(dlv).Error
if err != nil {
return nil, fmt.Errorf(
"creating delivery for target %s: %w", targetID, err,
)
}
err = database.AddTargetTotals(tx, database.TargetTotals{
TargetID: targetID, Deliveries: 1,
})
if err != nil {
return nil, err
}
return dlv, nil
}
+386
View File
@@ -0,0 +1,386 @@
package delivery_test
import (
"net/http"
"strings"
"testing"
"time"
"github.com/google/uuid"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
)
// storeTestTarget creates an active http target of the setup's webhook
// with every one of a target's settings set, so a task that drops one
// of them differs from the target.
func storeTestTarget(
t *testing.T, s iSetup, name string,
) database.Target {
t.Helper()
target := database.Target{
WebhookID: s.WebhookID,
Name: name,
Type: database.TargetTypeHTTP,
Active: true,
URL: "http://93.184.216.34/" + name,
Headers: map[string]string{"X-Api-Key": "k"},
TimeoutSeconds: 7,
ForwardQuery: true,
ArchiveExpiry: time.Hour,
ArchiveRotation: rotationDaily,
MaxAttempts: 3,
}
require.NoError(t, s.MainDB.Create(&target).Error)
return target
}
// newStoreTestEvent returns an event of the setup's webhook as the
// receiver builds one, not yet stored.
func newStoreTestEvent(s iSetup, body string) *database.Event {
return &database.Event{
WebhookID: s.WebhookID,
EntrypointID: uuid.New().String(),
Method: http.MethodPost,
RawQuery: "a=1&b=2",
Headers: `{"X-Test":["yes"]}`,
ContentType: testContentType,
Body: body,
}
}
// queuedTasks takes every task the engine has queued so far. The
// engine in these tests is never started, so nothing else takes them.
func queuedTasks(e *delivery.Engine) []delivery.Task {
var tasks []delivery.Task
for {
select {
case task := <-e.ExportDeliveryCh():
tasks = append(tasks, task)
default:
return tasks
}
}
}
// assertTaskFor proves task carries delivery deliveryID of event to
// target: every one of the target's settings and every event field.
// They are copied into the task one field at a time, and a field left
// out is sent empty without any error.
func assertTaskFor(
t *testing.T,
task delivery.Task,
deliveryID string,
event *database.Event,
target *database.Target,
) {
t.Helper()
assert.Equal(t, deliveryID, task.DeliveryID)
assert.Equal(t, event.ID, task.EventID)
assert.Equal(t, event.WebhookID, task.WebhookID)
assert.Equal(t, event.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Name, task.TargetName)
assert.Equal(t, target.Type, task.TargetType)
assert.Equal(t, target.URL, task.TargetURL)
assert.Equal(t, target.Headers, task.TargetHeaders)
assert.Equal(t, target.TimeoutSeconds, task.TimeoutSeconds)
assert.Equal(t, target.ForwardQuery, task.ForwardQuery)
assert.Equal(t, target.ArchiveExpiry, task.ArchiveExpiry)
assert.Equal(t, target.ArchiveRotation, task.ArchiveRotation)
assert.Equal(t, target.MaxAttempts, task.MaxAttempts)
assert.Equal(t, event.Method, task.Method)
assert.Equal(t, event.RawQuery, task.RawQuery)
assert.Equal(t, event.Headers, task.Headers)
assert.Equal(t, event.ContentType, task.ContentType)
require.NotNil(t, task.Body)
assert.Equal(t, event.Body, *task.Body)
assert.Equal(t, 1, task.AttemptNum)
}
// storedDeliveries reads every delivery of an event.
func storedDeliveries(
t *testing.T, db *gorm.DB, eventID string,
) []database.Delivery {
t.Helper()
var deliveries []database.Delivery
require.NoError(t, db.Where("event_id = ?", eventID).
Find(&deliveries).Error)
return deliveries
}
// storedEventTotals reads the webhook's event totals.
func storedEventTotals(t *testing.T, db *gorm.DB) database.EventTotals {
t.Helper()
var totals database.EventTotals
require.NoError(t, db.First(&totals).Error)
return totals
}
// countRows counts the rows of model in db.
func countRows(t *testing.T, db *gorm.DB, model any) int64 {
t.Helper()
var n int64
require.NoError(t, db.Model(model).Count(&n).Error)
return n
}
// TestStoreEvent_StoresDeliveriesToActiveTargets proves StoreEvent
// stores the event with one pending delivery to each active target and
// none to an inactive one, adds them to the totals, and queues one task
// per delivery.
func TestStoreEvent_StoresDeliveriesToActiveTargets(t *testing.T) {
t.Parallel()
s := newISetup(t)
targets := []database.Target{
storeTestTarget(t, s, "first"),
storeTestTarget(t, s, "second"),
}
off := storeTestTarget(t, s, "off")
require.NoError(t, s.MainDB.Model(&database.Target{}).
Where("id = ?", off.ID).Update("active", false).Error)
event := newStoreTestEvent(s, `{"hello":"world"}`)
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Equal(t, 2, created)
var stored database.Event
require.NoError(t, s.WebhookDB.First(&stored, "id = ?", event.ID).Error)
assert.Equal(t, event.Body, stored.Body)
assert.Equal(t, int64(len(event.Body)), stored.BodyBytes)
rows := storedDeliveries(t, s.WebhookDB, event.ID)
queued := queuedTasks(s.Engine)
require.Len(t, rows, 2)
require.Len(t, queued, 2)
deliveries := map[string]database.Delivery{}
for _, dlv := range rows {
deliveries[dlv.TargetID] = dlv
}
tasks := map[string]delivery.Task{}
for _, task := range queued {
tasks[task.TargetID] = task
}
for i := range targets {
dlv, ok := deliveries[targets[i].ID]
require.True(t, ok, "no delivery to %s", targets[i].Name)
assert.Equal(t, database.DeliveryStatusPending, dlv.Status)
assert.False(t, dlv.Replay)
assert.Equal(t, int64(1),
targetTotals(t, s.WebhookDB, targets[i].ID).Deliveries)
assertTaskFor(t, tasks[targets[i].ID], dlv.ID, event, &targets[i])
}
totals := storedEventTotals(t, s.WebhookDB)
assert.Equal(t, int64(1), totals.Events)
require.NotNil(t, totals.LastEventAt)
assert.True(t, totals.LastEventAt.Equal(stored.CreatedAt))
var entrypoint database.EntrypointTotals
require.NoError(t, s.WebhookDB.First(
&entrypoint, "entrypoint_id = ?", event.EntrypointID,
).Error)
assert.True(t, entrypoint.LastEventAt.Equal(stored.CreatedAt))
}
// TestStoreEvent_ResubmittedCopyLeavesTheEntrypoint proves a
// resubmitted copy is counted among the webhook's events but leaves its
// entrypoint's last event as it is: it did not arrive on that URL.
func TestStoreEvent_ResubmittedCopyLeavesTheEntrypoint(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
sourceID := uuid.New().String()
event := newStoreTestEvent(s, `{"copy":true}`)
event.ResubmittedFromID = &sourceID
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Equal(t, 1, created)
assert.Len(t, queuedTasks(s.Engine), 1)
assert.Equal(t, int64(1), storedEventTotals(t, s.WebhookDB).Events)
assert.Zero(t, countRows(t, s.WebhookDB, &database.EntrypointTotals{}))
}
// TestStoreEvent_NoActiveTarget proves an event of a webhook with no
// active target is stored with no delivery and nothing queued.
func TestStoreEvent_NoActiveTarget(t *testing.T) {
t.Parallel()
s := newISetup(t)
event := newStoreTestEvent(s, `{"to":"nobody"}`)
created, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
assert.Zero(t, created)
assert.Equal(t, int64(1), countRows(t, s.WebhookDB, &database.Event{}))
assert.Empty(t, storedDeliveries(t, s.WebhookDB, event.ID))
assert.Empty(t, queuedTasks(s.Engine))
}
// TestStoreEvent_LargeBodyIsNotCarried proves a body at the inline
// limit is stored whole but left off the task, for the attempt to read
// from the event row.
func TestStoreEvent_LargeBodyIsNotCarried(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
event := newStoreTestEvent(
s, strings.Repeat("x", delivery.MaxInlineBodySize),
)
_, err := s.Engine.StoreEvent(event)
require.NoError(t, err)
tasks := queuedTasks(s.Engine)
require.Len(t, tasks, 1)
assert.Nil(t, tasks[0].Body)
var stored database.Event
require.NoError(t, s.WebhookDB.First(&stored, "id = ?", event.ID).Error)
assert.Equal(t, event.Body, stored.Body)
}
// TestStoreEvent_RollsBackWhenAWriteFails proves the event, its
// deliveries and its totals are one transaction: when the entrypoint
// totals write fails, the last one, after the event, its delivery, the
// target totals and the event totals were written, none of them is left
// and nothing is queued.
func TestStoreEvent_RollsBackWhenAWriteFails(t *testing.T) {
t.Parallel()
s := newISetup(t)
storeTestTarget(t, s, "only")
require.NoError(t,
s.WebhookDB.Migrator().DropTable(&database.EntrypointTotals{}))
_, err := s.Engine.StoreEvent(newStoreTestEvent(s, `{"lost":true}`))
require.Error(t, err)
assert.Zero(t, countRows(t, s.WebhookDB, &database.Event{}))
assert.Zero(t, countRows(t, s.WebhookDB, &database.Delivery{}))
assert.Zero(t, countRows(t, s.WebhookDB, &database.TargetTotals{}))
totals := storedEventTotals(t, s.WebhookDB)
assert.Zero(t, totals.Events)
assert.Nil(t, totals.LastEventAt)
assert.Empty(t, queuedTasks(s.Engine))
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// TestCreateReplay_CreatesAndQueuesAReplay proves CreateReplay adds a
// new pending delivery marked as a replay, counts it in the target's
// totals, queues its task built from the target it was given and the
// stored event, and leaves the delivery it repeats as it was. The
// stored event has every event field set, so a task that drops one of
// them differs from the event.
func TestCreateReplay_CreatesAndQueuesAReplay(t *testing.T) {
t.Parallel()
s := newISetup(t)
target := storeTestTarget(t, s, "replayed")
event := newStoreTestEvent(s, `{"replay":"me"}`)
require.NoError(t, s.WebhookDB.Create(event).Error)
original := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusFailed,
)
deliveryID, err := s.Engine.CreateReplay(s.WebhookID, event.ID, &target)
require.NoError(t, err)
replay := storedDelivery(t, s.WebhookDB, deliveryID)
assert.Equal(t, event.ID, replay.EventID)
assert.Equal(t, target.ID, replay.TargetID)
assert.Equal(t, database.DeliveryStatusPending, replay.Status)
assert.True(t, replay.Replay)
assert.Equal(t, database.DeliveryStatusFailed,
storedDelivery(t, s.WebhookDB, original.ID).Status)
assert.Equal(t, int64(1),
targetTotals(t, s.WebhookDB, target.ID).Deliveries)
tasks := queuedTasks(s.Engine)
require.Len(t, tasks, 1)
assertTaskFor(t, tasks[0], deliveryID, event, &target)
}
// TestCreateReplay_RollsBackWhenAWriteFails proves the replay delivery
// and its target totals are one transaction: when the totals write
// fails, after the delivery was written, no new delivery is left and
// nothing is queued.
func TestCreateReplay_RollsBackWhenAWriteFails(t *testing.T) {
t.Parallel()
s := newISetup(t)
target := storeTestTarget(t, s, "replayed")
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"replay":"me"}`)
iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusFailed,
)
require.NoError(t,
s.WebhookDB.Migrator().DropTable(&database.TargetTotals{}))
_, err := s.Engine.CreateReplay(s.WebhookID, event.ID, &target)
require.Error(t, err)
assert.Len(t, storedDeliveries(t, s.WebhookDB, event.ID), 1)
assert.Empty(t, queuedTasks(s.Engine))
assert.Zero(t, s.Engine.ExportInflightHeld())
}
// storedDelivery reads one delivery back.
func storedDelivery(
t *testing.T, db *gorm.DB, deliveryID string,
) database.Delivery {
t.Helper()
var dlv database.Delivery
require.NoError(t, db.First(&dlv, "id = ?", deliveryID).Error)
return dlv
}
+9 -18
View File
@@ -13,8 +13,8 @@ import (
// The engine provides one to each target so a target can own // The engine provides one to each target so a target can own
// its retries durably: it records the attempt, marks the // its retries durably: it records the attempt, marks the
// delivery retrying, and asks the Scheduler to deliver the // delivery retrying, and asks the Scheduler to deliver the
// next attempt after delay — exactly what the engine does for // next attempt after delay — exactly what the engine's
// its own restart recovery. // recovery routine does for a retrying delivery.
type Scheduler interface { type Scheduler interface {
ScheduleRetry(task Task, delay time.Duration) ScheduleRetry(task Task, delay time.Duration)
} }
@@ -45,29 +45,20 @@ type Target interface {
} }
// rescheduler is implemented by targets that own durable // rescheduler is implemented by targets that own durable
// retries. The engine's restart recovery and periodic sweep // retries. The engine's recovery routine uses it to let the
// use it to let the target recompute the schedule for an // target recompute the schedule for an orphaned retrying
// orphaned retrying delivery, keeping the retry schedule // delivery, keeping the retry schedule target-owned.
// target-owned. Fire-and-forget targets do not implement it // Fire-and-forget targets do not implement it, and a retrying
// and their (never-occurring) retrying deliveries are // delivery found for one is terminally failed.
// skipped.
type rescheduler interface { type rescheduler interface {
// remainingBackoff returns how long to wait before the // remainingBackoff returns how long to wait before the
// next attempt of a recovered retrying delivery. // next attempt of a recovered retrying delivery: zero once
// its backoff has passed.
remainingBackoff( remainingBackoff(
webhookDB *gorm.DB, webhookDB *gorm.DB,
deliveryID string, deliveryID string,
attemptNum int, attemptNum int,
) time.Duration ) time.Duration
// backoffElapsed reports whether the backoff window for
// the last attempt has already passed, so the periodic
// sweep can re-enqueue the delivery now.
backoffElapsed(
webhookDB *gorm.DB,
deliveryID string,
attemptNum int,
) bool
} }
// attemptResult is the outcome of a single delivery attempt, // attemptResult is the outcome of a single delivery attempt,
+86
View File
@@ -1,6 +1,7 @@
package delivery_test package delivery_test
import ( import (
"context"
"database/sql" "database/sql"
"fmt" "fmt"
"log/slog" "log/slog"
@@ -20,6 +21,7 @@ import (
"sneak.berlin/go/webhooker/internal/archive" "sneak.berlin/go/webhooker/internal/archive"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/database/databasetest" "sneak.berlin/go/webhooker/internal/database/databasetest"
"sneak.berlin/go/webhooker/internal/db/migrations"
"sneak.berlin/go/webhooker/internal/delivery" "sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/gormlog" "sneak.berlin/go/webhooker/internal/gormlog"
) )
@@ -307,3 +309,87 @@ func TestDeliverDatabase_ArchiveFailureFailsDelivery(
"no archive file should exist for a failed write", "no archive file should exist for a failed write",
) )
} }
// seedArchivedRow creates the archive file at path holding one row, for
// eventID, archived at archivedAt.
func seedArchivedRow(
t *testing.T, path, eventID string, archivedAt time.Time,
) {
t.Helper()
sqlDB, err := sql.Open(
"sqlite", fmt.Sprintf("file:%s?mode=rwc", path),
)
require.NoError(t, err)
defer func() { _ = sqlDB.Close() }()
gdb, err := gorm.Open(
sqlite.Dialector{Conn: sqlDB},
&gorm.Config{Logger: gormlog.New(slog.New(slog.DiscardHandler))},
)
require.NoError(t, err)
require.NoError(t, migrations.Apply(gdb, migrations.Archive))
require.NoError(t, gdb.Exec(
"INSERT INTO archived_events (event_id, archived_at) VALUES (?, ?)",
eventID, archivedAt,
).Error)
}
// TestDeliverDatabase_TaskArchiveSettings proves a task's ArchiveExpiry
// and ArchiveRotation reach the archive through the engine. The
// target's row has neither, so only the task can bring them: with daily
// rotation the event goes to the file for its day, and opening that
// file with a one-hour expiry prunes the row it already held, archived
// two hours ago.
func TestDeliverDatabase_TaskArchiveSettings(t *testing.T) {
t.Parallel()
s := newISetup(t)
webhook := database.Webhook{UserID: uuid.NewString(), Name: "settings"}
webhook.ID = s.WebhookID
require.NoError(t, s.MainDB.Create(&webhook).Error)
target := database.Target{
WebhookID: s.WebhookID,
Name: "archive",
Type: database.TargetTypeDatabase,
Active: true,
MaxAttempts: 1,
}
require.NoError(t, s.MainDB.Create(&target).Error)
event := iSeedEvent(t, s.WebhookDB, s.WebhookID, `{"archived":true}`)
d := iSeedDelivery(
t, s.WebhookDB, event.ID, target.ID,
database.DeliveryStatusPending,
)
task := iTask(
d, event, s.WebhookID, target.ID, target.Name, "", 1, 1,
&event.Body,
)
task.TargetType = database.TargetTypeDatabase
task.ArchiveExpiry = time.Hour
task.ArchiveRotation = "daily"
daily := target
daily.ArchiveRotation = task.ArchiveRotation
dailyFile := archive.PathAt(s.DBMgr, &webhook, &daily, event.CreatedAt)
seedArchivedRow(t, dailyFile, "expired", time.Now().Add(-2*time.Hour))
s.Engine.ExportProcessNewTask(context.TODO(), &task)
iAssertStatus(t, s.WebhookDB, d.ID, database.DeliveryStatusDelivered)
var rows []archivedRow
require.NoError(t, openArchiveDBForRead(t, dailyFile).
Table("archived_events").Find(&rows).Error)
require.Len(t, rows, 1)
assert.Equal(t, event.ID, rows[0].EventID,
"the event goes to its day's file and the expired row is pruned",
)
}
+4 -26
View File
@@ -60,8 +60,8 @@ func (c *httpCore) deliver(
// engine is stopping, which cancels the context its request is made // engine is stopping, which cancels the context its request is made
// with. Such an attempt is not recorded: no DeliveryResult, no circuit // with. Such an attempt is not recorded: no DeliveryResult, no circuit
// breaker outcome, no status change, no count in the attempt metrics. // breaker outcome, no status change, no count in the attempt metrics.
// The delivery stays pending or retrying, and startup recovery sends // The delivery stays pending or retrying, and the recovery routine
// it again on the next start. // sends it again on the next start.
// //
// A target's own timeout leaves that context alone, so its attempt is // A target's own timeout leaves that context alone, so its attempt is
// still recorded as the failure it is. // still recorded as the failure it is.
@@ -310,7 +310,8 @@ func (c *httpCore) publishCircuitState(
// remainingBackoff returns how long remains of the backoff // remainingBackoff returns how long remains of the backoff
// window for the last attempt of a recovered retrying // window for the last attempt of a recovered retrying
// delivery. It implements rescheduler. // delivery: zero once it has passed, or when the last attempt
// cannot be read. It implements rescheduler.
func (c *httpCore) remainingBackoff( func (c *httpCore) remainingBackoff(
webhookDB *gorm.DB, webhookDB *gorm.DB,
deliveryID string, deliveryID string,
@@ -333,29 +334,6 @@ func (c *httpCore) remainingBackoff(
return max(remaining, 0) return max(remaining, 0)
} }
// backoffElapsed reports whether the backoff window for the
// last attempt of a retrying delivery has passed. It
// implements rescheduler.
func (c *httpCore) backoffElapsed(
webhookDB *gorm.DB,
deliveryID string,
attemptNum int,
) bool {
var lastResult database.DeliveryResult
err := webhookDB.
Where("delivery_id = ?", deliveryID).
Order("created_at DESC").
First(&lastResult).Error
if err != nil {
return true
}
backoff := Backoff(attemptNum)
return time.Since(lastResult.CreatedAt) >= backoff
}
// Backoff is how long an http or slack target with retries waits after // Backoff is how long an http or slack target with retries waits after
// a delivery's failed attempt attemptNum before trying it again. // a delivery's failed attempt attemptNum before trying it again.
func Backoff(attemptNum int) time.Duration { func Backoff(attemptNum int) time.Duration {
+38 -37
View File
@@ -178,7 +178,7 @@ func TestRecoverSingleRetry_TargetDeleted(t *testing.T) {
) )
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
iAssertStatus( iAssertStatus(
@@ -201,7 +201,7 @@ func TestRecoverSingleRetry_TargetDeleted(t *testing.T) {
) )
} }
func TestSweepSingleRetry_TargetDeleted(t *testing.T) { func TestRecoverSingleRetry_OnTick_TargetDeleted(t *testing.T) {
t.Parallel() t.Parallel()
s := newISetup(t) s := newISetup(t)
@@ -218,11 +218,11 @@ func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
// Twice, because the bug was an error the sweep repeated every // Twice, because the bug was an error the sweep repeated every
// minute for the life of the database: the second sweep must // minute for the life of the database: the second sweep must
// find nothing left to do. // find nothing left to do.
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertStatus( iAssertStatus(
@@ -239,10 +239,11 @@ func TestSweepSingleRetry_TargetDeleted(t *testing.T) {
assert.Zero(t, s.Engine.ExportInflightHeld()) assert.Zero(t, s.Engine.ExportInflightHeld())
} }
// TestSweepSingleRetry_TargetNeverExisted covers the other half of the // TestRecoverSingleRetry_OnTick_TargetNeverExisted covers the other
// soft-delete distinction: an id with no row at all, deleted or // half of the soft-delete distinction: an id with no row at all,
// otherwise, must not be reported as something the operator deleted. // deleted or otherwise, must not be reported as something the operator
func TestSweepSingleRetry_TargetNeverExisted(t *testing.T) { // deleted.
func TestRecoverSingleRetry_OnTick_TargetNeverExisted(t *testing.T) {
t.Parallel() t.Parallel()
s := newISetup(t) s := newISetup(t)
@@ -264,8 +265,8 @@ func TestSweepSingleRetry_TargetNeverExisted(t *testing.T) {
iSeedFailedResult(t, s.WebhookDB, d.ID) iSeedFailedResult(t, s.WebhookDB, d.ID)
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertStatus( iAssertStatus(
@@ -303,8 +304,8 @@ func TestFailMissingTarget_WritesNoTargetRow(
database.DeliveryStatusRetrying, database.DeliveryStatusRetrying,
) )
s.Engine.ExportSweepWebhookRetries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, onTick,
) )
iAssertStatus( iAssertStatus(
@@ -545,7 +546,7 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
require.NoError(t, sqlDB.Close()) require.NoError(t, sqlDB.Close())
s.Engine.ExportRecoverRetryingDeliveries( s.Engine.ExportRecoverRetryingDeliveries(
s.WebhookDB, s.WebhookID, context.Background(), s.WebhookDB, s.WebhookID,
) )
iAssertStatus( iAssertStatus(
@@ -563,7 +564,7 @@ func TestRecoverSingleRetry_TargetUnreadable_LeavesDeliveryAlone(
// --- 4. A pending delivery whose target is gone --- // --- 4. A pending delivery whose target is gone ---
func TestRecoverPending_TargetDeleted(t *testing.T) { func TestRecoverPendingDeliveries_TargetDeleted(t *testing.T) {
t.Parallel() t.Parallel()
s := newISetup(t) s := newISetup(t)
@@ -576,7 +577,7 @@ func TestRecoverPending_TargetDeleted(t *testing.T) {
) )
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
iAssertStatus( iAssertStatus(
@@ -599,10 +600,10 @@ func TestRecoverPending_TargetDeleted(t *testing.T) {
) )
} }
// TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone: the // TestRecoverPendingDeliveries_TargetDeleted_LeavesAnOwnedDeliveryAlone:
// terminal write takes ownership like every other recovery write, so a // the terminal write takes ownership like every other recovery write, so
// delivery the engine still holds is not failed underneath its worker. // a delivery the engine still holds is not failed underneath its worker.
func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone( func TestRecoverPendingDeliveries_TargetDeleted_LeavesAnOwnedDeliveryAlone(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -619,7 +620,7 @@ func TestRecoverPending_TargetDeleted_LeavesAnOwnedDeliveryAlone(
require.True(t, s.Engine.ExportRetainDelivery(deliveryID)) require.True(t, s.Engine.ExportRetainDelivery(deliveryID))
s.Engine.ExportRecoverWebhookDeliveries( s.Engine.ExportRecoverWebhookDeliveries(
context.Background(), s.WebhookID, context.Background(), s.WebhookID, atStart,
) )
iAssertStatus( iAssertStatus(
@@ -674,18 +675,18 @@ func TestFailMissingTarget_LeavesASettledDeliveryAlone(
assert.Zero(t, s.Engine.ExportInflightHeld()) assert.Zero(t, s.Engine.ExportInflightHeld())
} }
// TestSweepPending_TargetDeleted sweeps twice over a batch that also // TestRecoverPendingDeliveries_OnTick_TargetDeleted sweeps twice over a
// holds a healthy stranded delivery. The one whose target is gone is // batch that also holds a healthy stranded delivery. The one whose
// failed once and then left alone; the healthy one is queued by the // target is gone is failed once and then left alone; the healthy one is
// first sweep and not again by the second. // queued by the first sweep and not again by the second.
func TestSweepPending_TargetDeleted(t *testing.T) { func TestRecoverPendingDeliveries_OnTick_TargetDeleted(t *testing.T) {
t.Parallel() t.Parallel()
liveTargetID := uuid.New().String() liveTargetID := uuid.New().String()
s := fSweepSetup(t, liveTargetID, "still-there") s := fSweepSetup(t, liveTargetID, "still-there")
deliveryID := tSeedDeletedTarget( deliveryID := tSeedDeletedTarget(
t, s, "gone-on-pending-sweep", "http://example.com/hook", t, s, "gone-while-pending-on-tick", "http://example.com/hook",
database.DeliveryStatusPending, database.DeliveryStatusPending,
) )
rAgePending(t, s.WebhookDB, deliveryID) rAgePending(t, s.WebhookDB, deliveryID)
@@ -702,7 +703,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
ctx := context.Background() ctx := context.Background()
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID) s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
tasks := fDrain(s.Engine) tasks := fDrain(s.Engine)
require.Len(t, tasks, 1, require.Len(t, tasks, 1,
@@ -710,7 +711,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
) )
assert.Equal(t, healthy.ID, tasks[0].DeliveryID) assert.Equal(t, healthy.ID, tasks[0].DeliveryID)
s.Engine.ExportSweepWebhookRetries(ctx, s.WebhookID) s.Engine.ExportRecoverWebhookDeliveries(ctx, s.WebhookID, onTick)
assert.Empty(t, fDrain(s.Engine), assert.Empty(t, fDrain(s.Engine),
"the second sweep queued a delivery again", "the second sweep queued a delivery again",
@@ -723,7 +724,7 @@ func TestSweepPending_TargetDeleted(t *testing.T) {
last := tLastResult(t, s, deliveryID, 2) last := tLastResult(t, s, deliveryID, 2)
assert.Contains(t, last.Error, "gone-on-pending-sweep") assert.Contains(t, last.Error, "gone-while-pending-on-tick")
assert.Contains(t, last.Error, "was deleted") assert.Contains(t, last.Error, "was deleted")
} }
@@ -774,11 +775,11 @@ func TestSendRecoveredDeliveries_TargetMissingFromMap(
) )
} }
// TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone: a failed // TestRecoverPendingDeliveries_TargetUnreadable_LeavesDeliveryAlone: a
// read of the main database is not a deleted target. Restart recovery // failed read of the main database is not a deleted target. At start the
// holds every pending delivery of the webhook in one batch, so failing // recovery routine holds every pending delivery of the webhook in one
// on this would fail all of them. // batch, so failing on this would fail all of them.
func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone( func TestRecoverPendingDeliveries_TargetUnreadable_LeavesDeliveryAlone(
t *testing.T, t *testing.T,
) { ) {
t.Parallel() t.Parallel()
@@ -806,7 +807,7 @@ func TestRecoverPending_TargetUnreadable_LeavesDeliveryAlone(
require.NoError(t, sqlDB.Close()) require.NoError(t, sqlDB.Close())
s.Engine.ExportRecoverPendingDeliveries( s.Engine.ExportRecoverPendingDeliveries(
context.Background(), s.WebhookDB, s.WebhookID, context.Background(), s.WebhookDB, s.WebhookID, atStart,
) )
iAssertStatus( iAssertStatus(
+12 -95
View File
@@ -5,9 +5,7 @@ import (
"github.com/go-chi/chi" "github.com/go-chi/chi"
"gorm.io/gorm" "gorm.io/gorm"
"gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
) )
// The outcomes of a replay POST, as the notice codes its redirect // The outcomes of a replay POST, as the notice codes its redirect
@@ -46,12 +44,12 @@ const (
// HandleDeliveryReplay re-sends a finished delivery's event to its // HandleDeliveryReplay re-sends a finished delivery's event to its
// target. // target.
// //
// A replay never touches the delivery it repeats. It creates a NEW // A replay never touches the delivery it repeats. The delivery engine
// pending delivery row for the same event and target and hands it to // creates a NEW pending delivery row for the same event and target and
// the delivery engine through the same Notifier the receiver uses, so // queues it as it queues the receiver's, so the original's status,
// the original's status, attempts and timestamps stand as the record // attempts and timestamps stand as the record of what actually
// of what actually happened, and the replay is retried, SSRF-guarded // happened, and the replay is retried, SSRF-guarded and circuit-broken
// and circuit-broken exactly as a first attempt is. // exactly as a first attempt is.
// //
// What is re-sent is the stored EVENT body, never the response the // What is re-sent is the stored EVENT body, never the response the
// original delivery received. // original delivery received.
@@ -145,7 +143,8 @@ func (h *Handlers) loadReplaySource(
return &original, true return &original, true
} }
// queueReplay writes the new delivery and hands it to the engine. // queueReplay has the delivery engine create and queue the new
// delivery, unless an earlier replay is still in flight.
func (h *Handlers) queueReplay( func (h *Handlers) queueReplay(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
@@ -171,18 +170,8 @@ func (h *Handlers) queueReplay(
return return
} }
var event database.Event deliveryID, err := h.engine.CreateReplay(
webhook.ID, original.EventID, target,
err = webhookDB.
First(&event, "id = ?", original.EventID).Error
if err != nil {
h.serverError(w, r, "failed to load event for replay", err)
return
}
task, err := createReplayDelivery(
webhookDB, webhook.ID, &event, target,
) )
if err != nil { if err != nil {
h.serverError( h.serverError(
@@ -193,15 +182,14 @@ func (h *Handlers) queueReplay(
} }
h.mtr.DeliveryReplayed(target.Type) h.mtr.DeliveryReplayed(target.Type)
h.notifier.Notify([]delivery.Task{task})
h.log.Info( h.log.Info(
"delivery replay queued", "delivery replay queued",
"webhook_id", webhook.ID, "webhook_id", webhook.ID,
"event_id", event.ID, "event_id", original.EventID,
"target_id", target.ID, "target_id", target.ID,
"replayed_delivery_id", original.ID, "replayed_delivery_id", original.ID,
"delivery_id", task.DeliveryID, "delivery_id", deliveryID,
) )
redirectToEventLog(w, r, webhook, replayQueued) redirectToEventLog(w, r, webhook, replayQueued)
@@ -263,77 +251,6 @@ func countInFlightDeliveries(
return count, err return count, err
} }
// createReplayDelivery writes the new pending delivery row, adds it to
// its target's totals in the same transaction, and returns the task
// that carries it to the delivery engine.
//
// The row is written with associations omitted, and neither Event nor
// Target is populated on it: GORM's SaveBeforeAssociations would
// otherwise upsert the whole target row — plaintext config, which for a
// Slack target is the credential — into the per-webhook event database.
// See https://git.eeqj.de/sneak/webhooker/issues/206.
func createReplayDelivery(
webhookDB *gorm.DB,
webhookID string,
event *database.Event,
target *database.Target,
) (delivery.Task, error) {
dlv := &database.Delivery{
EventID: event.ID,
TargetID: target.ID,
Status: database.DeliveryStatusPending,
Replay: true,
}
err := webhookDB.Transaction(func(tx *gorm.DB) error {
err := tx.Omit(clause.Associations).Create(dlv).Error
if err != nil {
return err
}
return database.AddTargetTotals(tx, database.TargetTotals{
TargetID: dlv.TargetID, Deliveries: 1,
})
})
if err != nil {
return delivery.Task{}, err
}
return delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: webhookID,
EntrypointID: event.EntrypointID,
TargetID: target.ID,
TargetName: target.Name,
TargetType: target.Type,
TargetURL: target.URL,
TargetHeaders: target.Headers,
TimeoutSeconds: target.TimeoutSeconds,
ForwardQuery: target.ForwardQuery,
ArchiveExpiry: target.ArchiveExpiry,
ArchiveRotation: target.ArchiveRotation,
MaxAttempts: target.MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: replayBody(event.Body),
AttemptNum: 1,
}, nil
}
// replayBody returns the stored event body for a replay task to carry
// inline, or nil when it is large enough that the engine should fetch
// it from the per-webhook database instead.
func replayBody(body string) *string {
if len(body) >= delivery.MaxInlineBodySize {
return nil
}
return &body
}
// redirectToEventLog redirects a replay or resubmit back to the event // redirectToEventLog redirects a replay or resubmit back to the event
// log it was triggered from, carrying the outcome as its notice. A // log it was triggered from, carrying the outcome as its notice. A
// Replay form carries the list it was pressed in as show, so a replay // Replay form carries the list it was pressed in as show, so a replay
+48 -79
View File
@@ -12,7 +12,6 @@ import (
"gorm.io/gorm" "gorm.io/gorm"
"gorm.io/gorm/clause" "gorm.io/gorm/clause"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/handlers" "sneak.berlin/go/webhooker/internal/handlers"
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
@@ -23,7 +22,7 @@ const paramDeliveryID = "deliveryID"
// replayTargetURL is a public destination, so a target configured with // replayTargetURL is a public destination, so a target configured with
// it is one the SSRF guard would accept. Nothing in these tests // it is one the SSRF guard would accept. Nothing in these tests
// dispatches to it: the notifier is recorded, not run. // dispatches to it: the delivery engine is never started.
const replayTargetURL = "http://93.184.216.34/hook" const replayTargetURL = "http://93.184.216.34/hook"
// replayEventQuery is the query string a seeded event arrived with. // replayEventQuery is the query string a seeded event arrived with.
@@ -163,14 +162,8 @@ func postReplay(
// TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal is the // TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal is the
// core requirement: replaying a failed delivery succeeds, appends a // core requirement: replaying a failed delivery succeeds, appends a
// new delivery, and leaves the original row and its recorded attempt // new delivery marked as a replay, and leaves the original row and its
// exactly as they were. // recorded attempt exactly as they were.
//
// It also pins the two things a replay would be wrong to get from the
// original: the task carries the target's CURRENT configuration, which
// this test changes between the failure and the replay, and it carries
// the stored EVENT body rather than anything the failed attempt
// received back.
func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal( func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
t *testing.T, t *testing.T,
) { ) {
@@ -181,10 +174,9 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -204,15 +196,6 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
before := loadDelivery(t, webhookDB, original.ID) before := loadDelivery(t, webhookDB, original.ID)
// The operator fixes the destination, which is the whole reason
// to replay. The replay must use this, not the URL the original
// delivery ran against.
const fixedURL = "http://93.184.216.34/fixed"
require.NoError(t, db.DB().Model(&database.Target{}).
Where("id = ?", tgt.ID).
Update("url", fixedURL).Error)
w := postReplay(t, h, sess, wh.ID, original.ID) w := postReplay(t, h, sess, wh.ID, original.ID)
require.Equal(t, http.StatusSeeOther, w.Code) require.Equal(t, http.StatusSeeOther, w.Code)
@@ -234,14 +217,9 @@ func TestHandleDeliveryReplay_AppendsDeliveryAndLeavesOriginal(
assert.Equal( assert.Equal(
t, database.DeliveryStatusPending, replayed.Status, t, database.DeliveryStatusPending, replayed.Status,
) )
assert.True(t, replayed.Replay)
assertDeliveryUntouched(t, webhookDB, before) assertDeliveryUntouched(t, webhookDB, before)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assertReplayTask(
t, tasks[0], wh.ID, event, tgt, replayed.ID, fixedURL,
)
assertNoLeakedTarget(t, webhookDB) assertNoLeakedTarget(t, webhookDB)
} }
@@ -275,45 +253,6 @@ func assertDeliveryUntouched(
) )
} }
// assertReplayTask proves the task handed to the delivery engine is
// the one the receiver would build for this event and this target, and
// that it carries wantURL — the target's URL as it stands now rather
// than as the original delivery ran against it.
func assertReplayTask(
t *testing.T,
task delivery.Task,
webhookID string,
event *database.Event,
target *database.Target,
wantDeliveryID, wantURL string,
) {
t.Helper()
assert.Equal(t, wantDeliveryID, task.DeliveryID)
assert.Equal(t, event.ID, task.EventID)
assert.Equal(t, webhookID, task.WebhookID)
assert.Equal(t, event.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Type, task.TargetType)
assert.Equal(
t, wantURL, task.TargetURL,
"replay must use the target's current URL",
)
assert.Equal(t, event.Method, task.Method)
assert.Equal(
t, replayEventQuery, task.RawQuery,
"replay re-sends the stored query string",
)
assert.Equal(t, event.Headers, task.Headers)
assert.Equal(t, event.ContentType, task.ContentType)
assert.Equal(t, 1, task.AttemptNum)
require.NotNil(t, task.Body)
assert.Equal(
t, event.Body, *task.Body,
"replay re-sends the stored event body",
)
}
// assertNoLeakedTarget proves the per-webhook database holds no // assertNoLeakedTarget proves the per-webhook database holds no
// target: an association write would have upserted the whole target, // target: an association write would have upserted the whole target,
// plaintext config and all, into the event database. See // plaintext config and all, into the event database. See
@@ -327,9 +266,49 @@ func assertNoLeakedTarget(t *testing.T, webhookDB *gorm.DB) {
) )
} }
// TestHandleDeliveryReplay_FailedStoreIsAnError proves a replay the
// engine cannot store answers with the 500 error page and adds no
// delivery.
func TestHandleDeliveryReplay_FailedStoreIsAnError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
tgt := seedConfiguredTarget(
t, db, wh.ID, database.TargetTypeHTTP,
database.Target{URL: replayTargetURL},
)
event, original := seedFailedDelivery(t, dbMgr, wh.ID, tgt.ID)
// Without its target totals table the webhook's database refuses
// the totals write, which comes after the delivery row.
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.TargetTotals{}))
w := postReplay(t, h, sess, wh.ID, original.ID)
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Contains(t, w.Body.String(), "500 Internal Server Error")
assert.Len(t, listDeliveries(t, webhookDB, event.ID), 1)
}
// TestHandleDeliveryReplay_RefusesDeletedTarget proves the required // TestHandleDeliveryReplay_RefusesDeletedTarget proves the required
// refusal: a target deleted since the delivery ran is reported as // refusal: a target deleted since the delivery ran is reported as
// deleted rather than erroring, and nothing is created or queued. // deleted rather than erroring, and nothing is created.
func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) { func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
t.Parallel() t.Parallel()
@@ -338,10 +317,9 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -377,10 +355,6 @@ func TestHandleDeliveryReplay_RefusesDeletedTarget(t *testing.T) {
t, listDeliveries(t, webhookDB, event.ID), 1, t, listDeliveries(t, webhookDB, event.ID), 1,
"a refused replay must create no delivery", "a refused replay must create no delivery",
) )
assert.Empty(
t, notif.Tasks(),
"a refused replay must queue nothing",
)
// The refusal is specific, which is why the target is looked up // The refusal is specific, which is why the target is looked up
// including soft-deleted rows: an id that never named a target // including soft-deleted rows: an id that never named a target
@@ -413,10 +387,9 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -454,10 +427,6 @@ func TestHandleDeliveryReplay_RefusesWhileEarlierReplayInFlight(
t, listDeliveries(t, webhookDB, event.ID), 2, t, listDeliveries(t, webhookDB, event.ID), 2,
"the refused second replay must add nothing", "the refused second replay must add nothing",
) )
assert.Len(
t, notif.Tasks(), 1,
"only the first replay reaches the delivery engine",
)
// A delivery the engine has not finished is not replayable // A delivery the engine has not finished is not replayable
// either, which is the same rule seen from the other side. // either, which is the same rule seen from the other side.
+13 -21
View File
@@ -171,37 +171,29 @@ func loadResubmitSource(
return src, true, nil return src, true, nil
} }
// queueResubmit stores the copy and fans it out to the webhook's // queueResubmit has the delivery engine store the copy, with a delivery
// active targets. // for each of the webhook's active targets as they stand now, through
// the same StoreEvent the receiver uses. The one difference is that the
// copy leaves its entrypoint's last event as it is, since it did not
// arrive on that URL.
func (h *Handlers) queueResubmit( func (h *Handlers) queueResubmit(
w http.ResponseWriter, w http.ResponseWriter,
r *http.Request, r *http.Request,
webhook database.Webhook, webhook database.Webhook,
src resubmitSource, src resubmitSource,
) { ) {
// The receiver's own query, run now: an active target created event := &database.Event{
// after the original event arrived is included, and an
// inactive one is skipped rather than refused.
targets, err := h.loadActiveTargets(webhook.ID)
if err != nil {
h.serverError(w, r, "failed to query targets", err)
return
}
event, tasks, err := h.createAndFanOut(
eventSource{
WebhookID: webhook.ID, WebhookID: webhook.ID,
EntrypointID: src.EntrypointID, EntrypointID: src.EntrypointID,
Method: src.Method, Method: src.Method,
RawQuery: src.RawQuery, RawQuery: src.RawQuery,
HeadersJSON: src.Headers, Headers: src.Headers,
ContentType: src.ContentType, ContentType: src.ContentType,
Body: src.Body, Body: string(src.Body),
ResubmittedFromID: &src.ID, ResubmittedFromID: &src.ID,
}, }
targets,
) deliveries, err := h.engine.StoreEvent(event)
if err != nil { if err != nil {
h.serverError(w, r, "failed to store resubmitted event", err) h.serverError(w, r, "failed to store resubmitted event", err)
@@ -215,11 +207,11 @@ func (h *Handlers) queueResubmit(
"webhook_id", webhook.ID, "webhook_id", webhook.ID,
"event_id", event.ID, "event_id", event.ID,
"resubmitted_from_id", src.ID, "resubmitted_from_id", src.ID,
"target_count", len(tasks), "target_count", deliveries,
) )
code := resubmitQueued code := resubmitQueued
if len(tasks) == 0 { if deliveries == 0 {
code = resubmitNoTargets code = resubmitNoTargets
} }
+72 -93
View File
@@ -19,7 +19,7 @@ import (
// resubmitTargetURL is a public destination, so a target configured // resubmitTargetURL is a public destination, so a target configured
// with it is one the SSRF guard would accept. Nothing in these tests // with it is one the SSRF guard would accept. Nothing in these tests
// dispatches to it: the notifier is recorded, not run. // dispatches to it: the delivery engine is never started.
const resubmitTargetURL = "http://93.184.216.34/hook" const resubmitTargetURL = "http://93.184.216.34/hook"
// resubmitEventHeaders and resubmitEventQuery are the stored header // resubmitEventHeaders and resubmitEventQuery are the stored header
@@ -134,10 +134,9 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -189,11 +188,6 @@ func TestHandleEventResubmit_DeliversToTargetCreatedAfterTheEvent(
t, database.DeliveryStatusPending, deliveries[0].Status, t, database.DeliveryStatusPending, deliveries[0].Status,
) )
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Equal(t, deliveries[0].ID, tasks[0].DeliveryID)
assertResubmitTask(t, tasks[0], wh.ID, &fresh, tgt)
assertNoLeakedTarget(t, webhookDB) assertNoLeakedTarget(t, webhookDB)
} }
@@ -225,32 +219,6 @@ func assertEventCopy(
) )
} }
// assertResubmitTask proves the task handed to the delivery engine is
// the one the receiver would build for the NEW event and this target.
func assertResubmitTask(
t *testing.T,
task delivery.Task,
webhookID string,
fresh *database.Event,
target *database.Target,
) {
t.Helper()
assert.Equal(t, fresh.ID, task.EventID)
assert.Equal(t, webhookID, task.WebhookID)
assert.Equal(t, fresh.EntrypointID, task.EntrypointID)
assert.Equal(t, target.ID, task.TargetID)
assert.Equal(t, target.Type, task.TargetType)
assert.Equal(t, fresh.Method, task.Method)
assert.Equal(t, fresh.RawQuery, task.RawQuery)
assert.Equal(t, fresh.Headers, task.Headers)
assert.Equal(t, fresh.ContentType, task.ContentType)
assert.Equal(t, 1, task.AttemptNum)
require.NotNil(t, task.Body)
assert.Equal(t, fresh.Body, *task.Body)
}
// TestHandleEventResubmit_IsRepeatable proves the requirement replay // TestHandleEventResubmit_IsRepeatable proves the requirement replay
// deliberately does not meet: firing the same captured event at a // deliberately does not meet: firing the same captured event at a
// backend over and over must work, with no in-flight refusal, and each // backend over and over must work, with no in-flight refusal, and each
@@ -263,10 +231,9 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -300,30 +267,18 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
events := listEvents(t, webhookDB) events := listEvents(t, webhookDB)
require.Len(t, events, presses+1) require.Len(
t, events, presses+1, "each resubmit creates its own event",
)
tasks := notif.Tasks() assert.Empty(
require.Len(t, tasks, presses) t, listDeliveries(t, webhookDB, original.ID),
seen := make(map[string]struct{}, presses)
for _, task := range tasks {
assert.Equal(t, tgt.ID, task.TargetID)
assert.NotEqual(
t, original.ID, task.EventID,
"each resubmit delivers its own new event", "each resubmit delivers its own new event",
) )
_, dup := seen[task.EventID] // Every copy has its own delivery, and names the same source
assert.False(t, dup, "each resubmit creates its own event") // event, so twenty presses stay traceable to the one captured
// request.
seen[task.EventID] = struct{}{}
require.Len(t, listDeliveries(t, webhookDB, task.EventID), 1)
}
// Every copy names the same source event, so twenty presses stay
// traceable to the one captured request.
for _, e := range events { for _, e := range events {
if e.ID == original.ID { if e.ID == original.ID {
continue continue
@@ -331,6 +286,10 @@ func TestHandleEventResubmit_IsRepeatable(t *testing.T) {
require.NotNil(t, e.ResubmittedFromID) require.NotNil(t, e.ResubmittedFromID)
assert.Equal(t, original.ID, *e.ResubmittedFromID) assert.Equal(t, original.ID, *e.ResubmittedFromID)
deliveries := listDeliveries(t, webhookDB, e.ID)
require.Len(t, deliveries, 1)
assert.Equal(t, tgt.ID, deliveries[0].TargetID)
} }
} }
@@ -346,10 +305,9 @@ func TestHandleEventResubmit_OversizeBodySurvivesIntact(t *testing.T) {
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -385,22 +343,7 @@ func TestHandleEventResubmit_OversizeBodySurvivesIntact(t *testing.T) {
t, stored, fresh.Body, t, stored, fresh.Body,
"the stored body must be copied byte for byte", "the stored body must be copied byte for byte",
) )
assert.Len(t, listDeliveries(t, webhookDB, fresh.ID), 1)
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Nil(
t, tasks[0].Body,
"a body over the inline limit is fetched from the new "+
"event row rather than carried on the task",
)
// The engine's own read of the body, against the new event id:
// what it would send is what was stored.
var loaded database.Event
require.NoError(t, webhookDB.Select("body").
First(&loaded, "id = ?", tasks[0].EventID).Error)
assert.Equal(t, stored, loaded.Body)
} }
// TestHandleEventResubmit_SkipsInactiveTarget proves a deactivated // TestHandleEventResubmit_SkipsInactiveTarget proves a deactivated
@@ -414,10 +357,9 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -448,21 +390,19 @@ func TestHandleEventResubmit_SkipsInactiveTarget(t *testing.T) {
"an inactive target is skipped, not an error", "an inactive target is skipped, not an error",
) )
tasks := notif.Tasks()
require.Len(t, tasks, 1)
assert.Equal(t, active.ID, tasks[0].TargetID)
webhookDB, err := dbMgr.GetDB(wh.ID) webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err) require.NoError(t, err)
fresh := theOtherEvent(t, listEvents(t, webhookDB), original.ID) fresh := theOtherEvent(t, listEvents(t, webhookDB), original.ID)
require.Len(t, listDeliveries(t, webhookDB, fresh.ID), 1) deliveries := listDeliveries(t, webhookDB, fresh.ID)
require.Len(t, deliveries, 1)
assert.Equal(t, active.ID, deliveries[0].TargetID)
} }
// TestHandleEventResubmit_NoActiveTargetsStillStoresEvent proves a // TestHandleEventResubmit_NoActiveTargetsStillStoresEvent proves a
// source with nothing to deliver to behaves as the receiver does: the // source with nothing to deliver to behaves as the receiver does: the
// event is stored, nothing is queued, and the operator is told so // event is stored with no delivery, and the operator is told so rather
// rather than being shown an error. // than being shown an error.
func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent( func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
t *testing.T, t *testing.T,
) { ) {
@@ -473,10 +413,9 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -493,12 +432,14 @@ func TestHandleEventResubmit_NoActiveTargetsStillStoresEvent(
w.Header().Get("Location"), w.Header().Get("Location"),
) )
assert.Empty(t, notif.Tasks())
webhookDB, err := dbMgr.GetDB(wh.ID) webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err) require.NoError(t, err)
assert.Len(t, listEvents(t, webhookDB), 2) events := listEvents(t, webhookDB)
assert.Len(t, events, 2)
fresh := theOtherEvent(t, events, original.ID)
assert.Empty(t, listDeliveries(t, webhookDB, fresh.ID))
} }
// TestHandleEventResubmit_RefusesEventOfAnotherWebhook proves the // TestHandleEventResubmit_RefusesEventOfAnotherWebhook proves the
@@ -514,10 +455,9 @@ func TestHandleEventResubmit_RefusesEventOfAnotherWebhook(
sess *session.Session sess *session.Session
db *database.Database db *database.Database
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
notif *recordingNotifier
) )
app := newTestApp(t, &h, &sess, &db, &dbMgr, &notif) app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart() app.RequireStart()
t.Cleanup(app.RequireStop) t.Cleanup(app.RequireStop)
@@ -548,11 +488,50 @@ func TestHandleEventResubmit_RefusesEventOfAnotherWebhook(
w = postResubmit(t, h, sess, mine.ID, "not-a-uuid") w = postResubmit(t, h, sess, mine.ID, "not-a-uuid")
assert.Equal(t, http.StatusNotFound, w.Code) assert.Equal(t, http.StatusNotFound, w.Code)
assert.Empty( for _, webhookID := range []string{theirs.ID, mine.ID} {
t, notif.Tasks(), webhookDB, err := dbMgr.GetDB(webhookID)
"a refused resubmit must queue nothing", require.NoError(t, err)
assert.Len(
t, listEvents(t, webhookDB), 1,
"a refused resubmit must store nothing",
) )
} }
}
// TestHandleEventResubmit_FailedStoreIsAnError proves a copy the engine
// cannot store answers with the 500 error page and leaves no new event.
func TestHandleEventResubmit_FailedStoreIsAnError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
sess *session.Session
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &sess, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
wh := seedWebhook(t, db)
original := seedStoredEvent(t, dbMgr, wh.ID, `{"not":"stored"}`)
// Without its event totals table the webhook's database refuses
// the totals write, which comes after the event row.
webhookDB, err := dbMgr.GetDB(wh.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.EventTotals{}))
w := postResubmit(t, h, sess, wh.ID, original.ID)
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Contains(t, w.Body.String(), "500 Internal Server Error")
assert.Len(t, listEvents(t, webhookDB), 1)
}
// TestHandleSourceLogs_ShowsResubmitProvenance proves the event log // TestHandleSourceLogs_ShowsResubmitProvenance proves the event log
// reports the relationship in both directions, which is what keeps it // reports the relationship in both directions, which is what keeps it
+3 -3
View File
@@ -64,7 +64,7 @@ type HandlersParams struct {
Healthcheck *healthcheck.Healthcheck Healthcheck *healthcheck.Healthcheck
Session *session.Session Session *session.Session
Middleware *middleware.Middleware Middleware *middleware.Middleware
Notifier delivery.Notifier Engine *delivery.Engine
Archives archive.Archives Archives archive.Archives
CircuitBreakers delivery.CircuitBreakers CircuitBreakers delivery.CircuitBreakers
SSRFGuard *delivery.Guard SSRFGuard *delivery.Guard
@@ -82,7 +82,7 @@ type Handlers struct {
dbMgr *database.WebhookDBManager dbMgr *database.WebhookDBManager
session *session.Session session *session.Session
mw *middleware.Middleware mw *middleware.Middleware
notifier delivery.Notifier engine *delivery.Engine
archives archive.Archives archives archive.Archives
breakers delivery.CircuitBreakers breakers delivery.CircuitBreakers
mtr *metrics.Set mtr *metrics.Set
@@ -138,7 +138,7 @@ func New(
s.dbMgr = params.WebhookDBMgr s.dbMgr = params.WebhookDBMgr
s.session = params.Session s.session = params.Session
s.mw = params.Middleware s.mw = params.Middleware
s.notifier = params.Notifier s.engine = params.Engine
s.archives = params.Archives s.archives = params.Archives
s.breakers = params.CircuitBreakers s.breakers = params.CircuitBreakers
s.mtr = params.Metrics s.mtr = params.Metrics
+6 -32
View File
@@ -30,33 +30,6 @@ import (
"sneak.berlin/go/webhooker/internal/session" "sneak.berlin/go/webhooker/internal/session"
) )
// recordingNotifier is a delivery.Notifier that records the tasks it
// was handed, so a test can prove a handler queued the delivery it
// claims to have queued — and, on the refusal paths, that it queued
// nothing.
type recordingNotifier struct {
mu sync.Mutex
tasks []delivery.Task
}
func (n *recordingNotifier) Notify(tasks []delivery.Task) {
n.mu.Lock()
defer n.mu.Unlock()
n.tasks = append(n.tasks, tasks...)
}
// Tasks returns a copy of the recorded tasks.
func (n *recordingNotifier) Tasks() []delivery.Task {
n.mu.Lock()
defer n.mu.Unlock()
out := make([]delivery.Task, len(n.tasks))
copy(out, n.tasks)
return out
}
// recordingArchives is an archive.Archives that records what it // recordingArchives is an archive.Archives that records what it
// was asked to do, so a test can prove that a deletion or rename // was asked to do, so a test can prove that a deletion or rename
// path reached the archive writers. After FailRenames, every // path reached the archive writers. After FailRenames, every
@@ -264,11 +237,12 @@ func newTestAppWithConfig(
database.NewWebhookDBManager, database.NewWebhookDBManager,
healthcheck.New, healthcheck.New,
session.New, session.New,
func() *recordingNotifier { archive.NewWriters,
return &recordingNotifier{} // The engine is built on a lifecycle that is never started,
}, // so no worker runs: every delivery the engine creates for a
func(n *recordingNotifier) delivery.Notifier { // handler stays pending, where the test can read it.
return n func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
}, },
func() *recordingArchives { func() *recordingArchives {
return &recordingArchives{} return &recordingArchives{}
+20 -264
View File
@@ -2,14 +2,11 @@ package handlers
import ( import (
"encoding/json" "encoding/json"
"fmt"
"io" "io"
"net/http" "net/http"
"github.com/go-chi/chi" "github.com/go-chi/chi"
"gorm.io/gorm"
"sneak.berlin/go/webhooker/internal/database" "sneak.berlin/go/webhooker/internal/database"
"sneak.berlin/go/webhooker/internal/delivery"
"sneak.berlin/go/webhooker/internal/middleware" "sneak.berlin/go/webhooker/internal/middleware"
) )
@@ -70,8 +67,9 @@ func (h *Handlers) HandleWebhook() http.HandlerFunc {
} }
} }
// processWebhookRequest reads the body, serializes headers, loads // processWebhookRequest reads the request into an event and has the
// targets, and delivers the event. // delivery engine store it, with a delivery for each of the webhook's
// active targets, and answers the sender.
// //
// Nothing about the request itself is authenticated: the entrypoint // Nothing about the request itself is authenticated: the entrypoint
// UUID in the path is the credential, and reaching here means it // UUID in the path is the credential, and reaching here means it
@@ -93,30 +91,24 @@ func (h *Handlers) processWebhookRequest(
return return
} }
targets, err := h.loadActiveTargets(entrypoint.WebhookID) event := &database.Event{
WebhookID: entrypoint.WebhookID,
EntrypointID: entrypoint.ID,
Method: r.Method,
RawQuery: r.URL.RawQuery,
Headers: string(headersJSON),
ContentType: r.Header.Get("Content-Type"),
Body: string(body),
}
deliveries, err := h.engine.StoreEvent(event)
if err != nil { if err != nil {
h.receiverError(w, "failed to query targets", err) h.receiverError(w, "failed to store webhook event", err)
return return
} }
h.createAndDeliverEvent( h.finishWebhookResponse(w, event, entrypoint, deliveries)
w, r, entrypoint, body, headersJSON, targets,
)
}
// loadActiveTargets returns all active targets for a webhook.
func (h *Handlers) loadActiveTargets(
webhookID string,
) ([]database.Target, error) {
var targets []database.Target
err := h.db.DB().Where(
"webhook_id = ? AND active = ?",
webhookID, true,
).Find(&targets).Error
return targets, err
} }
// lookupEntrypoint finds an entrypoint by UUID path. // lookupEntrypoint finds an entrypoint by UUID path.
@@ -177,28 +169,6 @@ func (h *Handlers) readWebhookBody(
return body, true return body, true
} }
// createAndDeliverEvent stores the received event, fans it out to the
// webhook's targets, and answers the sender.
func (h *Handlers) createAndDeliverEvent(
w http.ResponseWriter,
r *http.Request,
entrypoint database.Entrypoint,
body, headersJSON []byte,
targets []database.Target,
) {
event, tasks, err := h.createAndFanOut(
requestEventSource(r, entrypoint, headersJSON, body),
targets,
)
if err != nil {
h.receiverError(w, "failed to store webhook event", err)
return
}
h.finishWebhookResponse(w, event, entrypoint, tasks)
}
// receiverError logs an error and answers the sender with a plain-text // receiverError logs an error and answers the sender with a plain-text
// 500. The receiver's answers are for programs, so it never sends the // 500. The receiver's answers are for programs, so it never sends the
// error page the web UI uses. // error page the web UI uses.
@@ -212,172 +182,19 @@ func (h *Handlers) receiverError(
) )
} }
// eventSource carries the fields a new event is built from. The // finishWebhookResponse logs the stored event, with how many
// receiver fills it from the live request; the resubmit handler fills // deliveries it has, and writes the HTTP response.
// it from a stored event. Both then go through createAndFanOut, so an
// event is constructed and fanned out in one place however it entered
// the system.
type eventSource struct {
WebhookID string
EntrypointID string
Method string
RawQuery string
HeadersJSON string
ContentType string
Body []byte
// ResubmittedFromID names the event this one copies. Only the
// resubmit path sets it.
ResubmittedFromID *string
}
// event builds the row this source stores.
func (s eventSource) event() *database.Event {
return &database.Event{
WebhookID: s.WebhookID,
EntrypointID: s.EntrypointID,
Method: s.Method,
RawQuery: s.RawQuery,
Headers: s.HeadersJSON,
Body: string(s.Body),
BodyBytes: int64(len(s.Body)),
ContentType: s.ContentType,
ResubmittedFromID: s.ResubmittedFromID,
}
}
// requestEventSource describes the event a live receiver request
// stores.
func requestEventSource(
r *http.Request,
entrypoint database.Entrypoint,
headersJSON, body []byte,
) eventSource {
return eventSource{
WebhookID: entrypoint.WebhookID,
EntrypointID: entrypoint.ID,
Method: r.Method,
RawQuery: r.URL.RawQuery,
HeadersJSON: string(headersJSON),
ContentType: r.Header.Get("Content-Type"),
Body: body,
}
}
// createAndFanOut writes the event and one pending delivery per target,
// and adds them to the webhook's running totals, in a single
// transaction, then hands the tasks to the delivery engine. Every
// event is created here, received or resubmitted, so a resubmitted
// event is retried, SSRF-guarded and circuit-broken exactly as a
// received one is. Per-delivery replay is the one other path that
// creates a delivery: it adds one to an existing event without
// coming through here.
//
// The tasks are returned as well as queued, so a caller can report how
// many targets the event went to.
func (h *Handlers) createAndFanOut(
src eventSource,
targets []database.Target,
) (*database.Event, []delivery.Task, error) {
webhookDB, err := h.dbMgr.GetDB(src.WebhookID)
if err != nil {
return nil, nil, fmt.Errorf(
"getting webhook database: %w", err,
)
}
tx := webhookDB.Begin()
if tx.Error != nil {
return nil, nil, fmt.Errorf(
"beginning transaction: %w", tx.Error,
)
}
event := src.event()
err = tx.Create(event).Error
if err != nil {
tx.Rollback()
return nil, nil, fmt.Errorf("creating event: %w", err)
}
tasks, err := buildDeliveryTasks(
tx, event, targets, inlineBody(src.Body),
)
if err != nil {
tx.Rollback()
return nil, nil, err
}
err = database.AddEventTotals(tx, database.EventTotals{
Events: 1, LastEventAt: &event.CreatedAt,
})
if err != nil {
tx.Rollback()
return nil, nil, err
}
// A resubmitted copy did not arrive on its entrypoint's URL, so it
// leaves the entrypoint's last event as it is.
if src.ResubmittedFromID == nil {
err = database.AddEntrypointTotals(tx, database.EntrypointTotals{
EntrypointID: event.EntrypointID, LastEventAt: event.CreatedAt,
})
if err != nil {
tx.Rollback()
return nil, nil, err
}
}
err = tx.Commit().Error
if err != nil {
return nil, nil, fmt.Errorf(
"committing transaction: %w", err,
)
}
// Counted here, after the commit: an event exists once it is
// durably stored, which is what the delivery counters are
// compared against on a dashboard. A resubmitted event counts
// too, because it produces deliveries that the delivery side
// counts; the resubmit counter is what separates the two.
h.mtr.EventReceived()
if len(tasks) > 0 {
h.notifier.Notify(tasks)
}
return event, tasks, nil
}
// inlineBody returns a pointer to body as a string if it fits
// within the inline size limit, or nil otherwise.
func inlineBody(body []byte) *string {
if len(body) < delivery.MaxInlineBodySize {
s := string(body)
return &s
}
return nil
}
// finishWebhookResponse logs the event and writes the HTTP response.
func (h *Handlers) finishWebhookResponse( func (h *Handlers) finishWebhookResponse(
w http.ResponseWriter, w http.ResponseWriter,
event *database.Event, event *database.Event,
entrypoint database.Entrypoint, entrypoint database.Entrypoint,
tasks []delivery.Task, deliveries int,
) { ) {
h.log.Info("webhook event created", h.log.Info("webhook event created",
"event_id", event.ID, "event_id", event.ID,
"webhook_id", entrypoint.WebhookID, "webhook_id", entrypoint.WebhookID,
"entrypoint_id", entrypoint.ID, "entrypoint_id", entrypoint.ID,
"target_count", len(tasks), "target_count", deliveries,
) )
w.WriteHeader(http.StatusOK) w.WriteHeader(http.StatusOK)
@@ -389,64 +206,3 @@ func (h *Handlers) finishWebhookResponse(
) )
} }
} }
// buildDeliveryTasks creates one pending delivery per target in the
// transaction, adds each to its target's totals, and returns the tasks
// for the delivery engine. The caller owns the transaction and rolls
// it back on error.
func buildDeliveryTasks(
tx *gorm.DB,
event *database.Event,
targets []database.Target,
bodyPtr *string,
) ([]delivery.Task, error) {
tasks := make([]delivery.Task, 0, len(targets))
for i := range targets {
dlv := &database.Delivery{
EventID: event.ID,
TargetID: targets[i].ID,
Status: database.DeliveryStatusPending,
}
err := tx.Create(dlv).Error
if err != nil {
return nil, fmt.Errorf(
"creating delivery for target %s: %w",
targets[i].ID, err,
)
}
err = database.AddTargetTotals(tx, database.TargetTotals{
TargetID: targets[i].ID, Deliveries: 1,
})
if err != nil {
return nil, err
}
tasks = append(tasks, delivery.Task{
DeliveryID: dlv.ID,
EventID: event.ID,
WebhookID: event.WebhookID,
EntrypointID: event.EntrypointID,
TargetID: targets[i].ID,
TargetName: targets[i].Name,
TargetType: targets[i].Type,
TargetURL: targets[i].URL,
TargetHeaders: targets[i].Headers,
TimeoutSeconds: targets[i].TimeoutSeconds,
ForwardQuery: targets[i].ForwardQuery,
ArchiveExpiry: targets[i].ArchiveExpiry,
ArchiveRotation: targets[i].ArchiveRotation,
MaxAttempts: targets[i].MaxAttempts,
Method: event.Method,
RawQuery: event.RawQuery,
Headers: event.Headers,
ContentType: event.ContentType,
Body: bodyPtr,
AttemptNum: 1,
})
}
return tasks, nil
}
+84
View File
@@ -206,3 +206,87 @@ func receivedLine(t *testing.T, buf *bytes.Buffer) map[string]any {
return found[0] return found[0]
} }
// TestHandleWebhook_StoresTheEvent proves an accepted request is
// answered with the receiver's 200 and body, and that the event is
// stored with a pending delivery to the webhook's active target.
func TestHandleWebhook_StoresTheEvent(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := seedWebhook(t, db)
entrypoint := seedEntrypoint(t, db, webhook.ID)
target := seedConfiguredTarget(
t, db, webhook.ID, database.TargetTypeHTTP,
database.Target{URL: replayTargetURL},
)
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, postRequest(
"/h/"+entrypoint.Path, nil,
map[string]string{"uuid": entrypoint.Path},
))
require.Equal(t, http.StatusOK, w.Code)
assert.JSONEq(t, `{"status":"ok"}`, w.Body.String())
webhookDB, err := dbMgr.GetDB(webhook.ID)
require.NoError(t, err)
events := listEvents(t, webhookDB)
require.Len(t, events, 1)
assert.Equal(t, entrypoint.ID, events[0].EntrypointID)
deliveries := listDeliveries(t, webhookDB, events[0].ID)
require.Len(t, deliveries, 1)
assert.Equal(t, target.ID, deliveries[0].TargetID)
assert.Equal(t, database.DeliveryStatusPending, deliveries[0].Status)
}
// TestHandleWebhook_FailedStoreAnswersPlainError proves that an event
// the engine cannot store is answered with the receiver's plain-text
// 500, and that nothing of it is left stored.
func TestHandleWebhook_FailedStoreAnswersPlainError(t *testing.T) {
t.Parallel()
var (
h *handlers.Handlers
db *database.Database
dbMgr *database.WebhookDBManager
)
app := newTestApp(t, &h, &db, &dbMgr)
app.RequireStart()
t.Cleanup(app.RequireStop)
webhook := seedWebhook(t, db)
entrypoint := seedEntrypoint(t, db, webhook.ID)
// Without its event totals table the webhook's database refuses
// the totals write, which comes after the event row.
webhookDB, err := dbMgr.GetDB(webhook.ID)
require.NoError(t, err)
require.NoError(t,
webhookDB.Migrator().DropTable(&database.EventTotals{}))
w := httptest.NewRecorder()
h.HandleWebhook().ServeHTTP(w, postRequest(
"/h/"+entrypoint.Path, nil,
map[string]string{"uuid": entrypoint.Path},
))
assert.Equal(t, http.StatusInternalServerError, w.Code)
assert.Equal(t, "Internal server error\n", w.Body.String())
assert.Empty(t, listEvents(t, webhookDB))
}
+7 -6
View File
@@ -21,8 +21,9 @@ const budget = 64
// batchRunes is how many consecutive code points the charge test logs // batchRunes is how many consecutive code points the charge test logs
// in one value from U+1000 up. Logging each of those on its own line // in one value from U+1000 up. Logging each of those on its own line
// is too slow for the suite under the race detector; 4,096 at a time // was too slow for the suite when it ran under the race detector;
// is 271 batches, each logged on two lines, so 542 lines per handler. // 4,096 at a time is 271 batches, each logged on two lines, so 542
// lines per handler.
const batchRunes = 4096 const batchRunes = 4096
// newHandlers are the two handlers internal/logger can install. Time // newHandlers are the two handlers internal/logger can install. Time
@@ -152,10 +153,10 @@ func TestEncodedBytes_ChargesAtLeastWhatTheHandlersEmit(t *testing.T) {
batches = append(batches, batch.String()) batches = append(batches, batch.String())
} }
// What EncodedBytes charges for each batch. Under -race -cover this // What EncodedBytes charges for each batch. When the suite ran
// takes longer than logging the batches, so it is worked out once, // under -race -cover this took longer than logging the batches, so
// by whichever handler finishes logging first, while the other is // it is worked out once, by whichever handler finishes logging
// still logging. // first, while the other is still logging.
charged := sync.OnceValue(func() []int { charged := sync.OnceValue(func() []int {
costs := make([]int, len(batches)) costs := make([]int, len(batches))
+6 -5
View File
@@ -131,10 +131,6 @@ func run(
return code, stdout.String(), stderr.String() return code, stdout.String(), stderr.String()
} }
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
type noopArchives struct { type noopArchives struct {
renameMu sync.Mutex renameMu sync.Mutex
} }
@@ -190,7 +186,12 @@ func newServerApp(
database.NewWebhookDBManager, database.NewWebhookDBManager,
healthcheck.New, healthcheck.New,
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, archive.NewWriters,
// The engine is built on a lifecycle that is never started,
// so no worker runs.
func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
},
func() archive.Archives { return &noopArchives{} }, func() archive.Archives { return &noopArchives{} },
func() delivery.CircuitBreakers { func() delivery.CircuitBreakers {
return &noopCircuitBreakers{} return &noopCircuitBreakers{}
+7 -8
View File
@@ -10,14 +10,12 @@ import (
"sneak.berlin/go/webhooker/internal/server" "sneak.berlin/go/webhooker/internal/server"
) )
// earlyStopIterations is how many start/stop cycles the race test // earlyStopIterations is how many start/stop cycles the test below
// runs. The window it aims at is the gap between the OnStart hook // runs. The window it aims at is the gap between the OnStart hook
// returning and the serving goroutine reaching its first field // returning and the serving goroutine reaching its first field
// access, which is microseconds wide. The race detector reports an // access, which is microseconds wide, so a single cycle can miss it
// unsynchronised pair whenever it observes one, but it has to observe // purely on scheduling. Repetition makes hitting it reliable; the
// one, so a single cycle can miss purely on scheduling. Repetition // collaborators are built once, so the cycles themselves are cheap.
// makes the observation reliable; the collaborators are built once,
// so the cycles themselves are cheap.
const earlyStopIterations = 25 const earlyStopIterations = 25
// TestEarlyShutdown_NoPanicAndNoRace stops the application // TestEarlyShutdown_NoPanicAndNoRace stops the application
@@ -38,8 +36,9 @@ const earlyStopIterations = 25
// //
// httpServer is now built in New, on the constructing goroutine, so // httpServer is now built in New, on the constructing goroutine, so
// it is written before any hook exists and can never be nil. // it is written before any hook exists and can never be nil.
// sentryEnabled is atomic. This test is what catches either one // sentryEnabled is atomic. This test catches the nil dereference
// coming back — under -race, which is how the suite runs. // coming back, as a panic. The suite runs without Go's race
// detector, so the data race coming back would go unreported here.
func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) { func TestEarlyShutdown_NoPanicAndNoRace(t *testing.T) {
t.Parallel() t.Parallel()
+6 -5
View File
@@ -48,10 +48,6 @@ const (
metricsAuthValue = "s3cret" metricsAuthValue = "s3cret"
) )
type noopNotifier struct{}
func (n *noopNotifier) Notify([]delivery.Task) {}
// noopArchives satisfies handlers.New's archive.Archives // noopArchives satisfies handlers.New's archive.Archives
// dependency. No test here checks what gets evicted or renamed, so // dependency. No test here checks what gets evicted or renamed, so
// it records nothing. Its rename lock is a plain mutex. // it records nothing. Its rename lock is a plain mutex.
@@ -145,7 +141,12 @@ func newTestEnvWithConfig(
database.NewWebhookDBManager, database.NewWebhookDBManager,
healthcheck.New, healthcheck.New,
session.New, session.New,
func() delivery.Notifier { return &noopNotifier{} }, archive.NewWriters,
// The engine is built on a lifecycle that is never started,
// so no worker runs and nothing is sent anywhere.
func(p delivery.EngineParams) *delivery.Engine {
return delivery.New(fxtest.NewLifecycle(t), p)
},
func() archive.Archives { return &noopArchives{} }, func() archive.Archives { return &noopArchives{} },
func() delivery.CircuitBreakers { func() delivery.CircuitBreakers {
return &noopCircuitBreakers{} return &noopCircuitBreakers{}