From 2fd908d42d792e2e284e2d2768d2e867a48b508c Mon Sep 17 00:00:00 2001 From: "d.reznichenko" Date: Tue, 3 Dec 2024 17:42:00 +0100 Subject: [PATCH 1/2] added use of errgroup's ctx to handle feiled http server startup gracefully --- server.go | 5 +++-- server_test.go | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/server.go b/server.go index e99e31a..1dd8a65 100644 --- a/server.go +++ b/server.go @@ -84,7 +84,8 @@ func (s *Server) Start(ctx context.Context) error { shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) defer shutdownCancel() - g, _ := errgroup.WithContext(ctx) + // creates ctx which will be canceled on first failed goroutine + g, ctx := errgroup.WithContext(ctx) // Start the server in a new goroutine within the errgroup g.Go(func() error { @@ -166,7 +167,7 @@ func (s *Server) Close(ctx context.Context) error { // signalChan sets up a channel to listen for OS signals for shutdown func signalChan() <-chan os.Signal { stop := make(chan os.Signal, 1) - signal.Notify(stop, os.Interrupt, syscall.SIGTERM) + signal.Notify(stop, os.Interrupt, syscall.SIGTERM, os.Kill) return stop } diff --git a/server_test.go b/server_test.go index 441eed0..34c528c 100644 --- a/server_test.go +++ b/server_test.go @@ -46,7 +46,7 @@ func TestServer(t *testing.T) { cancel() // Wait for server to shut down with timeout - shutdownTimeout := time.After(5 * time.Second) + shutdownTimeout := time.After(time.Second) select { case err := <-serverErr: require.True(t, err == nil || errors.Is(err, context.Canceled), @@ -59,3 +59,40 @@ func TestServer(t *testing.T) { _, err = http.Get(fmt.Sprintf("http://%s", listenAddr)) require.Error(t, err, "Expected error after server shutdown") } + +func TestErrorServerStart(t *testing.T) { + listenAddr := "localhost:9999" + + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = fmt.Fprintln(w, "Hello, World!") + }) + server, err := httpserver.New(listenAddr, handler) + require.NoError(t, err, "Unexpected error creating server") + + // Create a context with cancel for server control + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Channel to catch server errors, no need for buffer because we catch error in current test control flow + serverErr := make(chan error) + + // Start the server in a goroutine + go func() { + serverErr <- server.Start(ctx) + }() + // Start server will wail because of already used port + go func() { + serverErr <- server.Start(ctx) + }() + + select { + case err = <-serverErr: + require.ErrorIs(t, err, httpserver.ErrServerStart, + "Expected ErrServerStart error, got: %v", err) + case <-time.After(time.Second): + t.Fatal("Server shutdown timed out") + } + // Verify server is no longer accepting connections + _, err = http.Get(fmt.Sprintf("http://%s", listenAddr)) + require.Error(t, err, "Expected error after server shutdown") +} From b946e36cf82f4aa5cbf6d00c0d617b68438789e1 Mon Sep 17 00:00:00 2001 From: "d.reznichenko" Date: Tue, 3 Dec 2024 18:29:31 +0100 Subject: [PATCH 2/2] removed os.Kill signal --- server.go | 2 +- server_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server.go b/server.go index 1dd8a65..34b8032 100644 --- a/server.go +++ b/server.go @@ -167,7 +167,7 @@ func (s *Server) Close(ctx context.Context) error { // signalChan sets up a channel to listen for OS signals for shutdown func signalChan() <-chan os.Signal { stop := make(chan os.Signal, 1) - signal.Notify(stop, os.Interrupt, syscall.SIGTERM, os.Kill) + signal.Notify(stop, os.Interrupt, syscall.SIGTERM) return stop } diff --git a/server_test.go b/server_test.go index 34c528c..11a2432 100644 --- a/server_test.go +++ b/server_test.go @@ -16,7 +16,7 @@ func TestServer(t *testing.T) { listenAddr := "localhost:9999" handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - fmt.Fprintln(w, "Hello, World!") + _, _ = fmt.Fprintln(w, "Hello, World!") }) server, err := httpserver.New(listenAddr, handler) require.NoError(t, err, "Unexpected error creating server")