From 0731349d0a2681df022975c814102f57fd9b18c7 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 4 Oct 2026 07:08:32 +0000 Subject: [PATCH] Let fx own signals and the exit code in the server example (closes #86) The lifecycle example computed an exit code that never reached os.Exit, installed its own SIGINT/SIGTERM handler beside the one fx's Run() installs, and exited from a goroutine when Sentry failed to start, so no stop hook ran. Now only fx handles those signals; a listen error asks fx to shut down with exit code 1 through fx.Shutdowner; Sentry's error is returned from the server's start hook; and the server's stop hook shuts the HTTP server down within 5 seconds and fails when requests are still running. A new paragraph says who owns signals and the exit code. SIGPIPE is still ignored, now in main. Model: opus-5-5 --- TODO.md | 9 +++ prompts/GO_HTTP_SERVER_CONVENTIONS.md | 112 +++++++++++++------------- 2 files changed, 64 insertions(+), 57 deletions(-) diff --git a/TODO.md b/TODO.md index 769cf3a..ee679eb 100644 --- a/TODO.md +++ b/TODO.md @@ -21,6 +21,15 @@ fmt-check, and commit. # Completed Steps +- 2026-10-04: Fixed the server lifecycle example in + `prompts/GO_HTTP_SERVER_CONVENTIONS.md` (issue 86). Only fx handles SIGINT and + SIGTERM, and `Run()` in `main` exits with the shutdown's exit code. A listen + error asks fx to shut down with exit code 1 through `fx.Shutdowner`; a Sentry + start failure is returned from the server's start hook instead of calling + `os.Exit` from a goroutine, so the stop hooks of what had started still run. + The server's stop hook shuts the HTTP server down within 5 seconds and fails + when requests are still running. A new paragraph says who owns signals and the + exit code. - 2026-10-04: `package.json` now has `"license": "MIT"`, matching `LICENSE`, so yarn no longer prints "No license field" when `script/bootstrap` runs it inside the Docker phases (issue 76). That was the only yarn warning there. diff --git a/prompts/GO_HTTP_SERVER_CONVENTIONS.md b/prompts/GO_HTTP_SERVER_CONVENTIONS.md index 84e1230..d700d3d 100644 --- a/prompts/GO_HTTP_SERVER_CONVENTIONS.md +++ b/prompts/GO_HTTP_SERVER_CONVENTIONS.md @@ -106,6 +106,9 @@ project-root/ package main import ( + "os/signal" + "syscall" + "yourproject/internal/config" "yourproject/internal/database" "yourproject/internal/globals" @@ -126,6 +129,9 @@ func main() { globals.Appname = Appname globals.Version = Version + // A write to a closed stdout or stderr must not end the process. + signal.Ignore(syscall.SIGPIPE) + fx.New( fx.Provide( config.New, @@ -198,7 +204,8 @@ Providers are resolved automatically by fx, but conceptually follow this order: Database) 6. `middleware.New` - Middleware (depends on Logger, Globals, Config) 7. `handlers.New` - Handlers (depends on Logger, Globals, Database, Healthcheck) -8. `server.New` - Server (depends on all above) +8. `server.New` - Server (depends on all above, and on `fx.Shutdowner`, which fx + provides itself) --- @@ -217,16 +224,14 @@ type ServerParams struct { Config *config.Config Middleware *middleware.Middleware Handlers *handlers.Handlers + Shutdowner fx.Shutdowner } type Server struct { startupTime time.Time port int - exitCode int sentryEnabled bool log *slog.Logger - ctx context.Context - cancelFunc context.CancelFunc httpServer *http.Server router *chi.Mux params ServerParams @@ -248,13 +253,15 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) { lc.Append(fx.Hook{ OnStart: func(ctx context.Context) error { s.startupTime = time.Now() - go s.Run() - return nil - }, - OnStop: func(ctx context.Context) error { - // Server shutdown logic + if err := s.enableSentry(); err != nil { + return err + } + s.SetupRoutes() + s.httpServer = s.newHTTPServer() + go s.serveUntilShutdown() return nil }, + OnStop: s.cleanShutdown, }) return s, nil } @@ -264,23 +271,25 @@ func New(lc fx.Lifecycle, params ServerParams) (*Server, error) { ```go // internal/server/http.go -func (s *Server) serveUntilShutdown() { - listenAddr := fmt.Sprintf(":%d", s.params.Config.Port) - s.httpServer = &http.Server{ - Addr: listenAddr, +func (s *Server) newHTTPServer() *http.Server { + return &http.Server{ + Addr: fmt.Sprintf(":%d", s.params.Config.Port), ReadTimeout: 10 * time.Second, WriteTimeout: 10 * time.Second, MaxHeaderBytes: 1 << 20, Handler: s, } +} - s.SetupRoutes() - - s.log.Info("http begin listen", "listenaddr", listenAddr) +// serveUntilShutdown returns when the stop hook shuts the HTTP server down. +// If it stops for any other reason, such as its port being taken, it asks fx +// to shut down with exit code 1. +func (s *Server) serveUntilShutdown() { + s.log.Info("http begin listen", "listenaddr", s.httpServer.Addr) if err := s.httpServer.ListenAndServe(); err != nil && err != http.ErrServerClosed { s.log.Error("listen error", "error", err) - if s.cancelFunc != nil { - s.cancelFunc() + if err := s.params.Shutdowner.Shutdown(fx.ExitCode(1)); err != nil { + s.log.Error("shutdown request failed", "error", err) } } } @@ -292,43 +301,30 @@ func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { ## Signal Handling and Graceful Shutdown +fx owns SIGINT, SIGTERM and the exit code. `Run()` in `main` waits for one of +those signals or for a call to `Shutdown()` on `fx.Shutdowner`, runs the stop +hooks, and exits 0 after a signal, or with the code the call gave in +`fx.ExitCode`. It exits 1 instead when a start hook or a stop hook returns an +error; when a start hook fails, fx first runs the stop hooks of everything +already started. No other code calls `signal.Notify` or `os.Exit`: the listen +error above asks fx to shut down with `fx.ExitCode(1)`, and a Sentry start +failure is returned from the start hook. Each component releases its own +resources in its own stop hook, which fx runs in the reverse order of start. + ```go -func (s *Server) serve() int { - s.ctx, s.cancelFunc = context.WithCancel(context.Background()) - - // Signal watcher - go func() { - c := make(chan os.Signal, 1) - signal.Ignore(syscall.SIGPIPE) - signal.Notify(c, os.Interrupt, syscall.SIGTERM) - sig := <-c - s.log.Info("signal received", "signal", sig) - if s.cancelFunc != nil { - s.cancelFunc() - } - }() - - go s.serveUntilShutdown() - - for range s.ctx.Done() { - } - s.cleanShutdown() - return s.exitCode -} - -func (s *Server) cleanShutdown() { - s.exitCode = 0 - ctxShutdown, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second) - if err := s.httpServer.Shutdown(ctxShutdown); err != nil { - s.log.Error("server clean shutdown failed", "error", err) - } - if shutdownCancel != nil { - shutdownCancel() - } - s.cleanupForExit() +// cleanShutdown is the server's stop hook. It fails when requests are still +// running after 5 seconds. +func (s *Server) cleanShutdown(ctx context.Context) error { + ctxShutdown, shutdownCancel := context.WithTimeout(ctx, 5*time.Second) + defer shutdownCancel() + err := s.httpServer.Shutdown(ctxShutdown) if s.sentryEnabled { sentry.Flush(2 * time.Second) } + if err != nil { + return fmt.Errorf("http server shutdown: %w", err) + } + return nil } ``` @@ -1160,11 +1156,11 @@ s.router.Get("/.well-known/healthcheck", s.h.HandleHealthCheck()) Sentry is conditionally enabled based on `SENTRY_DSN` environment variable: ```go -func (s *Server) enableSentry() { +func (s *Server) enableSentry() error { s.sentryEnabled = false if s.params.Config.SentryDSN == "" { - return + return nil } err := sentry.Init(sentry.ClientOptions{ @@ -1172,15 +1168,17 @@ func (s *Server) enableSentry() { Release: fmt.Sprintf("%s-%s", s.params.Globals.Appname, s.params.Globals.Version), }) if err != nil { - s.log.Error("sentry init failure", "error", err) - os.Exit(1) - return + return fmt.Errorf("sentry init failure: %w", err) } s.log.Info("sentry error reporting activated") s.sentryEnabled = true + return nil } ``` +The server's start hook calls `enableSentry()` and returns its error, so a DSN +Sentry rejects stops startup and fx exits 1. + Sentry middleware with repanic (bubbles panics to chi's Recoverer): ```go @@ -1192,7 +1190,7 @@ if s.sentryEnabled { } ``` -Flush Sentry on shutdown: +Flush Sentry in the server's stop hook, `cleanShutdown()`: ```go if s.sentryEnabled {