Skip to content

feat(gateway): add graceful shutdown with signal handling #41

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
Arpita01-work wants to merge 6 commits into
base: main
Choose a base branch
from
Open

feat(gateway): add graceful shutdown with signal handling #41

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b4d0043
feat(gateway): add graceful shutdown with signal handling
Arpita01-work Jan 5, 2026
d94d91f
feat(gateway): add in-flight request tracking and shutdown tests
Arpita01-work Jan 7, 2026
4b63721
Merge remote-tracking branch 'upstream/main' into graceful-shutdown-g…
Arpita01-work Jan 10, 2026
1be2d92
chore(gateway): sync go modules after merging main
Arpita01-work Jan 10, 2026
a432b1b
fix(gateway): integrate request tracking, add server timeouts, clean …
Arpita01-work Jan 10, 2026
c910d79
fix(gateway): integrate request tracking and harden graceful shutdown
Arpita01-work Jan 10, 2026
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
34 changes: 32 additions & 2 deletions gateway/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ import (
"strconv"
"strings"
"time"
"os/signal"
"syscall"
Comment on lines +23 to +24

@coderabbitai coderabbitai Bot Jan 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix indentation to use tabs instead of spaces.

The import statements appear to use spaces for indentation instead of tabs. Run gofmt or go fmt to ensure consistent formatting with Go standards.

🔎 Fix formatting 
-    "os/signal"
-    "syscall"
+	"os/signal"
+	"syscall"
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"os/signal"
"syscall"
"os/signal"
"syscall"
🤖 Prompt for AI Agents 
In @gateway/main.go around lines 20-21, The import block containing "os/signal"
and "syscall" in gateway/main.go uses space indentation instead of tabs; run
gofmt (or go fmt) to reformat the file so the import statements and surrounding
code use Go's standard tab indentation, or manually replace the leading spaces
with tabs in the import block so the "os/signal" and "syscall" lines align with
the rest of the imports.

Comment on lines +23 to +24

@greptile-apps greptile-apps Bot Jan 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Import formatting is incorrect - these imports have leading spaces and should align with the other imports above.

Suggested change
"os/signal"
"syscall"
"os/signal"
"syscall"

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI 
This is a comment left during a code review.
Path: gateway/main.go
Line: 23:24

Comment:
Import formatting is incorrect - these imports have leading spaces and should align with the other imports above.

```suggestion
	"os/signal"
	"syscall"
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.


"github.com/gin-contrib/cors"
"github.com/gin-gonic/gin"
Expand Down Expand Up @@ -167,8 +169,36 @@ func main() {
port = "3000"
}

log.Printf("Go Gateway running on port %s", port)
r.Run(":" + port)
addr := ":" + port
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated

srv := &http.Server{
Addr: addr,
Handler: r,
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

go func() {
log.Printf("[INFO] Gateway listening on %s", addr)
if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
log.Fatalf("[FATAL] listen error: %v", err)
}
}()
Comment on lines +205 to +210

@coderabbitai coderabbitai Bot Jan 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: log.Fatalf in goroutine prevents graceful shutdown.

Using log.Fatalf (which calls os.Exit()) inside the server goroutine will immediately terminate the program if ListenAndServe returns an error, bypassing all graceful shutdown logic on lines 186-201. For example, if the port is already in use, the program exits without cleanup.

🔎 Proposed fix using error channel 
+	errChan := make(chan error, 1)
+
 	go func() {
 		log.Printf("[INFO] Gateway listening on %s", addr)
-		if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
-			log.Fatalf("[FATAL] listen error: %v", err)
-		}
+		errChan <- srv.ListenAndServe()
 	}()
 
 	// ---- Graceful shutdown ----
 	quit := make(chan os.Signal, 1)
 	signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
 
-	<-quit
-	log.Println("[INFO] Shutdown signal received, draining connections...")
+	select {
+	case err := <-errChan:
+		if err != nil && err != http.ErrServerClosed {
+			log.Fatalf("[FATAL] Server failed to start: %v", err)
+		}
+		return
+	case <-quit:
+		log.Println("[INFO] Shutdown signal received, draining connections...")
+	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
 	if err := srv.Shutdown(ctx); err != nil {
 		log.Printf("[ERROR] Server forced to shutdown: %v", err)
 	} else {
 		log.Println("[OK] Server shutdown completed")
 	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
go func() {
log.Printf("[INFO] Gateway listening on %s", addr)
if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
log.Fatalf("[FATAL] listen error: %v", err)
}
}()
errChan := make(chan error, 1)
go func() {
log.Printf("[INFO] Gateway listening on %s", addr)
errChan <- srv.ListenAndServe()
}()
// ---- Graceful shutdown ----
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
select {
case err := <-errChan:
if err != nil && err != http.ErrServerClosed {
log.Fatalf("[FATAL] Server failed to start: %v", err)
}
return
case <-quit:
log.Println("[INFO] Shutdown signal received, draining connections...")
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := srv.Shutdown(ctx); err != nil {
log.Printf("[ERROR] Server forced to shutdown: %v", err)
} else {
log.Println("[OK] Server shutdown completed")
}
🤖 Prompt for AI Agents 
In @gateway/main.go around lines 179-184, The goroutine currently calls
log.Fatalf on srv.ListenAndServe errors which exits immediately and skips the
graceful shutdown logic around the main shutdown handling; instead remove
log.Fatalf and propagate the error to the main goroutine via an error channel
(e.g., create errCh before launching the goroutine), have the goroutine send any
non-nil, non-http.ErrServerClosed error into errCh after logging, and let the
main select/receive on errCh alongside the existing shutdown signals so the main
goroutine performs the cleanup and graceful shutdown when ListenAndServe fails
on srv (address `addr`) rather than calling os.Exit inside the goroutine.


// ---- Graceful shutdown ----
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)

<-quit
log.Println("[INFO] Shutdown signal received, draining connections...")

ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()

if err := srv.Shutdown(ctx); err != nil {
log.Printf("[ERROR] Server forced to shutdown: %v", err)
} else {
log.Println("[OK] Server shutdown completed")
}

Comment on lines +212 to +234

@coderabbitai coderabbitai Bot Jan 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check timeout configurations in gateway directory
rg -n "time\.(Second|Minute|Duration)" gateway/ -A 2 -B 2

Repository: AnkanMisra/MicroAI-Paygate

Length of output: 11091

🏁 Script executed:

#!/bin/bash
# Also check around line 164 specifically for AI timeout
sed -n '150,180p' gateway/main.go

Repository: AnkanMisra/MicroAI-Paygate

Length of output: 958

🏁 Script executed:

#!/bin/bash
# Look for all timeout-related constants or configurations
rg -n "(timeout|TIMEOUT|Timeout)" gateway/ -i

Repository: AnkanMisra/MicroAI-Paygate

Length of output: 7862

Consider extending shutdown timeout to avoid race with AI request deadlines.

The graceful shutdown correctly uses srv.Shutdown() with a 30-second timeout to drain connections. However, both the AI endpoint timeout and shutdown timeout are set to 30 seconds, which creates a tight race condition: a request starting just before shutdown receives a fresh 30-second deadline that effectively expires at the same moment as the shutdown grace period, potentially aborting long-running requests before they complete naturally.

The timeouts are configurable (AI_REQUEST_TIMEOUT_SECONDS defaults to 30, shutdown hardcoded to 30). Consider setting the shutdown timeout to at least AI_TIMEOUT + buffer (e.g., 35–40 seconds) to ensure in-flight AI requests can either complete or timeout gracefully before forced shutdown kicks in.

🤖 Prompt for AI Agents 
In @gateway/main.go around lines 186-201, The shutdown grace period is hardcoded
to 30s and can race with AI request deadlines; change the context timeout used
in the graceful shutdown (the context.WithTimeout call that wraps srv.Shutdown)
to derive its duration from the configured AI_REQUEST_TIMEOUT_SECONDS plus a
small buffer (e.g., +5–10s) instead of the fixed 30s. Read the existing AI
timeout configuration (AI_REQUEST_TIMEOUT_SECONDS or the function/const that
provides it), compute shutdownTimeout :=
time.Duration(aiTimeoutSeconds+buffer)*time.Second, and pass that into
context.WithTimeout so srv.Shutdown uses the extended timeout.

Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
Comment on lines +213 to +234

@coderabbitai coderabbitai Bot Jan 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Race condition: wait for in-flight requests after stopping new connections.

The current sequence waits for in-flight requests (lines 219-223) before calling srv.Shutdown() (line 229), which means the server continues accepting new connections during the wait. This creates a race:

  1. Line 219 checks active count (e.g., 1 request)
  2. A new request arrives and is accepted by the still-running server
  3. Lines 220-223 wait for the original request to complete
  4. Line 229 calls srv.Shutdown(), which must now wait for the new request

The standard graceful shutdown pattern is to call srv.Shutdown() first—it stops accepting new connections and waits for existing ones to complete (up to the timeout). The manual WaitForInFlightRequests() wait before shutdown undermines this.

🔧 Recommended fix

Either remove the manual wait and rely on srv.Shutdown() alone:

 	<-quit
 	log.Println("[INFO] Shutdown signal received, draining connections...")
 
-	active := GetActiveRequestCount()
-	if active > 0 {
-		log.Printf("[INFO] Waiting for %d in-flight request(s)...", active)
-		WaitForInFlightRequests()
-		log.Println("[INFO] All in-flight requests completed")
-	}
-
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
 	if err := srv.Shutdown(ctx); err != nil {
 		log.Printf("[ERROR] Server forced to shutdown: %v", err)
 	} else {
 		log.Println("[OK] Server shutdown completed")
 	}

Or, if logging the active count is important, just log it without the manual wait:

 	<-quit
 	log.Println("[INFO] Shutdown signal received, draining connections...")
 
 	active := GetActiveRequestCount()
 	if active > 0 {
 		log.Printf("[INFO] %d in-flight request(s) detected, waiting for completion...", active)
-		WaitForInFlightRequests()
-		log.Println("[INFO] All in-flight requests completed")
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
 	if err := srv.Shutdown(ctx); err != nil {
 		log.Printf("[ERROR] Server forced to shutdown: %v", err)
 	} else {
 		log.Println("[OK] Server shutdown completed")
 	}

srv.Shutdown() internally waits for connections to idle, making the explicit WaitForInFlightRequests() redundant and potentially harmful.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
<-quit
log.Println("[INFO] Shutdown signal received, draining connections...")
active := GetActiveRequestCount()
if active > 0 {
log.Printf("[INFO] Waiting for %d in-flight request(s)...", active)
WaitForInFlightRequests()
log.Println("[INFO] All in-flight requests completed")
}
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := srv.Shutdown(ctx); err != nil {
log.Printf("[ERROR] Server forced to shutdown: %v", err)
} else {
log.Println("[OK] Server shutdown completed")
}
quit := make(chan os.Signal, 1)
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
<-quit
log.Println("[INFO] Shutdown signal received, draining connections...")
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
if err := srv.Shutdown(ctx); err != nil {
log.Printf("[ERROR] Server forced to shutdown: %v", err)
} else {
log.Println("[OK] Server shutdown completed")
}
🤖 Prompt for AI Agents 
In @gateway/main.go around lines 213 - 234, The shutdown sequence currently
calls GetActiveRequestCount() and WaitForInFlightRequests() before
srv.Shutdown(), which allows the server to keep accepting new connections and
creates a race; fix by removing the manual wait and invoking srv.Shutdown(ctx)
first (so the server stops accepting new connections and waits for in-flight
requests), and if you need visibility keep the GetActiveRequestCount() log but
do not call WaitForInFlightRequests() prior to srv.Shutdown(); ensure you still
use context.WithTimeout(...) and handle the error from srv.Shutdown(ctx) as
before.

}

// handleSummarize handles POST /api/ai/summarize requests. It validates
Expand Down
Loading