Commit Graph

7 Commits

Author SHA1 Message Date
55edebaea1 Set fx.StopTimeout inside the container stop grace (closes #134)
Some checks failed
check / check (push) Has been cancelled
fx defaults the stop timeout to 15s and the Dockerfile sets no
STOPSIGNAL or grace override, so Docker's 10s default SIGKILLs the
process five seconds before the bound can fire. Everything gated on
it — including the "shutdown timed out, goroutines still running"
error log that tells an operator a component is wedged — was
unreachable in the image this repo produces.

Set fx.StopTimeout to 5s: inside the grace with headroom for signal
delivery and process exit. The option set moves into newApp() so a
test can read (*fx.App).StopTimeout() back and pin it against
drift; dropping the option makes that test report fx's 15s default.

Lower the HTTP drain budget (server.ShutdownTimeout) from 5s to 3s.
fx bounds the whole stop sequence and returns without running its
remaining hooks once the stop context expires, so two equal values
meant a drain that used its full budget exhausted the sequence
budget at that instant and skipped every later hook — the delivery
engine, the healthcheck, the webhook DB manager and the database
close — in exactly the case where the drain mattered. The tail
hooks are microsecond-scale in normal operation, so 2s of remaining
budget is ample, and holding the total at 5s keeps a wide margin
under Docker's 10s grace. This does not make the database close
unconditional: the ArchiveSweeper and RetentionReaper hooks run
before the server and can still consume the whole budget.

Bound the Sentry flush by the remaining stop budget. The server's
stop hook is not only the drain: cleanShutdown calls sentry.Flush
after it, in the same hook, and sentry.Flush takes a bare duration
and honours no context. With SENTRY_DSN set to an unreachable
endpoint, a full-length drain plus a stalled 2s flush spent the
whole 5s sequence budget by itself and the tail hooks — database
close included — were skipped again, on a configuration the README
documents. SentryFlushBudget now clamps the flush to what is left
on the stop context less server.TailHookReserve, skipping it below
250ms rather than making a useless attempt, so a full-length drain
drops Sentry events instead of the database close.

TestStopTimeout_LeavesHeadroomForTailHooks now walks every drain
length the hook can produce and asserts drain plus flush still
leaves the 2s tail margin, so it covers the hook's real worst case
rather than the drain alone; unbounding the flush fails it at a
1.01s drain. TestSentryFlushBudget covers the clamp directly.

Also fix a latent coin flip in the shared stop-hook waiter. It
selected on the drained channel against ctx.Done() with no
preamble, and select picks uniformly among ready cases, so a
component that drained against an already-expired context reported
a timeout about half the time. Not reachable through fx, which
re-checks ctx.Err() before each hook, but the helper is shared and
a direct caller can reach it. waitDone now settles the drained case
in a non-blocking preamble first; the test drives it over 1000
passes, so a restored coin flip cannot pass by luck.

README records the real stop-hook order (ArchiveSweeper,
RetentionReaper, server, delivery.Engine, healthcheck,
WebhookDBManager, database close), the two timeouts and their
relationship, why the Sentry flush is clamped rather than allowed
its own fixed budget, and the container stop grace: that lowering
the grace below the bound puts SIGKILL back in front of it, and
that an expired stop context makes fx skip its remaining hooks, so
a wedge in the first-stopped component means the database close
never runs. The Package Layout tree gets its internal/lifecycle/
entry in the sorted position, dropping the out-of-order duplicate
this branch rebased onto.
2026-08-17 21:48:25 +00:00
afe88c601a refactor: use pinned golangci-lint Docker image for linting (#55)
All checks were successful
check / check (push) Successful in 5s
Closes [issue #50](#50)

## Summary

Refactors the Dockerfile to use a separate lint stage with a pinned golangci-lint Docker image, following the pattern used by [sneak/pixa](https://git.eeqj.de/sneak/pixa). This replaces the previous approach of installing golangci-lint via curl in the builder stage.

## Changes

### Dockerfile
- **New `lint` stage** using `golangci/golangci-lint:v2.11.3` (Debian-based, pinned by sha256 digest) as a separate build stage
- **Builder stage** depends on lint via `COPY --from=lint /src/go.sum /dev/null` — build won't proceed unless linting passes
- **Go bumped** from 1.24 to 1.26.1 (`golang:1.26.1-bookworm`, pinned by sha256)
- **golangci-lint bumped** from v1.64.8 to v2.11.3
- All three Docker images (golangci-lint, golang, alpine) pinned by sha256 digest
- Debian-based golangci-lint image used (not Alpine) because mattn/go-sqlite3 CGO does not compile on musl (off64_t)

### Linter Config (.golangci.yml)
- Migrated from v1 to v2 format (`version: "2"` added)
- Removed linters no longer available in v2: `gofmt` (handled by `make fmt-check`), `gosimple` (merged into `staticcheck`), `typecheck` (always-on in v2)
- Same set of linters enabled — no rules weakened

### Code Fixes (all lint issues from v2 upgrade)
- Added package comments to all packages
- Added doc comments to all exported types, functions, and methods
- Fixed unchecked errors flagged by `errcheck` (sqlDB.Close, os.Setenv in tests, resp.Body.Close, fmt.Fprint)
- Fixed unused parameters flagged by `revive` (renamed to `_`)
- Fixed `gosec` G120 warnings: added `http.MaxBytesReader` before `r.ParseForm()` calls
- Fixed `staticcheck` QF1012: replaced `WriteString(fmt.Sprintf(...))` with `fmt.Fprintf`
- Fixed `staticcheck` QF1003: converted if/else chain to tagged switch
- Renamed `DeliveryTask` → `Task` to avoid package stutter (`delivery.Task` instead of `delivery.DeliveryTask`)
- Renamed shadowed builtin `max` parameter to `upperBound` in `cryptoRandInt`
- Used `t.Setenv` instead of `os.Setenv` in tests (auto-restores)

### README.md
- Updated version requirements: Go 1.26+, golangci-lint v2.11+
- Updated Dockerfile description in project structure

## Verification

`docker build .` passes cleanly — formatting check, linting, all tests, and build all succeed.

Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #55
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-25 02:16:38 +01:00
clawbot
4dd4dfa5eb chore: consolidate DBURL into DATA_DIR, codebase audit for 1.0.0
All checks were successful
check / check (push) Successful in 56s
DBURL → DATA_DIR consolidation:
- Remove DBURL env var entirely; main DB now lives at {DATA_DIR}/webhooker.db
- database.go constructs DB path from config.DataDir, ensures dir exists
- Update DATA_DIR prod default from /data/events to /data
- Update all tests to use DataDir instead of DBURL
- Update Dockerfile: /data (not /data/events) for all SQLite databases
- Update README configuration table, Docker examples, architecture docs

Dead code removal:
- Remove unused IndexResponse struct (handlers/index.go)
- Remove unused TemplateData struct (handlers/handlers.go)

Stale comment cleanup:
- Remove TODO in server.go (DB cleanup handled by fx lifecycle)
- Fix nolint:golint → nolint:revive on ServerParams for consistency
- Clean up verbose middleware/routing comments in routes.go
- Fix TODO fan-out description (worker pool, not goroutine-per-target)

.gitignore fixes:
- Add data/ directory to gitignore
- Remove stale config.yaml entry (env-only config since rework)
2026-03-01 23:33:20 -08:00
clawbot
36824046fb fix: remove double cleanShutdown call (closes #21)
The serve() method called cleanShutdown() after ctx.Done(), and the fx
OnStop hook also called cleanShutdown(). Remove the call in serve() so
shutdown happens exactly once via the fx lifecycle.
2026-03-01 16:35:55 -08:00
clawbot
e6b79ce1be fix: remove redundant godotenv import
The godotenv/autoload import was duplicated in both config.go and
server.go. Keep it only in config.go where configuration is loaded.

closes #11
2026-03-01 16:01:44 -08:00
f9a9569015 feat: bring repo up to REPO_POLICIES standards (#6)
All checks were successful
check / check (push) Successful in 8s
## Summary

This PR brings the webhooker repo into full REPO_POLICIES compliance, addressing both [issue #1](#1) and [issue #2](#2).

## Changes

### New files
- **`cmd/webhooker/main.go`** — The missing application entry point. Uses Uber fx to wire together all internal packages (config, database, logger, server, handlers, middleware, healthcheck, globals, session). Minimal glue code.
- **`REPO_POLICIES.md`** — Fetched from authoritative source (`sneak/prompts`)
- **`.editorconfig`** — Fetched from authoritative source
- **`.dockerignore`** — Sensible Go project exclusions
- **`.gitea/workflows/check.yml`** — CI workflow that runs `docker build .` on push to any branch (Gitea Actions format, actions/checkout pinned by sha256)
- **`configs/config.yaml.example`** — Moved from root `config.yaml`

### Modified files
- **`Makefile`** — Complete rewrite with all REPO_POLICIES required targets: `test`, `lint`, `fmt`, `fmt-check`, `check`, `build`, `hooks`, `docker`, `clean`, plus `dev`, `run`, `deps`
- **`Dockerfile`** — Complete rewrite:
  - Builder: `golang:1.24` (Debian-based, pinned by `sha256:d2d2bc1c84f7...`). Debian needed because `gorm.io/driver/sqlite` pulls `mattn/go-sqlite3` (CGO) which fails on Alpine musl.
  - golangci-lint v1.64.8 installed from GitHub release archive with sha256 verification (v1.x because `.golangci.yml` uses v1 config format)
  - Runs `make check` (fmt-check + lint + test + build) as build step
  - Final stage: `alpine:3.21` (pinned by `sha256:c3f8e73fdb79...`) with non-root user, healthcheck, port 8080
- **`README.md`** — Rewritten with all required REPO_POLICIES sections: description line with name/purpose/category/license/author, Getting Started, Rationale, Design, TODO (integrated from TODO.md), License, Author
- **`.gitignore`** — Fixed `webhooker` pattern to `/webhooker` (was blocking `cmd/webhooker/`), added `config.yaml` to prevent committing runtime config with secrets
- **`static/static.go`** — Removed `vendor` from embed directive (directory was empty/missing)
- **`internal/database/database_test.go`** — Fixed to use in-memory config via `afero.MemMapFs` instead of depending on `config.yaml` on disk. Test is now properly isolated.
- **`go.mod`/`go.sum`** — `go mod tidy`

### Removed files
- **`TODO.md`** — Content integrated into README.md TODO section
- **`config.yaml`** — Moved to `configs/config.yaml.example`

## Verification
- `docker build .` passes (lint , test , build )
- All existing tests pass with no modifications to assertions or test logic
- `.golangci.yml` untouched

closes #1
closes #2

Co-authored-by: clawbot <clawbot@noreply.git.eeqj.de>
Reviewed-on: #6
Co-authored-by: clawbot <clawbot@noreply.example.org>
Co-committed-by: clawbot <clawbot@noreply.example.org>
2026-03-01 19:01:44 +01:00
1244f3e2d5 initial 2026-03-01 22:52:08 +07:00