Commit Graph
3 Commits
Author SHA1 Message Date
zhangwenjian 8ee4141af6 fix🐛: check the certificate before announcing that the port is reachable
AfterListen promises a hook that the port answers. The bind was moved onto the
caller's goroutine to keep that promise, but with ssl enabled there was a second
way to fail after the announcement: ServeTLS reads the certificate files itself,
on the serving goroutine, so a bad path or an unreadable key surfaced once the
hooks had already run.

tls.LoadX509KeyPair now runs before anything is announced, and its error is
returned from startServing the way a failed bind is. ServeTLS still does the
real work - handing it a tls.Listener built here instead would take over the
HTTP/2 negotiation it sets up, and quietly drop h2 for every TLS deployment. The
cost is one extra read of the certificate at startup.

The fatal in the serving goroutine said "listen:". Neither the bind nor the
certificate reaches it any more, so it says "serve:".

The test covers the certificate path alongside the bind: neither may announce
the phase, and neither may seal it.

The counter-proof is not clean, and saying so is the point. Removing the check
does turn the run red, but through log.Fatal killing the process from the
serving goroutine - "fatal serve: open no-such.pem: no such file or directory" -
rather than through the assertion. That still demonstrates the defect, because
the process could only get there after startServing had returned successfully
and the phase had been announced; it cannot be observed from inside the test,
because the fatal races the assertion that would report it.

Also: the redis-backed queue tests now fail instead of skipping when CI is set
and GO_ADMIN_TEST_REDIS_ADDR is not. A workflow that renamed the variable or
dropped the service would otherwise stay green while those two tests quietly did
nothing - the same shape as the defect they exist to cover. Locally, with no CI
in the environment, they still skip.
2026-09-05 22:41:53 +08:00
zhangwenjian 36f2549172 test: pin BeforeRouter to the moment before the engine exists
buildRouter is split out of run() so the order it establishes can be asserted:
the phase is announced, then initRouter builds the engine, then runStartupHooks
drains the registries.

The test covers both halves of the distinction the contract draws. A
BeforeRouter callback sees no engine - that is what the phase means, the last
point at which a module can still affect how routes are built. A callback in the
before registry, two lines later, sees one. The names invite treating them as
the same moment and they are not.

No database is involved. AuthInit reads ApplicationConfig.Mode and JwtConfig and
nothing else, and building a router registers handlers rather than calling them,
so the whole sequence runs in a package test with two package-level values set.

freshRuntime swaps the global runtime for the duration: runStartupHooks seals
the registries it drains, and a sealed registry silently drops everything
registered afterwards, which would leave every later test in this binary passing
while proving nothing.

The counter-proof compiles and fails - announcing the phase after initRouter
reports "BeforeRouter saw engine &{...}, want nil".
2026-09-05 22:31:12 +08:00
zhangwenjian d52dca1cb6 feat: announce BeforeRouter and AfterListen, and bind before either
Two phases are now announced from the command that serves traffic, so a module
can attach to them instead of being called by name from here.

The listener is opened by this goroutine rather than left to ListenAndServe,
which binds on the goroutine that serves. That mattered for the phase: a hook
on AfterListen is promised a reachable port, and with the bind happening out of
sight there was no way to keep that promise - "address already in use" surfaced
on a goroutine nobody read, after the banner had already announced the server
was up. It is now returned from run() and the process exits non-zero without
claiming anything.

AfterListen is announced synchronously. Moving it to a goroutine to save the
few milliseconds would let it overlap the shutdown, and on a fast SIGTERM the
cleanup callbacks could finish before the startup ones did.

What is left in the serving goroutine is still log.Fatal, deliberately: the
bind is no longer among the errors that reach it, so what remains is a serve
that failed after the port was taken, and carrying on would park the process on
<-quit with nothing serving. ServeTLS is the one case that can still fail
immediately, since it reads the certificate files - with ssl enabled a hook can
still run against a server on its way down. That is not a regression (the old
code printed the banner in the same situation) and it is not fixed here.

BeforeRouter is placed before initRouter, which is a different moment from the
before registry runStartupHooks drains: those callbacks run after the engine
has been built, not before it.

AfterListen is tested here, in one test rather than two because the phase seals
itself once it has run: a second test would find a closed registry and pass
while proving nothing. Both counter-proofs compile and fail - announcing on a
failed bind reports "AfterListen ran 1 times after a failed bind", and
`go RunPhase(...)` reports "ran 0 times, want 1" against the hook's own pause.

BeforeRouter's placement is not asserted in this commit. The test for it comes
with the buildRouter extraction later in this branch.
2026-09-05 22:31:11 +08:00