Skip to content

refactor: improve server lifecycle management and context handling #467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
e-repo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

refactor: improve server lifecycle management and context handling #467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 80 additions & 57 deletions internal/app/app.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package app

import (
"context"
"errors"
"fmt"
"os"
"os/signal"
"syscall"
"time"

"github.com/evrone/go-clean-template/config"
amqprpc "github.com/evrone/go-clean-template/internal/controller/amqp_rpc"
Expand All @@ -31,20 +33,30 @@ import (
rmqRPCServer "github.com/evrone/go-clean-template/pkg/rabbitmq/rmq_rpc/server"
"github.com/evrone/go-clean-template/pkg/tracing"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"golang.org/x/sync/errgroup"
pbgrpc "google.golang.org/grpc"
)

const (
_tracingShutdownTimeout = 5 * time.Second
)

// Server represents a service that can be started and gracefully stopped.
type Server interface {

@soltanoff soltanoff Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Start(ctx context.Context) error
}

type useCases struct {
translation usecase.Translation
user usecase.User
task usecase.Task
}

type servers struct {
rmq *rmqRPCServer.Server
nats *natsRPCServer.Server
grpc *grpcserver.Server
http *httpserver.Server
rmq Server
nats Server
grpc Server
http Server
}

func initUseCases(pg *postgres.Postgres, jwtManager *jwt.Manager) useCases {
Expand Down Expand Up @@ -99,58 +111,12 @@ func initServers(cfg *config.Config, uc useCases, jwtManager *jwt.Manager, l log
}
}

func (s *servers) startServers() {
s.rmq.Start()
s.nats.Start()
s.grpc.Start()
s.http.Start()
}

func (s *servers) waitForShutdown(l logger.Interface) {
interrupt := make(chan os.Signal, 1)
signal.Notify(interrupt, os.Interrupt, syscall.SIGTERM)

var err error

select {
case sig := <-interrupt:
l.Info("app - Run - signal: %s", sig.String())
case err = <-s.http.Notify():
l.Error(fmt.Errorf("app - Run - httpServer.Notify: %w", err))
case err = <-s.grpc.Notify():
l.Error(fmt.Errorf("app - Run - grpcServer.Notify: %w", err))
case err = <-s.rmq.Notify():
l.Error(fmt.Errorf("app - Run - rmqServer.Notify: %w", err))
case err = <-s.nats.Notify():
l.Error(fmt.Errorf("app - Run - natsServer.Notify: %w", err))
}

s.shutdownServers(l)
}

func (s *servers) shutdownServers(l logger.Interface) {
if err := s.http.Shutdown(); err != nil {
l.Error(fmt.Errorf("app - Run - httpServer.Shutdown: %w", err))
}

if err := s.grpc.Shutdown(); err != nil {
l.Error(fmt.Errorf("app - Run - grpcServer.Shutdown: %w", err))
}

if err := s.rmq.Shutdown(); err != nil {
l.Error(fmt.Errorf("app - Run - rmqServer.Shutdown: %w", err))
}

if err := s.nats.Shutdown(); err != nil {
l.Error(fmt.Errorf("app - Run - natsServer.Shutdown: %w", err))
}
}

// Run creates objects via constructors.
// Run creates objects via constructors and starts the application.
func Run(cfg *config.Config) {
l := logger.New(cfg.Log.Level)

ctx := context.Background()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

// Tracing
shutdownTracing, err := tracing.New(ctx, tracing.Config{
Expand All @@ -165,7 +131,10 @@ func Run(cfg *config.Config) {
l.Fatal(fmt.Errorf("app - Run - tracing.New: %w", err))
}
defer func() {
if err := shutdownTracing(ctx); err != nil {
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), _tracingShutdownTimeout)
defer shutdownCancel()

if err := shutdownTracing(shutdownCtx); err != nil {
l.Error(fmt.Errorf("app - Run - shutdownTracing: %w", err))
}
}()
Expand All @@ -180,8 +149,62 @@ func Run(cfg *config.Config) {
// JWT
jwtManager := jwt.New(cfg.JWT.Secret, cfg.JWT.TokenExpiry)

// Initialize use cases and servers
uc := initUseCases(pg, jwtManager)
s := initServers(cfg, uc, jwtManager, l)
s.startServers()
s.waitForShutdown(l)
srv := initServers(cfg, uc, jwtManager, l)

// Start all servers and wait for shutdown signal
runServers(ctx, cancel, srv, l)

l.Info("app - Run - application stopped gracefully")
}

func runServers(ctx context.Context, cancel context.CancelFunc, srv servers, l logger.Interface) {
g, gCtx := errgroup.WithContext(ctx)

g.Go(func() error {
l.Info("app - Run - starting HTTP server")

return srv.http.Start(gCtx)
})

g.Go(func() error {
l.Info("app - Run - starting gRPC server")

return srv.grpc.Start(gCtx)
})

g.Go(func() error {
l.Info("app - Run - starting RabbitMQ RPC server")

return srv.rmq.Start(gCtx)
})

g.Go(func() error {
l.Info("app - Run - starting NATS RPC server")

return srv.nats.Start(gCtx)
})

// Wait for interrupt signal
g.Go(func() error {
sigCh := make(chan os.Signal, 1)

signal.Notify(sigCh, os.Interrupt, syscall.SIGTERM)

@soltanoff soltanoff Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

defer signal.Stop(sigCh)

select {
case sig := <-sigCh:
l.Info("app - Run - received signal: %s", sig.String())
cancel()
case <-gCtx.Done():
}

return nil
})

// Wait for all servers to finish
if err := g.Wait(); err != nil && !errors.Is(err, context.Canceled) {

@soltanoff soltanoff Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

l.Error(fmt.Errorf("app - Run - servers stopped with error: %w", err))
}
}
95 changes: 39 additions & 56 deletions pkg/grpcserver/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,36 @@ package grpcserver

import (
"context"
"errors"
"fmt"
"net"
"time"

"github.com/evrone/go-clean-template/pkg/logger"
"golang.org/x/sync/errgroup"
pbgrpc "google.golang.org/grpc"
)

const (
_defaultAddr = ":80"
_defaultAddr = ":80"
_defaultShutdownTimeout = 3 * time.Second
)

// Server -.
type Server struct {
ctx context.Context
eg *errgroup.Group
App *pbgrpc.Server

App *pbgrpc.Server
notify chan error
address string
serverOpts []pbgrpc.ServerOption
address string
serverOpts []pbgrpc.ServerOption
shutdownTimeout time.Duration

@soltanoff soltanoff Aug 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


logger logger.Interface
}

// New -.
func New(l logger.Interface, opts ...Option) *Server {
group, ctx := errgroup.WithContext(context.Background())
group.SetLimit(1)

s := &Server{
ctx: ctx,
eg: group,
notify: make(chan error, 1),
address: _defaultAddr,
logger: l,
address: _defaultAddr,
shutdownTimeout: _defaultShutdownTimeout,
logger: l,
}

for _, opt := range opts {
Expand All @@ -49,53 +43,42 @@ func New(l logger.Interface, opts ...Option) *Server {
return s
}

// Start -.
func (s *Server) Start() {
s.eg.Go(func() error {
var lc net.ListenConfig
// Start starts the gRPC server and blocks until context is canceled.
func (s *Server) Start(ctx context.Context) error {
lc := net.ListenConfig{}

ln, err := lc.Listen(s.ctx, "tcp", s.address)
if err != nil {
s.notify <- err
ln, err := lc.Listen(ctx, "tcp", s.address)
if err != nil {
return fmt.Errorf("listen: %w", err)
}

close(s.notify)
s.logger.Info("grpc server - Server - Started on %s", s.address)

return err
}
// Start graceful shutdown goroutine
go func() {
<-ctx.Done()
s.logger.Info("grpc server - Server - Shutting down...")

err = s.App.Serve(ln)
if err != nil {
s.notify <- err
stopCh := make(chan struct{})

close(s.notify)
go func() {
s.App.GracefulStop()
close(stopCh)
}()

return err
select {
case <-stopCh:
s.logger.Info("grpc server - Server - Shutdown complete")
case <-time.After(s.shutdownTimeout):
s.logger.Info("grpc server - Server - Shutdown timeout, forcing stop")
s.App.Stop()
}
}()

return nil
})

s.logger.Info("grpc server - Server - Started")
}

// Notify -.
func (s *Server) Notify() <-chan error {
return s.notify
}

// Shutdown -.
func (s *Server) Shutdown() error {
var shutdownErrors []error

s.App.GracefulStop()

err := s.eg.Wait()
if err != nil && !errors.Is(err, context.Canceled) {
s.logger.Error(err, "grpc server - Server - Shutdown - s.eg.Wait")
shutdownErrors = append(shutdownErrors, err)
// Serve blocks until server stops
if err := s.App.Serve(ln); err != nil {
return fmt.Errorf("serve: %w", err)
}

s.logger.Info("grpc server - Server - Shutdown")

return errors.Join(shutdownErrors...)
return ctx.Err()
}
Loading