Repository: gofr-dev/gofr

Issue:
Complete inflight requests after SIGTERM
Unless I'm mistaken, when a SIGTERM is recieved gofr immediately exits closing all connections.

Would it be possible to gracefully handle the shutdown?

Off the top of my head the following might be useful, especially for those of us who plan to scale up/down:
- Ability to set a threshold (seconds) where gofr will still accept new connections after a sigterm
- Ability to set a threshold (seconds) where gofr will continue to handle inflight requests after the threshold for new connections (above)

Traefik's lifecycle procedure might be a good example of how it could be handled: https://doc.traefik.io/traefik/routing/entrypoints/#lifecycle 

Discussion:
@Umang01-hash: @sparkacus GoFr has a graceful shutdown implemented, we added it using this PR: https://github.com/gofr-dev/gofr/pull/837

And regarding ability to set thresholds we can discuss about it. 
@sparkacus: @Umang01-hash You're right, apologies.

I'm using the 'air' package for hot reloading, so a CTRL+C resulted in the process being killed instantly, but that's not gofrs doing.... sorry.

I notice that the shutdown timeout is hardcoded to 30s - It might be useful to allow us to override that via a config i.e. I might want to lower or increase the value. An example could be when using SPOT instances on AWS, from my understanding they'll kill the instance regardless 30s after SIGTERM, so we might want to lower it to ensure gofr can completly shutdown gracefully, which is useful for observability purposes. 

EDIT: I notice that gofr does have a REQUEST_TIMEOUT config which would likely resolve the example I provided (I'm certainly not advocating that requests should last longer than 30s :), but sometimes there are reasons)

Concerning the thresholds, it certainly would be useful, especially if gofr is targeted directly from an LB (It might not be a kubernetes service which will remove the target from the endpointslices). Ideally when a SIGTERM is recieved it would immdiately fail the healthcheck endpoint, but being able to handle new requests for a period of time gives the LB an opertunity to detect the failed endpoint and remove it as a target, but at least those new connections will be served. @vikash: health check and liveness probe should fail after SIGTERM is received, even if the server is not currently closed, it should be able to finish all in-flight requests. 

SERVER_SHUTDOWN_GRACE_PERIOD or something like this can be added as well - current default of 30 second can be the default to ensure no changes in existing behaviour even after the change. @sparkacus: > health check and liveness probe should fail after SIGTERM is received, even if the server is not currently closed, it should be able to finish all in-flight requests.
> 
> SERVER_SHUTDOWN_GRACE_PERIOD or something like this can be added as well - current default of 30 second can be the default to ensure no changes in existing behaviour even after the change.

Yes, having the ability to tweak the shutdown period could be very useful. It may be that 30 seconds is too long, so it will help us optimise based on various circumstances.

Concerning the healthcheck/liveness probe, you're right, at the point of SIGTERM it should remove the IP from the endpoint slice. Kubernetes might have been a bad example because it handles these situations well.

Lets use another example. The gofr service is say hosted on an instance that is sitting behind a traditional LB e.g. AWS ALB... those target groups are based on interval health checks, so if the gofr service shuts down (lets assume not due to the instance being stopped), the ALB will still forward the request to the service which is now technically offline (it's in the graceful shutdown mode) and thus the request will fail. That target will only be removed once whatever healthcheck thresholds have been reached.

If gofr can still handle incoming requests during the graceful shutdown but return a non-200 response to the healthchecks, that should negate request failures. It also gives the administrator peace of mind that they can stop the service without cuasing an outage (although technically if they knew they were going to stop the service they'd remove the target first... but still)@Umang01-hash: Hi @sparkacus,

While the PR (#1529) introducing the new `SERVER_SHUTDOWN_THRESHOLD` config, which indeed adds flexibility to the graceful shutdown process.

Regarding the health check behavior, our current design follows the traditional approach: once SIGTERM is received, the server stops accepting new connections (**leading to errors like ECONNREFUSED**) while still processing inflight requests.  This method clearly signals to load balancers that the instance is going out of service, and modern load balancers (e.g., AWS ALB) are typically tuned to remove targets quickly based on these immediate health check failures.

In AWS environments that use autoscaling groups, this behavior fits well into deployment strategies like blue-green or rolling deployments. Typically, a new instance with the updated version is launched, and once it’s healthy, the old instance is automatically deregistered and terminated. This process ensures smooth transitions without needing to keep the health check endpoint reachable during shutdown.

For those who might need an alternative, the recommended approach is to adjust the load balancer or deployment configuration to fit their specific requirements.

Thanks.

Solution patch:
diff --git a/docs/references/configs/page.md b/docs/references/configs/page.md
index e135d155..3e982f3b 100644
--- a/docs/references/configs/page.md
+++ b/docs/references/configs/page.md
@@ -101,6 +101,13 @@ This document lists all the configuration options supported by the GoFr framewor
 -  CMD_LOGS_FILE
 -  File to save the logs in case of a CMD application
 
+---
+
+-  SHUTDOWN_GRACE_PERIOD
+-  Timeout duration for server shutdown process
+-  30s
+
+
 {% /table %}
 
 ## HTTP
diff --git a/pkg/gofr/gofr.go b/pkg/gofr/gofr.go
index 7b56bf25..14a47644 100644
--- a/pkg/gofr/gofr.go
+++ b/pkg/gofr/gofr.go
@@ -65,7 +65,6 @@ func (a *App) Shutdown(ctx context.Context) error {
 	}
 
 	if err != nil {
-		a.container.Logger.Errorf("error while shutting down: %v", err)
 		return err
 	}
 
diff --git a/pkg/gofr/run.go b/pkg/gofr/run.go
index e99aec6b..ae0067a8 100644
--- a/pkg/gofr/run.go
+++ b/pkg/gofr/run.go
@@ -19,19 +19,29 @@ func (a *App) Run() {
 	ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGINT, syscall.SIGTERM)
 	defer stop()
 
+	timeout, err := getShutdownTimeoutFromConfig(a.Config)
+	if err != nil {
+		a.Logger().Errorf("error parsing value of shutdown timeout from config: %v. Setting default timeout of 30 sec.", err)
+	}
+
 	// Goroutine to handle shutdown when context is canceled
 	go func() {
 		<-ctx.Done()
 
 		// Create a shutdown context with a timeout
-		shutdownCtx, done := context.WithTimeout(context.WithoutCancel(ctx), shutDownTimeout)
+		shutdownCtx, done := context.WithTimeout(context.WithoutCancel(ctx), timeout)
 		defer done()
 
 		if a.hasTelemetry() {
 			a.sendTelemetry(http.DefaultClient, false)
 		}
 
-		_ = a.Shutdown(shutdownCtx)
+		a.Logger().Infof("Shutting down server with a timeout of %v", timeout)
+
+		shutdownErr := a.Shutdown(shutdownCtx)
+		if shutdownErr != nil {
+			a.Logger().Debugf("Server shutdown failed: %v", shutdownErr)
+		}
 	}()
 
 	if a.hasTelemetry() {
diff --git a/pkg/gofr/shutdown.go b/pkg/gofr/shutdown.go
index 767dfef7..e2bb564f 100644
--- a/pkg/gofr/shutdown.go
+++ b/pkg/gofr/shutdown.go
@@ -3,6 +3,9 @@ package gofr
 import (
 	"context"
 	"errors"
+	"time"
+
+	"gofr.dev/pkg/gofr/config"
 )
 
 // ShutdownWithContext handles the shutdown process with context timeout.
@@ -29,3 +32,17 @@ func ShutdownWithContext(ctx context.Context, shutdownFunc func(ctx context.Cont
 		return err
 	}
 }
+
+func getShutdownTimeoutFromConfig(cfg config.Config) (time.Duration, error) {
+	value := cfg.GetOrDefault("SHUTDOWN_GRACE_PERIOD", "30s")
+	if value == "" {
+		return shutDownTimeout, nil
+	}
+
+	timeout, err := time.ParseDuration(value)
+	if err != nil {
+		return shutDownTimeout, err
+	}
+
+	return timeout, nil
+}



Test patch:
diff --git a/pkg/gofr/gofr_test.go b/pkg/gofr/gofr_test.go
index d45cbe93..1c240d43 100644
--- a/pkg/gofr/gofr_test.go
+++ b/pkg/gofr/gofr_test.go
@@ -690,7 +690,10 @@ func Test_UseMiddleware(t *testing.T) {
 			port:   port,
 		},
 		container: c,
-		Config:    config.NewMockConfig(map[string]string{"REQUEST_TIMEOUT": "5"}),
+		Config: config.NewMockConfig(map[string]string{
+			"REQUEST_TIMEOUT":       "5",
+			"SHUTDOWN_GRACE_PERIOD": "1s",
+		}),
 	}
 
 	app.UseMiddleware(testMiddleware)
diff --git a/pkg/gofr/shutdown_test.go b/pkg/gofr/shutdown_test.go
index 6a255534..7da17af5 100644
--- a/pkg/gofr/shutdown_test.go
+++ b/pkg/gofr/shutdown_test.go
@@ -5,7 +5,10 @@ import (
 	"testing"
 	"time"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"gofr.dev/pkg/gofr/config"
 )
 
 func TestShutdownWithContext_ContextTimeout(t *testing.T) {
@@ -39,3 +42,35 @@ func TestShutdownWithContext_SuccessfulShutdown(t *testing.T) {
 
 	require.NoError(t, err, "Expected successful shutdown without error")
 }
+
+func Test_getShutdownTimeoutFromConfig_Success(t *testing.T) {
+	tests := []struct {
+		name          string
+		configValue   string
+		expectedValue time.Duration
+	}{
+		{"Valid timeout", "1s", 1 * time.Second},
+		{"Empty timeout", "", 30 * time.Second},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mockConfig := config.NewMockConfig(map[string]string{
+				"SHUTDOWN_GRACE_PERIOD": tt.configValue,
+			})
+
+			timeout, err := getShutdownTimeoutFromConfig(mockConfig)
+			require.NoError(t, err)
+			assert.Equal(t, tt.expectedValue, timeout)
+		})
+	}
+}
+
+func Test_getShutdownTimeoutFromConfig_Error(t *testing.T) {
+	mockConfig := config.NewMockConfig(map[string]string{
+		"SHUTDOWN_GRACE_PERIOD": "invalid",
+	})
+
+	_, err := getShutdownTimeoutFromConfig(mockConfig)
+	require.Error(t, err)
+}