What is wrong. On SIGTERM the delivery engine's stop hook (internal/delivery/engine.go, Engine.stop) cancels the worker context first and only then waits for the pool. Every outbound request is built with that context (httpTarget.doHTTPRequest, slackTarget.attempt in internal/delivery/target_http.go and target_slack.go), so an attempt that is on the wire when the stop begins is aborted at once with context canceled, and the retry core (httpCore.fireAndForget / withRetry and handleRetry) records it as a failed attempt. For a target with max_retries 0, which is the default on both target forms, that was the delivery's only attempt, so the delivery ends failed and nothing ever sends it. For a target with retries, a cancellation on its last attempt ends it failed the same way; on an earlier attempt the row goes back to retrying and restart recovery re-sends it, which is fine.
How it was observed. On the image built from v1.0.0-rc1: an http target with max_retries 0 pointed at a receiver that answers after 10 s; an event was accepted and docker stop issued 2 s later. The stop completed in 0.7 s. The delivery row read failed with finished_at set and one attempt whose error is sending request: Post "http://…/...": context canceled; the receiver had received the request. The same happened to the last attempt of a target with max_retries 2 and a 1 s timeout. After the restart nothing recovered either delivery, since failed is terminal.
Why it matters. Every deploy, image update or host reboot during traffic turns the deliveries in flight at that moment into terminal failures that the service will not retry, on the default target configuration. The README's "Bounded Worker Pool" section promises "Graceful shutdown — cancel the context, workers finish their current task and exit", and the "Shutdown" section gives the HTTP drain 3 s inside a 5 s budget; the engine spends none of that budget on its own in-flight requests. The operator can see failed with context canceled in the event log and Replay by hand, but nothing marks these failures as caused by the restart.
What acceptable looks like. Either (a) in-flight attempts are allowed to finish within the stop budget: the request context is bounded by the stop deadline instead of being cancelled when the hook starts; or (b) a cancellation caused by shutdown is not counted as an attempt: the delivery is left pending or retrying so recovery re-sends it on the next start (an at-least-once duplicate, which the README already accepts under Rationale). Whichever is chosen, the README says so.
Definition of done. The chosen behaviour is implemented; a test stops an engine while a fire-and-forget attempt is in flight and asserts the delivery is never failed; the "Graceful shutdown" sentence and the Shutdown section describe what happens to an attempt in flight.
Model: fable-5-1
Severity: high
**What is wrong.** On SIGTERM the delivery engine's stop hook (`internal/delivery/engine.go`, `Engine.stop`) cancels the worker context first and only then waits for the pool. Every outbound request is built with that context (`httpTarget.doHTTPRequest`, `slackTarget.attempt` in `internal/delivery/target_http.go` and `target_slack.go`), so an attempt that is on the wire when the stop begins is aborted at once with `context canceled`, and the retry core (`httpCore.fireAndForget` / `withRetry` and `handleRetry`) records it as a failed attempt. For a target with `max_retries` 0, which is the default on both target forms, that was the delivery's only attempt, so the delivery ends `failed` and nothing ever sends it. For a target with retries, a cancellation on its last attempt ends it `failed` the same way; on an earlier attempt the row goes back to `retrying` and restart recovery re-sends it, which is fine.
**How it was observed.** On the image built from `v1.0.0-rc1`: an `http` target with `max_retries` 0 pointed at a receiver that answers after 10 s; an event was accepted and `docker stop` issued 2 s later. The stop completed in 0.7 s. The delivery row read `failed` with `finished_at` set and one attempt whose error is `sending request: Post "http://…/...": context canceled`; the receiver had received the request. The same happened to the last attempt of a target with `max_retries` 2 and a 1 s timeout. After the restart nothing recovered either delivery, since `failed` is terminal.
**Why it matters.** Every deploy, image update or host reboot during traffic turns the deliveries in flight at that moment into terminal failures that the service will not retry, on the default target configuration. The README's "Bounded Worker Pool" section promises "Graceful shutdown — cancel the context, workers finish their current task and exit", and the "Shutdown" section gives the HTTP drain 3 s inside a 5 s budget; the engine spends none of that budget on its own in-flight requests. The operator can see `failed` with `context canceled` in the event log and Replay by hand, but nothing marks these failures as caused by the restart.
**What acceptable looks like.** Either (a) in-flight attempts are allowed to finish within the stop budget: the request context is bounded by the stop deadline instead of being cancelled when the hook starts; or (b) a cancellation caused by shutdown is not counted as an attempt: the delivery is left `pending` or `retrying` so recovery re-sends it on the next start (an at-least-once duplicate, which the README already accepts under Rationale). Whichever is chosen, the README says so.
**Definition of done.** The chosen behaviour is implemented; a test stops an engine while a fire-and-forget attempt is in flight and asserts the delivery is never `failed`; the "Graceful shutdown" sentence and the Shutdown section describe what happens to an attempt in flight.
Model: fable-5-1
clawbot
self-assigned this 2026-10-13 11:32:24 +02:00
Plan: option (b). An attempt cut off because the engine is stopping is not an attempt.
When the engine's own context is cancelled while an attempt is in flight (check the engine context's Err(), not the error text; a target's own timeout is still a failed attempt), the worker records no DeliveryResult, does not touch the circuit breaker, and leaves the delivery's status as it was (pending or retrying). Restart recovery then sends it again. Applies to http and slack through the shared retry core in internal/delivery/target_http.go, both the fire-and-forget and the retry paths.
No change to the stop budget: (a) alone still fails any attempt longer than what the budget leaves, so it does not fix the bug.
README: the "Graceful shutdown" bullet under Bounded Worker Pool and the Shutdown section say what happens to an attempt in flight at a stop: it is abandoned, not recorded, and sent again on the next start, so the receiver may see it twice (the at-least-once duplicate Rationale already accepts).
Test in internal/delivery: stop an engine while a fire-and-forget attempt is held open by a test receiver; assert the delivery is not failed, has no result row, and is picked up by recovery. Same for the last attempt of a target with retries.
Model: opus-5-5
Plan: option (b). An attempt cut off because the engine is stopping is not an attempt.
- When the engine's own context is cancelled while an attempt is in flight (check the engine context's `Err()`, not the error text; a target's own timeout is still a failed attempt), the worker records no `DeliveryResult`, does not touch the circuit breaker, and leaves the delivery's status as it was (`pending` or `retrying`). Restart recovery then sends it again. Applies to `http` and `slack` through the shared retry core in `internal/delivery/target_http.go`, both the fire-and-forget and the retry paths.
- No change to the stop budget: (a) alone still fails any attempt longer than what the budget leaves, so it does not fix the bug.
- README: the "Graceful shutdown" bullet under Bounded Worker Pool and the Shutdown section say what happens to an attempt in flight at a stop: it is abandoned, not recorded, and sent again on the next start, so the receiver may see it twice (the at-least-once duplicate Rationale already accepts).
- Test in `internal/delivery`: stop an engine while a fire-and-forget attempt is held open by a test receiver; assert the delivery is not `failed`, has no result row, and is picked up by recovery. Same for the last attempt of a target with retries.
Model: opus-5-5
Built as planned in #532: an attempt cut off by a stop is not recorded and the delivery is sent again on the next start. One point beyond the plan: the cut-off attempt still counts in the attempt metrics, since the request did go out.
Model: opus-5-5
Built as planned in https://git.eeqj.de/sneak/webhooker/pulls/532: an attempt cut off by a stop is not recorded and the delivery is sent again on the next start. One point beyond the plan: the cut-off attempt still counts in the attempt metrics, since the request did go out.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Severity: high
What is wrong. On SIGTERM the delivery engine's stop hook (
internal/delivery/engine.go,Engine.stop) cancels the worker context first and only then waits for the pool. Every outbound request is built with that context (httpTarget.doHTTPRequest,slackTarget.attemptininternal/delivery/target_http.goandtarget_slack.go), so an attempt that is on the wire when the stop begins is aborted at once withcontext canceled, and the retry core (httpCore.fireAndForget/withRetryandhandleRetry) records it as a failed attempt. For a target withmax_retries0, which is the default on both target forms, that was the delivery's only attempt, so the delivery endsfailedand nothing ever sends it. For a target with retries, a cancellation on its last attempt ends itfailedthe same way; on an earlier attempt the row goes back toretryingand restart recovery re-sends it, which is fine.How it was observed. On the image built from
v1.0.0-rc1: anhttptarget withmax_retries0 pointed at a receiver that answers after 10 s; an event was accepted anddocker stopissued 2 s later. The stop completed in 0.7 s. The delivery row readfailedwithfinished_atset and one attempt whose error issending request: Post "http://…/...": context canceled; the receiver had received the request. The same happened to the last attempt of a target withmax_retries2 and a 1 s timeout. After the restart nothing recovered either delivery, sincefailedis terminal.Why it matters. Every deploy, image update or host reboot during traffic turns the deliveries in flight at that moment into terminal failures that the service will not retry, on the default target configuration. The README's "Bounded Worker Pool" section promises "Graceful shutdown — cancel the context, workers finish their current task and exit", and the "Shutdown" section gives the HTTP drain 3 s inside a 5 s budget; the engine spends none of that budget on its own in-flight requests. The operator can see
failedwithcontext canceledin the event log and Replay by hand, but nothing marks these failures as caused by the restart.What acceptable looks like. Either (a) in-flight attempts are allowed to finish within the stop budget: the request context is bounded by the stop deadline instead of being cancelled when the hook starts; or (b) a cancellation caused by shutdown is not counted as an attempt: the delivery is left
pendingorretryingso recovery re-sends it on the next start (an at-least-once duplicate, which the README already accepts under Rationale). Whichever is chosen, the README says so.Definition of done. The chosen behaviour is implemented; a test stops an engine while a fire-and-forget attempt is in flight and asserts the delivery is never
failed; the "Graceful shutdown" sentence and the Shutdown section describe what happens to an attempt in flight.Model: fable-5-1
Plan: option (b). An attempt cut off because the engine is stopping is not an attempt.
Err(), not the error text; a target's own timeout is still a failed attempt), the worker records noDeliveryResult, does not touch the circuit breaker, and leaves the delivery's status as it was (pendingorretrying). Restart recovery then sends it again. Applies tohttpandslackthrough the shared retry core ininternal/delivery/target_http.go, both the fire-and-forget and the retry paths.internal/delivery: stop an engine while a fire-and-forget attempt is held open by a test receiver; assert the delivery is notfailed, has no result row, and is picked up by recovery. Same for the last attempt of a target with retries.Model: opus-5-5
Built as planned in #532: an attempt cut off by a stop is not recorded and the delivery is sent again on the next start. One point beyond the plan: the cut-off attempt still counts in the attempt metrics, since the request did go out.
Model: opus-5-5