mirror of
https://github.com/lukaszraczylo/graphql-monitoring-proxy.git
synced 2026-10-17 06:17:40 +00:00
fix(server): publish proxy app on listen and bound the shutdown drain
The shutdown component could observe the app before Listen ran. fasthttp then treated Shutdown as a no-op and reset its stop flag. The proxy started to serve after shutdown was requested. Publish httpProxyApp from the OnListen hook and clear it after Listen returns. Honor the shutdown context and retry until the drain completes or the context expires. Gate the tests on the publish signal with a bounded context.
This commit is contained in:
@@ -50,14 +50,46 @@ var (
|
||||
// shutdownHTTPProxy gracefully stops the HTTP proxy server. It is invoked from
|
||||
// the ShutdownManager during shutdown so server.Listen returns and the proxy
|
||||
// can finish in-flight requests.
|
||||
func shutdownHTTPProxy(_ context.Context) error {
|
||||
httpProxyMu.RLock()
|
||||
app := httpProxyApp
|
||||
httpProxyMu.RUnlock()
|
||||
if app != nil {
|
||||
return app.Shutdown()
|
||||
//
|
||||
// httpProxyApp is published by an OnListen hook just before fasthttp's
|
||||
// Serve() records its own listener, so a shutdown request arriving in that
|
||||
// narrow window would see no listener yet and ShutdownWithContext would
|
||||
// silently no-op instead of draining. Retry until StartHTTPProxy's Listen
|
||||
// call actually returns (httpProxyApp is cleared, proving the drain
|
||||
// happened) or ctx is done, so a shutdown racing server startup still stops
|
||||
// the server rather than letting it start serving after shutdown began.
|
||||
func shutdownHTTPProxy(ctx context.Context) error {
|
||||
for {
|
||||
httpProxyMu.RLock()
|
||||
app := httpProxyApp
|
||||
httpProxyMu.RUnlock()
|
||||
// A shutdown landing before OnListen has published httpProxyApp (app
|
||||
// is still nil here) is a no-op, same as "no server running" -
|
||||
// there is no fasthttp listener yet to stop. This function does not
|
||||
// close that window itself; main.go's ~1s wait after launching the
|
||||
// StartHTTPProxy goroutine (giving OnListen time to fire before
|
||||
// anything can call shutdown) is what closes it in practice.
|
||||
if app == nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
if err := app.ShutdownWithContext(ctx); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
httpProxyMu.RLock()
|
||||
stopped := httpProxyApp != app
|
||||
httpProxyMu.RUnlock()
|
||||
if stopped {
|
||||
return nil
|
||||
}
|
||||
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-time.After(2 * time.Millisecond):
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func StartHTTPProxy() error {
|
||||
@@ -75,9 +107,25 @@ func StartHTTPProxy() error {
|
||||
}
|
||||
|
||||
server := fiber.New(serverConfig)
|
||||
httpProxyMu.Lock()
|
||||
httpProxyApp = server
|
||||
httpProxyMu.Unlock()
|
||||
|
||||
// Publish httpProxyApp only once fiber has bound the listener and is about
|
||||
// to serve, not at construction time. Publishing earlier lets
|
||||
// shutdownHTTPProxy race server.Listen: fasthttp's ShutdownWithContext
|
||||
// no-ops when its listener isn't set yet, silently skipping the drain and
|
||||
// then letting Listen start serving after shutdown was requested.
|
||||
server.Hooks().OnListen(func(fiber.ListenData) error {
|
||||
httpProxyMu.Lock()
|
||||
httpProxyApp = server
|
||||
httpProxyMu.Unlock()
|
||||
return nil
|
||||
})
|
||||
defer func() {
|
||||
httpProxyMu.Lock()
|
||||
if httpProxyApp == server {
|
||||
httpProxyApp = nil
|
||||
}
|
||||
httpProxyMu.Unlock()
|
||||
}()
|
||||
|
||||
server.Use(cors.New(cors.Config{
|
||||
AllowOrigins: []string{"*"},
|
||||
|
||||
+67
-25
@@ -575,20 +575,25 @@ func TestStartHTTPProxy_StartsAndShutdown(t *testing.T) {
|
||||
errCh <- StartHTTPProxy()
|
||||
}()
|
||||
|
||||
// Wait for server to bind
|
||||
// Wait for the OnListen hook to publish httpProxyApp, i.e. for fiber to
|
||||
// actually be serving. A bare TCP dial is not a reliable "serving" signal:
|
||||
// the OS accepts connections into the listener's backlog as soon as it is
|
||||
// bound, before fiber's Serve loop starts pulling them off, so dialling a
|
||||
// freshly bound listener races server startup under -race.
|
||||
deadline := time.Now().Add(3 * time.Second)
|
||||
var conn net.Conn
|
||||
var serving bool
|
||||
for time.Now().Before(deadline) {
|
||||
conn, err = net.DialTimeout("tcp", fmt.Sprintf("127.0.0.1:%d", port), 100*time.Millisecond)
|
||||
if err == nil {
|
||||
httpProxyMu.RLock()
|
||||
serving = httpProxyApp != nil
|
||||
httpProxyMu.RUnlock()
|
||||
if serving {
|
||||
break
|
||||
}
|
||||
time.Sleep(50 * time.Millisecond)
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
}
|
||||
if conn == nil {
|
||||
t.Fatalf("server did not start on port %d within 3s", port)
|
||||
if !serving {
|
||||
t.Fatalf("server did not start serving on port %d within 3s", port)
|
||||
}
|
||||
_ = conn.Close()
|
||||
|
||||
// Send a health check to confirm it's serving
|
||||
httpResp, err := http.Get(fmt.Sprintf("http://127.0.0.1:%d/health?check_graphql=false&check_redis=false", port))
|
||||
@@ -599,6 +604,23 @@ func TestStartHTTPProxy_StartsAndShutdown(t *testing.T) {
|
||||
if httpResp.StatusCode != 200 {
|
||||
t.Errorf("want 200, got %d", httpResp.StatusCode)
|
||||
}
|
||||
|
||||
// Shut down and confirm StartHTTPProxy's server.Listen returns instead of
|
||||
// blocking forever, so in-flight requests get a chance to drain.
|
||||
shutdownCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
|
||||
defer cancel()
|
||||
if err := shutdownHTTPProxy(shutdownCtx); err != nil {
|
||||
t.Fatalf("shutdownHTTPProxy: %v", err)
|
||||
}
|
||||
|
||||
select {
|
||||
case err := <-errCh:
|
||||
if err != nil {
|
||||
t.Errorf("StartHTTPProxy returned error after shutdown: %v", err)
|
||||
}
|
||||
case <-time.After(3 * time.Second):
|
||||
t.Fatal("StartHTTPProxy did not return after shutdownHTTPProxy")
|
||||
}
|
||||
}
|
||||
|
||||
// Test_shutdownHTTPProxy verifies the graceful-shutdown helper: it is a no-op
|
||||
@@ -616,7 +638,12 @@ func (suite *Tests) Test_shutdownHTTPProxy() {
|
||||
httpProxyApp = original
|
||||
httpProxyMu.Unlock()
|
||||
}()
|
||||
suite.NoError(shutdownHTTPProxy(context.Background()))
|
||||
// Bound the context: if a clear-signal regression made the retry loop
|
||||
// spin forever, context.Background() would hang the whole package for
|
||||
// its default 10m test timeout instead of failing fast here.
|
||||
noopCtx, noopCancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer noopCancel()
|
||||
suite.NoError(shutdownHTTPProxy(noopCtx))
|
||||
|
||||
// With a listening server assigned, shutdown must stop it.
|
||||
app := fiber.New()
|
||||
@@ -625,33 +652,48 @@ func (suite *Tests) Test_shutdownHTTPProxy() {
|
||||
ln, err := net.Listen("tcp", "127.0.0.1:0")
|
||||
suite.Require().NoError(err)
|
||||
|
||||
// Signal off fiber's OnListen hook rather than dialling the raw listener:
|
||||
// the OS accepts connections into the listener's backlog as soon as it is
|
||||
// bound, before fiber's Serve loop is actually running, so a bare TCP
|
||||
// dial races server startup and makes shutdownHTTPProxy's no-op check
|
||||
// (fasthttp's listener isn't set yet) flaky under -race.
|
||||
serving := make(chan struct{})
|
||||
app.Hooks().OnListen(func(fiber.ListenData) error {
|
||||
close(serving)
|
||||
return nil
|
||||
})
|
||||
|
||||
listenDone := make(chan struct{})
|
||||
go func() {
|
||||
defer close(listenDone)
|
||||
_ = app.Listener(ln)
|
||||
// Mirror StartHTTPProxy's contract: clear httpProxyApp once Listener
|
||||
// returns, so shutdownHTTPProxy's retry loop (which waits for that
|
||||
// clear as proof the drain actually happened) can observe completion
|
||||
// instead of retrying until its context expires.
|
||||
httpProxyMu.Lock()
|
||||
if httpProxyApp == app {
|
||||
httpProxyApp = nil
|
||||
}
|
||||
httpProxyMu.Unlock()
|
||||
}()
|
||||
|
||||
// Wait until the listener is actually serving (poll the port) before
|
||||
// shutting down; otherwise Shutdown can race server startup and no-op.
|
||||
addr := ln.Addr().String()
|
||||
deadline := time.Now().Add(3 * time.Second)
|
||||
for {
|
||||
conn, err := net.Dial("tcp", addr)
|
||||
if err == nil {
|
||||
_ = conn.Close()
|
||||
break
|
||||
}
|
||||
if time.Now().After(deadline) {
|
||||
suite.Fail("proxy server never started listening on " + addr)
|
||||
return
|
||||
}
|
||||
time.Sleep(10 * time.Millisecond)
|
||||
select {
|
||||
case <-serving:
|
||||
case <-time.After(3 * time.Second):
|
||||
suite.Fail("proxy server never started serving on " + ln.Addr().String())
|
||||
return
|
||||
}
|
||||
|
||||
httpProxyMu.Lock()
|
||||
httpProxyApp = app
|
||||
httpProxyMu.Unlock()
|
||||
suite.NoError(shutdownHTTPProxy(context.Background()))
|
||||
// Bound the context: if a clear-signal regression made the retry loop
|
||||
// spin forever, context.Background() would hang the whole package for
|
||||
// its default 10m test timeout instead of failing fast here.
|
||||
shutdownCtx, shutdownCancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer shutdownCancel()
|
||||
suite.NoError(shutdownHTTPProxy(shutdownCtx))
|
||||
|
||||
select {
|
||||
case <-listenDone:
|
||||
|
||||
Reference in New Issue
Block a user