Refactor delivery targets to a Target interface (closes #77) (#81)
All checks were successful
check / check (push) Successful in 2m42s
All checks were successful
check / check (push) Successful in 2m42s
Refactors the delivery engine so each target TYPE is an implementation of a `Target` interface, dispatched from a registry, with each target owning its full delivery including durable retries. Implements the authoritative design from issue #77 (the corrected "hand the DB + Scheduler to the target" design). ## The new interface ```go type Scheduler interface { ScheduleRetry(task Task, delay time.Duration) } type Target interface { Deliver(ctx context.Context, webhookDB *gorm.DB, d *database.Delivery, task *Task, sched Scheduler) } ``` `Deliver` receives everything a target needs to be autonomous and durable: the request context, the per-webhook `*gorm.DB`, the `*database.Delivery`, the attempt `*Task`, and a `Scheduler` (the engine) for durable re-enqueue. The target makes one attempt, writes the `DeliveryResult`, updates `DeliveryStatus`, and — for retry targets — decides whether to retry, computes its own backoff, gates with its own circuit breaker, and reschedules via the injected `Scheduler`. `processDelivery` collapses to a registry lookup (`map[database.TargetType]Target`) and a `Deliver` call; an unknown target type still fails the delivery as before. ## Per-target ownership - `httpTarget` and `slackTarget` share a retry core (`httpCore`) that owns retry, exponential backoff, and the per-target circuit breaker. The core is fire-and-forget when `MaxRetries == 0` and adds breaker-gated backed-off retries when `MaxRetries > 0`. The per-attempt request differs (HTTP forwards the body + filtered headers; Slack posts a formatted message) and is supplied as a closure, so each keeps its exact recording semantics (e.g. HTTP records no error string for a non-2xx, Slack records `HTTP <code>`). - `databaseTarget` and `logTarget` are fire-and-forget: they record a single successful attempt. Moved wholesale into the http/slack targets: `deliverHTTP*`, `handleHTTPRetry`, `circuitBreakerBlock`, `calcBackoff` / `calcRemainingBackoff` / `backoffElapsed`, the circuit-breaker `sync.Map` + `getCircuitBreaker`, `clientForConfig`, `doHTTPRequest`, `applyRequestHeaders`, and the config parsers. The engine keeps `recordResult`, `updateDeliveryStatus`, and `ScheduleRetry`. ## Slack MaxRetries gating Slack is now on the same shared core as HTTP, with retry + breaker gated on `MaxRetries`. A `MaxRetries` of 0 stays single-attempt fire-and-forget, so **every existing Slack target is unchanged**; a Slack target configured with retries gets backoff + circuit breaker. ## Log-target full content `logTarget` now logs the ENTIRE inbound webhook — full request body and full request headers, plus method, content type, and the webhook id and entrypoint id — rather than a summary line. This supersedes the smaller log-summary work (#70). ## `Task.EntrypointID` To carry the entrypoint id to the log target, `Task` gains an `EntrypointID` field, populated in the webhook handler's `buildDeliveryTasks`, the engine's recovery-task builder, and `buildEventFromTask`. ## Durability / recovery The crash-durable async retry model is preserved unchanged: one attempt per worker turn; on failure the status is set `retrying`, backoff is computed, and the task is re-enqueued via `ScheduleRetry` (a `time.AfterFunc` onto the retry channel). On restart, `recoverRetryingDeliveries` and the 60s sweep hand each orphaned `retrying` delivery back to its target to recompute the remaining backoff and reschedule (targets that own retries implement an internal `rescheduler`; fire-and-forget targets, which never produce `retrying` deliveries, are skipped). ## How behaviour is preserved No external behaviour changes except the two called out above (log target full content; Slack gaining `MaxRetries`-gated retries). All existing delivery tests pass with only their `export_test.go` wrappers re-pointed at the new structure — `ExportDeliverHTTP/Slack/Database/Log` now call the targets, `ExportGetCircuitBreaker` / `ExportClient` / `ExportClientForConfig` / `ExportDoHTTPRequest` resolve against the HTTP target's shared client and breaker map, and `ExportParseHTTPConfig` / `ExportParseSlackConfig` call the relocated free functions. Added: a `logTarget` test asserting the log line contains the full body, headers, and ids, and a Slack `MaxRetries`-gated retry test. `docker build .` is green (fmt-check, lint, test, static build all pass). Closes #77 Co-authored-by: sneak <sneak@sneak.berlin> Reviewed-on: #81 Co-authored-by: clawbot <clawbot@noreply.example.org> Co-committed-by: clawbot <clawbot@noreply.example.org>
This commit was merged in pull request #81.
This commit is contained in:
499
internal/delivery/target_http.go
Normal file
499
internal/delivery/target_http.go
Normal file
@@ -0,0 +1,499 @@
|
||||
package delivery
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"gorm.io/gorm"
|
||||
"sneak.berlin/go/webhooker/internal/database"
|
||||
)
|
||||
|
||||
// Sentinel errors returned by the config parsers.
|
||||
var (
|
||||
errEmptyTargetConfig = errors.New(
|
||||
"empty target config",
|
||||
)
|
||||
errMissingTargetURL = errors.New(
|
||||
"target URL is required",
|
||||
)
|
||||
)
|
||||
|
||||
// HTTPTargetConfig holds configuration for http target
|
||||
// types.
|
||||
type HTTPTargetConfig struct {
|
||||
URL string `json:"url"`
|
||||
Headers map[string]string `json:"headers,omitempty"`
|
||||
Timeout int `json:"timeout,omitempty"`
|
||||
}
|
||||
|
||||
// httpCore holds the retry, backoff, and circuit-breaker
|
||||
// machinery shared by the HTTP and Slack targets. Each of
|
||||
// those targets owns its own httpCore instance (and thus its
|
||||
// own circuit breakers); the per-attempt request differs
|
||||
// between them and is supplied as a closure.
|
||||
type httpCore struct {
|
||||
eng *Engine
|
||||
|
||||
// circuitBreakers stores a *CircuitBreaker per target ID.
|
||||
circuitBreakers sync.Map
|
||||
}
|
||||
|
||||
// deliver runs one delivery attempt through the retry core.
|
||||
// A maxRetries of 0 is fire-and-forget: a single attempt is
|
||||
// recorded and no circuit breaker is consulted. A positive
|
||||
// maxRetries gates the attempt on the circuit breaker and
|
||||
// schedules a backed-off retry on failure.
|
||||
func (c *httpCore) deliver(
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxRetries int,
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
if maxRetries == 0 {
|
||||
c.fireAndForget(webhookDB, d, attempt())
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.withRetry(
|
||||
webhookDB, d, task, sched, maxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
func (c *httpCore) fireAndForget(
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
res attemptResult,
|
||||
) {
|
||||
c.eng.recordResult(
|
||||
webhookDB, d, 1, res.success,
|
||||
res.statusCode, res.respBody, res.errMsg,
|
||||
res.duration,
|
||||
)
|
||||
|
||||
if res.success {
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d,
|
||||
database.DeliveryStatusDelivered,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d, database.DeliveryStatusFailed,
|
||||
)
|
||||
}
|
||||
|
||||
func (c *httpCore) withRetry(
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxRetries int,
|
||||
attempt func() attemptResult,
|
||||
) {
|
||||
cb := c.getCircuitBreaker(task.TargetID)
|
||||
if c.circuitBreakerBlock(webhookDB, d, task, sched, cb) {
|
||||
return
|
||||
}
|
||||
|
||||
attemptNum := task.AttemptNum
|
||||
|
||||
res := attempt()
|
||||
|
||||
c.eng.recordResult(
|
||||
webhookDB, d, attemptNum, res.success,
|
||||
res.statusCode, res.respBody, res.errMsg,
|
||||
res.duration,
|
||||
)
|
||||
|
||||
if res.success {
|
||||
cb.RecordSuccess()
|
||||
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d,
|
||||
database.DeliveryStatusDelivered,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
cb.RecordFailure()
|
||||
|
||||
c.handleRetry(
|
||||
webhookDB, d, task, sched, maxRetries, attemptNum,
|
||||
)
|
||||
}
|
||||
|
||||
func (c *httpCore) circuitBreakerBlock(
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
cb *CircuitBreaker,
|
||||
) bool {
|
||||
if cb.Allow() {
|
||||
return false
|
||||
}
|
||||
|
||||
remaining := cb.CooldownRemaining()
|
||||
|
||||
c.eng.log.Info(
|
||||
"circuit breaker open, skipping delivery",
|
||||
"target_id", task.TargetID,
|
||||
"target_name", task.TargetName,
|
||||
"delivery_id", d.ID,
|
||||
"cooldown_remaining", remaining,
|
||||
)
|
||||
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d,
|
||||
database.DeliveryStatusRetrying,
|
||||
)
|
||||
|
||||
retryTask := *task
|
||||
sched.ScheduleRetry(retryTask, remaining)
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
func (c *httpCore) handleRetry(
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
maxRetries int,
|
||||
attemptNum int,
|
||||
) {
|
||||
if attemptNum >= maxRetries {
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d,
|
||||
database.DeliveryStatusFailed,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
c.eng.updateDeliveryStatus(
|
||||
webhookDB, d, database.DeliveryStatusRetrying,
|
||||
)
|
||||
|
||||
backoff := calcBackoff(attemptNum)
|
||||
|
||||
retryTask := *task
|
||||
retryTask.AttemptNum = attemptNum + 1
|
||||
sched.ScheduleRetry(retryTask, backoff)
|
||||
}
|
||||
|
||||
func (c *httpCore) getCircuitBreaker(
|
||||
targetID string,
|
||||
) *CircuitBreaker {
|
||||
if val, ok := c.circuitBreakers.Load(targetID); ok {
|
||||
cb, _ := val.(*CircuitBreaker)
|
||||
|
||||
return cb
|
||||
}
|
||||
|
||||
fresh := NewCircuitBreaker()
|
||||
|
||||
actual, _ := c.circuitBreakers.LoadOrStore(
|
||||
targetID, fresh,
|
||||
)
|
||||
|
||||
cb, _ := actual.(*CircuitBreaker)
|
||||
|
||||
return cb
|
||||
}
|
||||
|
||||
// remainingBackoff returns how long remains of the backoff
|
||||
// window for the last attempt of a recovered retrying
|
||||
// delivery. It implements rescheduler.
|
||||
func (c *httpCore) remainingBackoff(
|
||||
webhookDB *gorm.DB,
|
||||
deliveryID string,
|
||||
attemptNum int,
|
||||
) time.Duration {
|
||||
var lastResult database.DeliveryResult
|
||||
|
||||
err := webhookDB.
|
||||
Where("delivery_id = ?", deliveryID).
|
||||
Order("created_at DESC").
|
||||
First(&lastResult).Error
|
||||
if err != nil {
|
||||
return 0
|
||||
}
|
||||
|
||||
backoff := calcBackoff(attemptNum)
|
||||
elapsed := time.Since(lastResult.CreatedAt)
|
||||
remaining := backoff - elapsed
|
||||
|
||||
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 := calcBackoff(attemptNum)
|
||||
|
||||
return time.Since(lastResult.CreatedAt) >= backoff
|
||||
}
|
||||
|
||||
func calcBackoff(attemptNum int) time.Duration {
|
||||
shift := max(attemptNum-1, 0)
|
||||
shift = min(shift, maxBackoffShift)
|
||||
|
||||
return time.Duration(1<<uint(shift)) * time.Second
|
||||
}
|
||||
|
||||
// httpTarget delivers events to http targets. It forwards the
|
||||
// event body and (filtered) request headers to the configured
|
||||
// URL and owns retry, backoff, and circuit breaking through
|
||||
// the shared httpCore.
|
||||
type httpTarget struct {
|
||||
*httpCore
|
||||
|
||||
client *http.Client
|
||||
}
|
||||
|
||||
// Deliver implements Target.
|
||||
func (t *httpTarget) Deliver(
|
||||
ctx context.Context,
|
||||
webhookDB *gorm.DB,
|
||||
d *database.Delivery,
|
||||
task *Task,
|
||||
sched Scheduler,
|
||||
) {
|
||||
cfg, err := parseHTTPConfig(d.Target.Config)
|
||||
if err != nil {
|
||||
t.eng.log.Error(
|
||||
"invalid HTTP target config",
|
||||
"target_id", d.TargetID,
|
||||
"error", err,
|
||||
)
|
||||
|
||||
t.eng.recordResult(
|
||||
webhookDB, d, task.AttemptNum,
|
||||
false, 0, "", err.Error(), 0,
|
||||
)
|
||||
|
||||
t.eng.updateDeliveryStatus(
|
||||
webhookDB, d, database.DeliveryStatusFailed,
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
attempt := func() attemptResult {
|
||||
return t.attempt(ctx, cfg, &d.Event)
|
||||
}
|
||||
|
||||
t.deliver(
|
||||
webhookDB, d, task, sched,
|
||||
d.Target.MaxRetries, attempt,
|
||||
)
|
||||
}
|
||||
|
||||
// attempt performs a single HTTP delivery attempt and derives
|
||||
// the success flag and error message the same way the engine
|
||||
// did: a non-2xx response is a failure but carries no error
|
||||
// string; only a transport-level error does.
|
||||
func (t *httpTarget) attempt(
|
||||
ctx context.Context,
|
||||
cfg *HTTPTargetConfig,
|
||||
event *database.Event,
|
||||
) attemptResult {
|
||||
statusCode, respBody, duration, reqErr :=
|
||||
t.doHTTPRequest(ctx, cfg, event)
|
||||
|
||||
success := reqErr == nil &&
|
||||
statusCode >= httpSuccessMin &&
|
||||
statusCode < httpSuccessMax
|
||||
|
||||
errMsg := ""
|
||||
if reqErr != nil {
|
||||
errMsg = reqErr.Error()
|
||||
}
|
||||
|
||||
return attemptResult{
|
||||
statusCode: statusCode,
|
||||
respBody: respBody,
|
||||
duration: duration,
|
||||
success: success,
|
||||
errMsg: errMsg,
|
||||
}
|
||||
}
|
||||
|
||||
func (t *httpTarget) doHTTPRequest(
|
||||
ctx context.Context,
|
||||
cfg *HTTPTargetConfig,
|
||||
event *database.Event,
|
||||
) (int, string, int64, error) {
|
||||
start := time.Now()
|
||||
|
||||
req, reqErr := http.NewRequestWithContext(
|
||||
ctx,
|
||||
http.MethodPost,
|
||||
cfg.URL,
|
||||
bytes.NewReader([]byte(event.Body)),
|
||||
)
|
||||
if reqErr != nil {
|
||||
return 0, "", 0, fmt.Errorf(
|
||||
"creating request: %w", reqErr,
|
||||
)
|
||||
}
|
||||
|
||||
applyRequestHeaders(req, event, cfg)
|
||||
|
||||
client := t.clientForConfig(cfg)
|
||||
|
||||
resp, doErr := executeHTTPRequest(client, req)
|
||||
|
||||
dur := time.Since(start).Milliseconds()
|
||||
if doErr != nil {
|
||||
return 0, "", dur, fmt.Errorf(
|
||||
"sending request: %w", doErr,
|
||||
)
|
||||
}
|
||||
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
body, readErr := io.ReadAll(
|
||||
io.LimitReader(resp.Body, maxBodyLog),
|
||||
)
|
||||
if readErr != nil {
|
||||
return resp.StatusCode, "", dur,
|
||||
fmt.Errorf(
|
||||
"reading response body: %w", readErr,
|
||||
)
|
||||
}
|
||||
|
||||
return resp.StatusCode, string(body), dur, nil
|
||||
}
|
||||
|
||||
func (t *httpTarget) clientForConfig(
|
||||
cfg *HTTPTargetConfig,
|
||||
) *http.Client {
|
||||
if cfg.Timeout > 0 {
|
||||
// Reuse the shared client's SSRF-safe transport so
|
||||
// a per-target timeout does not drop the
|
||||
// request-time private-IP guard. Only the timeout
|
||||
// is overridden.
|
||||
return &http.Client{
|
||||
Timeout: time.Duration(
|
||||
cfg.Timeout,
|
||||
) * time.Second,
|
||||
Transport: t.client.Transport,
|
||||
}
|
||||
}
|
||||
|
||||
return t.client
|
||||
}
|
||||
|
||||
func parseHTTPConfig(
|
||||
configJSON string,
|
||||
) (*HTTPTargetConfig, error) {
|
||||
if configJSON == "" {
|
||||
return nil, errEmptyTargetConfig
|
||||
}
|
||||
|
||||
var cfg HTTPTargetConfig
|
||||
|
||||
err := json.Unmarshal(
|
||||
[]byte(configJSON), &cfg,
|
||||
)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf(
|
||||
"parsing config JSON: %w", err,
|
||||
)
|
||||
}
|
||||
|
||||
if cfg.URL == "" {
|
||||
return nil, errMissingTargetURL
|
||||
}
|
||||
|
||||
return &cfg, nil
|
||||
}
|
||||
|
||||
// isForwardableHeader returns true if the header should
|
||||
// be forwarded to targets.
|
||||
func isForwardableHeader(name string) bool {
|
||||
switch http.CanonicalHeaderKey(name) {
|
||||
case "Host", "Connection", "Keep-Alive",
|
||||
"Transfer-Encoding", "Te", "Trailer",
|
||||
"Upgrade", "Proxy-Authorization",
|
||||
"Proxy-Connection", "Content-Length":
|
||||
return false
|
||||
default:
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
func applyRequestHeaders(
|
||||
req *http.Request,
|
||||
event *database.Event,
|
||||
cfg *HTTPTargetConfig,
|
||||
) {
|
||||
if event.ContentType != "" {
|
||||
req.Header.Set(
|
||||
"Content-Type", event.ContentType,
|
||||
)
|
||||
}
|
||||
|
||||
var originalHeaders map[string][]string
|
||||
|
||||
if event.Headers != "" {
|
||||
jsonErr := json.Unmarshal(
|
||||
[]byte(event.Headers),
|
||||
&originalHeaders,
|
||||
)
|
||||
if jsonErr == nil {
|
||||
for k, vals := range originalHeaders {
|
||||
if isForwardableHeader(k) {
|
||||
for _, v := range vals {
|
||||
req.Header.Add(k, v)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
for k, v := range cfg.Headers {
|
||||
req.Header.Set(k, v)
|
||||
}
|
||||
|
||||
req.Header.Set("User-Agent", "webhooker/1.0")
|
||||
}
|
||||
|
||||
// executeHTTPRequest sends an HTTP request using the provided
|
||||
// client. URLs are validated by the config parsers and the
|
||||
// SSRF-safe transport before reaching here.
|
||||
func executeHTTPRequest(
|
||||
client *http.Client, req *http.Request,
|
||||
) (*http.Response, error) {
|
||||
return client.Do(req) //#nosec G704 -- URL validated by parseHTTPConfig/parseSlackConfig and SSRF-safe transport
|
||||
}
|
||||
Reference in New Issue
Block a user