Exit non-zero when the HTTP listener fails (closes #200) #218

clawbot · 2026-08-20T06:21:46+02:00

clawbot commented

2026-08-20 06:21:46 +02:00

Closes #200

Problem

Server's fx OnStart hook returns nil as soon as it spawns the serving goroutine, so a failed listen was only ever discovered inside that goroutine. It logged the error and called s.cancelFunc(), which nothing outside the goroutine observes: fx reported RUNNING and the process stayed alive with nothing bound. The service was down and looked up, so systemd Restart=on-failure and Docker restart policies never fired.

Approach

ServerParams now takes fx.Shutdowner. On a listen failure the server logs the error as before and then asks the Shutdowner to stop the app with server.ListenFailureExitCode (1). Shutdown goes through fx's normal stop sequence, so every OnStop hook — the HTTP drain, the delivery engine, the healthcheck, the DB manager, the database close — still runs. The clean-shutdown path itself is untouched.
The context cancel is kept, moved into shutdownOnListenFailure, where it only unwinds serve()'s own wait.
Server.exitCode is removed. Its only writer set it to zero in cleanShutdown and its only reader was serve()'s return value, which Run discards; the process status is fx's to decide. Left in place it would have become a real data race, since the new path makes cleanShutdown and serve()'s return run concurrently under -race.

Regression test

TestListenFailure_ShutsDownTheApp (internal/server/listen_failure_test.go) occupies a kernel-chosen port, starts the wired app on it, and requires a non-zero shutdown signal from app.Wait() within two seconds, then requires the stop sequence to complete. It reuses newTestEnv's wired collaborators rather than stubs, so the Server under test is the one that ships.

It fails without the fix. Reverting only the http.go hunk:

    listen_failure_test.go:80: listen failure left the app running
--- FAIL: TestListenFailure_ShutsDownTheApp (2.43s)
FAIL	sneak.berlin/go/webhooker/internal/server	2.704s

Definition of done

Item	Evidence
listen failure calls `fx.Shutdowner.Shutdown()`, process exits non-zero	manual repro below, exit code 1
second start on a bound `PORT` exits non-zero within a second or so, logging the bind error	280 ms, exit 1, bind error logged
regression test asserts termination rather than RUNNING	`TestListenFailure_ShutsDownTheApp`, fails without the fix
no change to the clean-shutdown drain	SIGTERM still exits 0 in 20 ms with all 7 `OnStop` hooks run

Gate evidence

make check — exit 0.

Cache-defeated container build on the rebased head, docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain . — exit 0, zero (cached) lines anywhere in the log:

#22 [lint 9/9] RUN --network=none golangci-lint run --config .golangci.yml ./...
#22 95.57 0 issues.
#22 DONE 97.7s

#35 [builder  9/11] RUN make test
#35 122.1 --- PASS: TestListenFailure_ShutsDownTheApp (1.22s)
#35 DONE 123.2s

The built image was removed; no containers were started.

Manual repro

Two instances of make build's binary on the same PORT, second one started while the first holds the port:

second instance exit code: 1 in 280 ms
{"level":"ERROR","msg":"listen error","error":"listen tcp :43219: bind: address already in use"}
[Fx] HOOK OnStop  ...database.New.func2() ... ran successfully in 186.506µs

The whole stop sequence, including the database close, runs before the non-zero exit. Before this change the same command sat at [Fx] RUNNING indefinitely.

Clean shutdown of a healthy instance, unchanged:

clean shutdown exit code: 0 in 20 ms
OnStop hooks completed: 7

TODO.md untouched, per #112.

Closes https://git.eeqj.de/sneak/webhooker/issues/200 ## Problem `Server`'s fx `OnStart` hook returns `nil` as soon as it spawns the serving goroutine, so a failed listen was only ever discovered inside that goroutine. It logged the error and called `s.cancelFunc()`, which nothing outside the goroutine observes: fx reported RUNNING and the process stayed alive with nothing bound. The service was down and looked up, so systemd `Restart=on-failure` and Docker restart policies never fired. ## Approach - `ServerParams` now takes `fx.Shutdowner`. On a listen failure the server logs the error as before and then asks the Shutdowner to stop the app with `server.ListenFailureExitCode` (1). Shutdown goes through fx's normal stop sequence, so every `OnStop` hook — the HTTP drain, the delivery engine, the healthcheck, the DB manager, the database close — still runs. The clean-shutdown path itself is untouched. - The context cancel is kept, moved into `shutdownOnListenFailure`, where it only unwinds `serve()`'s own wait. - `Server.exitCode` is removed. Its only writer set it to zero in `cleanShutdown` and its only reader was `serve()`'s return value, which `Run` discards; the process status is fx's to decide. Left in place it would have become a real data race, since the new path makes `cleanShutdown` and `serve()`'s return run concurrently under `-race`. ## Regression test `TestListenFailure_ShutsDownTheApp` (`internal/server/listen_failure_test.go`) occupies a kernel-chosen port, starts the wired app on it, and requires a non-zero shutdown signal from `app.Wait()` within two seconds, then requires the stop sequence to complete. It reuses `newTestEnv`'s wired collaborators rather than stubs, so the `Server` under test is the one that ships. It fails without the fix. Reverting only the `http.go` hunk: ``` listen_failure_test.go:80: listen failure left the app running --- FAIL: TestListenFailure_ShutsDownTheApp (2.43s) FAIL sneak.berlin/go/webhooker/internal/server 2.704s ``` ## Definition of done | Item | Evidence | | --- | --- | | listen failure calls `fx.Shutdowner.Shutdown()`, process exits non-zero | manual repro below, exit code 1 | | second start on a bound `PORT` exits non-zero within a second or so, logging the bind error | 280 ms, exit 1, bind error logged | | regression test asserts termination rather than RUNNING | `TestListenFailure_ShutsDownTheApp`, fails without the fix | | no change to the clean-shutdown drain | SIGTERM still exits 0 in 20 ms with all 7 `OnStop` hooks run | ## Gate evidence `make check` — exit 0. Cache-defeated container build on the rebased head, `docker build --no-cache-filter=lint --no-cache-filter=builder --progress=plain .` — exit 0, zero `(cached)` lines anywhere in the log: ``` #22 [lint 9/9] RUN --network=none golangci-lint run --config .golangci.yml ./... #22 95.57 0 issues. #22 DONE 97.7s #35 [builder 9/11] RUN make test #35 122.1 --- PASS: TestListenFailure_ShutsDownTheApp (1.22s) #35 DONE 123.2s ``` The built image was removed; no containers were started. ## Manual repro Two instances of `make build`'s binary on the same `PORT`, second one started while the first holds the port: ``` second instance exit code: 1 in 280 ms {"level":"ERROR","msg":"listen error","error":"listen tcp :43219: bind: address already in use"} [Fx] HOOK OnStop ...database.New.func2() ... ran successfully in 186.506µs ``` The whole stop sequence, including the database close, runs before the non-zero exit. Before this change the same command sat at `[Fx] RUNNING` indefinitely. Clean shutdown of a healthy instance, unchanged: ``` clean shutdown exit code: 0 in 20 ms OnStop hooks completed: 7 ``` `TODO.md` untouched, per https://git.eeqj.de/sneak/webhooker/issues/112.

clawbot added 1 commit 2026-08-20 06:21:47 +02:00

Exit non-zero when the HTTP listener fails (closes #200 )

check / check (push) Successful in 6m20s

Details

bfbf416393

The fx OnStart hook returned as soon as the serving goroutine was
spawned, so a failed listen was discovered only inside that goroutine.
It logged the error and cancelled the server's own context, which
nothing outside the goroutine observes: fx reported RUNNING and the
process stayed alive with nothing bound. The service was down and
looked up, so systemd Restart=on-failure and Docker restart policies
never fired.

The Server now takes fx.Shutdowner and, on a listen failure, asks it to
stop the app with ListenFailureExitCode. Shutdown runs through fx's
normal stop sequence, so every OnStop hook — including the HTTP drain
and the database close — still executes; the clean-shutdown path is
untouched.

Server.exitCode goes with it. Its only writer set it to zero during
cleanShutdown and its only reader was serve()'s return value, which Run
discards; the process status is fx's to decide. Left in place it would
have been a genuine data race, since the new path makes cleanShutdown
and serve()'s return run concurrently under -race.

Verified by TestListenFailure_ShutsDownTheApp, which occupies a port,
starts the wired app on it, and requires a non-zero shutdown signal
within two seconds. Reverting the fix fails it with "listen failure
left the app running".

clawbot added the needs-review label 2026-08-20 06:21:52 +02:00

clawbot self-assigned this 2026-08-20 06:21:54 +02:00

clawbot commented

2026-08-20 06:41:20 +02:00

PASS — satisfies every item of the definition of done in #200; no defects found.

Disclosure: my independent cache-defeated gate on bfbf416 did not go green overall. make test failed with internal/handlers timing out at 90s (TestFailedLogin_LogLineDoesNotTrackUsernameSize, 14 parallel subtests at ~50s each). Not attributable to this PR: internal/handlers is byte-identical between next and this head and does not import internal/server, and I reproduced the identical timeout on origin/next with no change applied (host load average 122 on 48 cores). Everything the change is responsible for was green — lint 0 issues. in 122.3s, zero (cached) lines, internal/server clean under -race, and reverting only the internal/server/http.go hunk reproduces listen_failure_test.go:84: listen failure left the app running exactly as claimed. Gitea CI on bfbf416 is green.

Forward flag for #201, not a defect here: on this new path fx.App.Run calls os.Exit(1) itself, so a defer in main never runs — whereas the clean-shutdown path exits 0 by returning from main, where a defer would run. A DATA_DIR lock released via defer in main would therefore work on docker stop and silently leak on listen failure; the release has to be an fx OnStop hook.

PASS — satisfies every item of the definition of done in https://git.eeqj.de/sneak/webhooker/issues/200; no defects found. Disclosure: my independent cache-defeated gate on `bfbf416` did not go green overall. `make test` failed with `internal/handlers` timing out at 90s (`TestFailedLogin_LogLineDoesNotTrackUsernameSize`, 14 parallel subtests at ~50s each). Not attributable to this PR: `internal/handlers` is byte-identical between `next` and this head and does not import `internal/server`, and I reproduced the identical timeout on `origin/next` with no change applied (host load average 122 on 48 cores). Everything the change is responsible for was green — lint `0 issues.` in 122.3s, zero `(cached)` lines, `internal/server` clean under `-race`, and reverting only the `internal/server/http.go` hunk reproduces `listen_failure_test.go:84: listen failure left the app running` exactly as claimed. Gitea CI on `bfbf416` is green. Forward flag for https://git.eeqj.de/sneak/webhooker/issues/201, not a defect here: on this new path `fx.App.Run` calls `os.Exit(1)` itself, so a `defer` in `main` never runs — whereas the clean-shutdown path exits 0 by returning from `main`, where a `defer` would run. A `DATA_DIR` lock released via `defer` in `main` would therefore work on `docker stop` and silently leak on listen failure; the release has to be an fx `OnStop` hook.

clawbot merged commit a13e5b7ded into next

2026-08-20 06:42:37 +02:00

clawbot deleted branch issue-200-listen-failure-shutdown

2026-08-20 06:42:37 +02:00

clawbot referenced this issue from a commit

2026-08-20 06:42:37 +02:00

Shut down the app when the listener fails (closes #200) (#218)

clawbot referenced this pull request

2026-08-20 06:42:47 +02:00

SIGTERM during startup panics on a nil httpServer, and cleanShutdown reads two fields with no happens-before edge #226

clawbot referenced this pull request

2026-08-20 06:49:32 +02:00