refactor: improve server lifecycle management and context handling

[e-repo]

Problem

The current server implementation had several architectural issues:

1. Context anti-pattern: Storing context.Context in server structs violates Go best practices (see official docs: "Do not store Contexts inside a struct type")

2. Redundant errgroup usage: Each server used errgroup.Group with SetLimit(1) for a single goroutine, adding unnecessary complexity

3. Duplicated error handling: Errors were sent both to notify channel and returned from errgroup, creating redundant code paths

4. Sequential shutdown: Servers shut down one by one, multiplying the total shutdown time (4 servers × 3s = 12s instead of ~3s)

5. Missing context propagation: Shutdown context wasn't properly isolated from cancellation state

Solution

Refactored server lifecycle management following production-ready patterns:

Server layer changes:

• Removed context.Context and errgroup.Group from server structs
• Replaced Start() + Notify() + Shutdown() pattern with single Start(ctx context.Context) error method
• Servers now block until context is canceled and handle graceful shutdown internally

Application layer changes:

• Introduced unified Server interface for all transport types
• Centralized lifecycle management using errgroup in app.Run()
• All servers start in parallel and share the same context
• Signal handling integrated into errgroup
• Proper shutdown context isolation using context.WithoutCancel()

[soltanoff]

Hi! Thanks for contributing. I'll review it 👀

[soltanoff]

Direction looks right — context out of the structs, no more errgroup with SetLimit(1) per server, parallel shutdown. build, vet and golangci-lint are all clean on the branch.

Comments are on the files. The rmq/nats one is the blocker: the cancelled context now reaches in-flight handlers, and messages get acked without the work being done.

Also worth adding a test or two — this is 195/279 lines of lifecycle and concurrency with none. Something like: server starts, context is cancelled, Start returns within a timeout, port is released.

One thing I checked that turned out fine: I thought grpc Serve returned before the detached GracefulStop finished draining RPCs. It doesn't — Serve blocks on <-s.done.Done() until stop() completes.

[soltanoff]

This passes the errgroup context down into the handler, and that context is cancelled on SIGTERM. It reaches the usecase and then pgx — internal/controller/amqp_rpc/v1/task.go:33 calls r.tk.Create(ctx, ...).

So during shutdown a handler that already opened a transaction gets context.Canceled from pgx, returns an error, serveCall publishes ErrInternalServer, and defer s.ack(d, false) acks the delivery anyway. The message is confirmed, the work was never done, and there is no redelivery. That happens on every deploy.

Before this change the handler context came from errgroup.WithContext(context.Background()) and was never cancelled during normal operation, so it is a regression.

Cheapest fix is to skip the ack when the handler failed with context.Canceled, so the message goes to another instance. The better one is to hand handlers a context that survives cancellation and drain them separately:

handlerCtx := context.WithoutCancel(ctx)
var inflight sync.WaitGroup

then wait on inflight with its own timeout before closing the connection, same idea as shutdownTimeout in httpserver.

[soltanoff]

Same as in the rmq server: ctx here is the errgroup context, so every in-flight handler sees context.Canceled the moment SIGTERM lands and the caller gets ErrInternalServer instead of a result.

[soltanoff]

Unsubscribe and Close run right after the context is cancelled, but handleMessage callbacks are still running on nats.go client goroutines and nothing waits for them. Their s.publish lands on a closed connection, so the caller times out instead of getting a reply. Unsubscribe also drops messages that were delivered but not processed yet.

nats.go has Drain for this — Subscription.Drain() / Conn.Drain() stop new deliveries, let the buffer finish and then close. Drain is asynchronous, so you still need to wait for it, either through nats.ClosedHandler or a bounded wait on conn.IsClosed().

[soltanoff]

Two things here.

The error is logged, then line 159 prints "application stopped gracefully" and Run returns with exit code 0. If the HTTP port is taken, the process exits successfully and an orchestrator will not restart it under a failure policy. Returning the error from runServers and calling l.Fatal on it would fix that.

The other one: every server returns ctx.Err() on a clean shutdown, so g.Wait() returns non-nil on the happy path and you have to filter context.Canceled out. That filter also swallows a real context.Canceled coming from pgx or an external client. Returning nil on graceful shutdown makes non-nil always mean failure.

[soltanoff]

signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM) does all of this, and then runServers no longer needs the cancel parameter either.

[soltanoff]

The comment promises a graceful stop, but there is no stop method — stopping is expressed by cancelling the context. Worth rewording.

[soltanoff]

Listen goes into a goroutine and the select below immediately waits on ctx.Done(). If cancellation arrives before fasthttp has bound — SIGTERM right after start, or another server in the group failing fast — ShutdownWithContext hits if s.ln == nil { return nil } in fasthttp server.go and does nothing, and its defer s.stop.Store(0) resets the stop flag. Start returns, and then this goroutine brings up a listening socket that nobody will ever close.

Narrow window, but reproducible. Signalling "listening" before entering the select, or waiting for this goroutine after shutdown, closes it.

[soltanoff]

This field has no Option, so it cannot be changed — httpserver has ShutdownTimeout in options.go. Either add one or keep it a plain constant.

Also, line 72 uses time.After, which keeps the timer alive until it fires; time.NewTimer with defer t.Stop() is cheaper.