Leave a delivery cut off by a stop for the next start (closes #527)
check / check (push) Waiting to run
check / check (push) Waiting to run
Stopping the engine cancels the context every http and slack request is sent with. An attempt cut off that way was recorded as failed, which for a target with no retries, or on a last retry, ended the delivery failed and nothing sent it again. The shared retry core now records nothing for a failed attempt when the engine's context is cancelled: no result, no circuit breaker outcome, no status change. The delivery stays pending or retrying and startup recovery sends it again. A target's own timeout is still a failed attempt. The README's Graceful shutdown bullet, Shutdown section and the at-least-once note say so. Model: opus-5-5
This commit is contained in:
@@ -1325,9 +1325,11 @@ webhooker solves this by acting as a durable intermediary:
|
||||
deliberately left in a recoverable state rather than marked done — losing a
|
||||
delivery is the worse failure — so the pending sweep picks it up about
|
||||
fifteen minutes later, or the next restart does, and the target receives a
|
||||
payload it already got. webhooker adds no delivery identifier of its own to
|
||||
an outbound request, so **make your receiver idempotent** against whatever
|
||||
the payload itself carries.
|
||||
payload it already got. A send still in flight when the service stops is
|
||||
left the same way: its attempt is not recorded, and the next start sends it
|
||||
again (see Shutdown). webhooker adds no delivery identifier of its own to an
|
||||
outbound request, so **make your receiver idempotent** against whatever the
|
||||
payload itself carries.
|
||||
|
||||
3. **Observability** — Full request/response logging for every webhook received
|
||||
and every delivery attempted. Prometheus metrics expose volume, latency, and
|
||||
@@ -1768,7 +1770,7 @@ is what bounds it.
|
||||
#### DeliveryResult
|
||||
|
||||
The result of a single delivery attempt. Every attempt (including retries) is
|
||||
individually logged for full observability.
|
||||
individually logged, except one cut off by a stop (see Shutdown).
|
||||
|
||||
| Field | Type | Description |
|
||||
| --------------- | ------- | ----------------------------------------- |
|
||||
@@ -2206,12 +2208,14 @@ This means:
|
||||
buffer rather than spawning more goroutines.
|
||||
- **Independent results** — each worker records its own delivery result in the
|
||||
per-webhook database without coordination.
|
||||
- **Graceful shutdown** — cancel the context, workers finish their current task
|
||||
and exit. The stop hook waits for the pool via `lifecycle.WaitForShutdown`,
|
||||
which bounds that wait by fx's stop timeout rather than blocking forever on a
|
||||
wedged worker. On timeout it logs at `ERROR` and returns an error, and the
|
||||
goroutines that did not finish are still running — an unclean shutdown is
|
||||
reported rather than hidden.
|
||||
- **Graceful shutdown** — the stop hook cancels the workers' context, and each
|
||||
worker exits once its current task returns. Cancelling cuts off an `http` or
|
||||
`slack` request still in flight; that attempt is not recorded, and the next
|
||||
start sends the delivery again (see Shutdown). The stop hook waits for the
|
||||
pool via `lifecycle.WaitForShutdown`, which bounds that wait by fx's stop
|
||||
timeout rather than blocking forever on a wedged worker. On timeout it logs at
|
||||
`ERROR` and returns an error, and the goroutines that did not finish are still
|
||||
running — an unclean shutdown is reported rather than hidden.
|
||||
|
||||
**Recovery paths:**
|
||||
|
||||
@@ -3284,7 +3288,8 @@ stop-hook log:
|
||||
3. `server` — the HTTP drain, bounded by `server.ShutdownTimeout` (**3
|
||||
seconds**) and by what the hooks before it left, then a Sentry flush if
|
||||
`SENTRY_DSN` is set
|
||||
4. `delivery.Engine` — waits for its workers, then closes the archive databases
|
||||
4. `delivery.Engine` — cuts off any `http` or `slack` request in flight, waits
|
||||
for its workers, then closes the archive databases
|
||||
5. `healthcheck`
|
||||
6. `WebhookDBManager`
|
||||
7. the database close
|
||||
@@ -3295,6 +3300,12 @@ by the stop context, so a wedge there consumes the 5 seconds before the HTTP
|
||||
server hook is ever entered. The hooks after the server are microsecond-scale in
|
||||
normal operation.
|
||||
|
||||
The delivery engine does not wait for a delivery request in flight: it cancels
|
||||
it. That attempt is not recorded and the delivery keeps its `pending` or
|
||||
`retrying` status, so startup recovery sends it again on the next start (see
|
||||
Recovery paths under Bounded Worker Pool). If the receiver already had it, it
|
||||
receives it twice: the at-least-once duplicate described under Rationale.
|
||||
|
||||
The HTTP drain budget is deliberately **shorter** than the sequence budget. Were
|
||||
the two equal, a drain that used its whole budget would exhaust the sequence
|
||||
budget at the instant it finished, and every later hook — the delivery engine,
|
||||
|
||||
Reference in New Issue
Block a user