fix(task): stop reporting abandoned requests as server errors (#2384)

apps/backend/internal/common/httpmw/logging.go

@@ -8,6 +8,10 @@ import (
 	"go.uber.org/zap"
 )
 
+// statusClientClosedRequest is nginx's 499, returned by handlers when the
+// caller abandoned the request. Go defines no constant for it.
+const statusClientClosedRequest = 499
+
 // RequestLogger logs HTTP request details after the handler completes.
 func RequestLogger(log *logger.Logger, serverName string) gin.HandlerFunc {
 	return func(c *gin.Context) {
@@ -35,7 +39,9 @@ func RequestLogger(log *logger.Logger, serverName string) gin.HandlerFunc {
 			zap.Int("bytes", size),
 		}
 		switch {
-		case status >= 400 && status != 404:
+		// 404 and 499 are routine: one is a miss, the other is the caller
+		// hanging up mid-request. Neither means something went wrong here.
+		case status >= 400 && status != 404 && status != statusClientClosedRequest:
 			log.Warn("http", fields...)
 		default:
 			log.Debug("http", fields...)

apps/backend/internal/task/handlers/errors.go

@@ -1,6 +1,7 @@
 package handlers
 
 import (
+	"context"
 	"errors"
 	"net/http"
 	"strings"
@@ -12,7 +13,15 @@ import (
 	"go.uber.org/zap"
 )
 
+// statusClientClosedRequest is nginx's 499. Go defines no constant for it and
+// no standard 4xx says "the client hung up", which is what actually happened.
+const statusClientClosedRequest = 499
+
 func handleNotFound(c *gin.Context, log *logger.Logger, err error, fallback string) {
+	if isClientDisconnect(err) {
+		abortClientDisconnect(c)
+		return
+	}
 	if isNotFound(err) {
 		c.JSON(http.StatusNotFound, gin.H{"error": fallback})
 		return
@@ -31,6 +40,8 @@ func handleNotFound(c *gin.Context, log *logger.Logger, err error, fallback stri
 
 func handleSelectedMoveError(c *gin.Context, log *logger.Logger, err error) {
 	switch {
+	case isClientDisconnect(err):
+		abortClientDisconnect(c)
 	case isNotFound(err):
 		c.JSON(http.StatusNotFound, gin.H{"error": "task or workflow not found"})
 	case isMoveConflict(err):
@@ -43,6 +54,22 @@ func handleSelectedMoveError(c *gin.Context, log *logger.Logger, err error) {
 	}
 }
 
+// isClientDisconnect reports a request the caller abandoned. Handlers derive
+// their context from c.Request.Context(), which the server cancels when the
+// peer goes away, so context.Canceled here means the browser navigated,
+// unmounted a component, or aborted an in-flight fetch. Nothing failed
+// server-side and nobody is left to read a response.
+//
+// DeadlineExceeded is deliberately not included: that is our own timeout
+// firing, and it stays a logged 500.
+func isClientDisconnect(err error) bool {
+	return err != nil && errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded)
+}
+
+func abortClientDisconnect(c *gin.Context) {
+	c.AbortWithStatus(statusClientClosedRequest)
+}
+
 func isNotFound(err error) bool {
 	if err == nil {
 		return false

apps/backend/internal/task/handlers/errors_test.go

@@ -47,6 +47,43 @@ func TestErrorsAreClassifiable(t *testing.T) {
 		}
 	})
 
+	// A browser that navigates away or unmounts a component aborts its
+	// in-flight fetches. That reaches handlers as a cancelled request context,
+	// which used to fall through to the 500 branch and log an ERROR with a
+	// full stacktrace for a response nobody was waiting on any more.
+	t.Run("client disconnect is not a server error", func(t *testing.T) {
+		gin.SetMode(gin.TestMode)
+		handlers := map[string]func(*gin.Context, error){
+			"handleNotFound": func(ctx *gin.Context, err error) {
+				handleNotFound(ctx, newTestLogger(t), err, "task sessions not found")
+			},
+			"handleSelectedMoveError": func(ctx *gin.Context, err error) {
+				handleSelectedMoveError(ctx, newTestLogger(t), err)
+			},
+		}
+		for name, handle := range handlers {
+			t.Run(name, func(t *testing.T) {
+				rec := httptest.NewRecorder()
+				ctx, _ := gin.CreateTestContext(rec)
+				handle(ctx, fmt.Errorf("list task sessions: %w", context.Canceled))
+				if rec.Code != statusClientClosedRequest {
+					t.Fatalf("status=%d, want %d", rec.Code, statusClientClosedRequest)
+				}
+			})
+		}
+	})
+
+	// Our own deadline firing is a real failure and must keep its 500.
+	t.Run("server timeout stays a server error", func(t *testing.T) {
+		gin.SetMode(gin.TestMode)
+		rec := httptest.NewRecorder()
+		ctx, _ := gin.CreateTestContext(rec)
+		handleNotFound(ctx, newTestLogger(t), fmt.Errorf("query: %w", context.DeadlineExceeded), "task not found")
+		if rec.Code != http.StatusInternalServerError {
+			t.Fatalf("status=%d, want %d", rec.Code, http.StatusInternalServerError)
+		}
+	})
+
 	t.Run("WIP limit is a conflict for task creation", func(t *testing.T) {
 		gin.SetMode(gin.TestMode)
 		rec := httptest.NewRecorder()