test: fourth review -- a hang-up anywhere in the body, an abandoned request is not an implicit 200, a database error is logged as its SQLSTATE (red)

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

internal/web/uploads_disconnect_handler_test.go

@@ -1,6 +1,7 @@
 package web_test
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"log/slog"
@@ -20,48 +21,85 @@ import (
 // listener (third adversarial review: over a socket every such case logged an ERROR and
 // half-close returned a 500, because the error page was rendered on a cancelled context).
 
-func partialUpload(t *testing.T, addr string, hangUp func(*net.TCPConn)) {
+// body positions at which the client can hang up: inside the title field, inside the file
+// part's own headers, and mid-file. Every one must be handled alike (fourth adversarial
+// review: the title branch had no guard and still logged an ERROR).
+const (
+	inTitle   = "in the title"
+	inHeaders = "in the file part's headers"
+	midFile   = "mid-file"
+)
+
+func partialUpload(t *testing.T, addr, where string, hangUp func(*net.TCPConn)) {
 	t.Helper()
 	c, err := net.Dial("tcp", addr)
 	require.NoError(t, err)
 	tc := c.(*net.TCPConn)
 	const boundary = "xBOUNDARYx"
-	head := "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filename=\"a.m4a\"\r\nContent-Type: audio/mp4\r\n\r\n"
+	var body string
-	_, err = fmt.Fprintf(tc, "POST /uploads HTTP/1.1\r\nHost: x\r\nContent-Type: multipart/form-data; boundary=%s\r\nContent-Length: 200000\r\n\r\n%s%s",
+	switch where {
-		boundary, head, strings.Repeat("a", 4000))
+	case inTitle:
+		body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"title\"\r\n\r\nhalf a tit"
+	case inHeaders:
+		body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filena"
+	default:
+		body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filename=\"a.m4a\"\r\nContent-Type: audio/mp4\r\n\r\n" + strings.Repeat("a", 4000)
+	}
+	_, err = fmt.Fprintf(tc, "POST /uploads HTTP/1.1\r\nHost: x\r\nContent-Type: multipart/form-data; boundary=%s\r\nContent-Length: 200000\r\n\r\n%s", boundary, body)
 	require.NoError(t, err)
-	time.Sleep(50 * time.Millisecond) // let the server start reading the part
+	time.Sleep(50 * time.Millisecond) // let the server start reading
 	hangUp(tc)
 }
 
-func TestUploadPost_AClientThatHangsUpMidFileIsLoggedAtInfoAndNeverAsAnError(t *testing.T) {
+func TestUploadPost_AClientThatHangsUpAnywhereInTheBodyIsLoggedAtInfoAndNeverAsAnError(t *testing.T) {
 	hangUps := map[string]func(*net.TCPConn){
 		"a half close":  func(c *net.TCPConn) { _ = c.CloseWrite(); time.Sleep(100 * time.Millisecond); _ = c.Close() },
 		"a full close":  func(c *net.TCPConn) { _ = c.Close() },
 		"a reset (RST)": func(c *net.TCPConn) { _ = c.SetLinger(0); _ = c.Close() },
 	}
-	for name, hangUp := range hangUps {
+	for _, where := range []string{inTitle, inHeaders, midFile} {
-		t.Run(name, func(t *testing.T) {
+		for name, hangUp := range hangUps {
-			app, sub := newUploadApp(t, audioOK, 200<<20)
+			t.Run(where+", "+name, func(t *testing.T) {
-			buf := &lockedBuf{}
+				app, sub := newUploadApp(t, audioOK, 200<<20)
-			app.Log = slog.New(slog.NewTextHandler(buf, nil))
+				buf := &lockedBuf{}
-			srv := httptest.NewServer(app.Router())
+				app.Log = slog.New(slog.NewTextHandler(buf, nil))
-			defer srv.Close()
+				srv := httptest.NewServer(app.Router())
+				defer srv.Close()
 
-			partialUpload(t, strings.TrimPrefix(srv.URL, "http://"), hangUp)
+				partialUpload(t, strings.TrimPrefix(srv.URL, "http://"), where, hangUp)
 
-			require.Eventually(t, func() bool {
+				require.Eventually(t, func() bool {
+					out := buf.String()
+					return strings.Contains(out, "upload abandoned") || strings.Contains(out, "upload rejected") || strings.Contains(out, "level=ERROR")
+				}, 3*time.Second, 20*time.Millisecond, "the server never finished handling the aborted upload")
+				time.Sleep(100 * time.Millisecond)
 				out := buf.String()
-				return strings.Contains(out, "upload abandoned") || strings.Contains(out, "upload rejected") || strings.Contains(out, "level=ERROR")
+				assert.NotContains(t, out, "level=ERROR", "a client hanging up is not a server fault:\n%s", out)
-			}, 3*time.Second, 20*time.Millisecond, "the server never finished handling the aborted upload")
+				assert.Empty(t, sub.jobs)
-			time.Sleep(100 * time.Millisecond)
+			})
-			out := buf.String()
+		}
-			assert.NotContains(t, out, "level=ERROR", "a client hanging up is not a server fault:\n%s", out)
-			assert.Empty(t, sub.jobs)
-		})
 	}
 }
 
+func TestUploadPost_AnAbandonedRequestGetsA499NotAnImplicitEmpty200(t *testing.T) {
+	// If the context is cancelled but the connection can still be written to (a client that
+	// half-closed after sending), a missing status becomes 200 with an empty body: a false
+	// success (fourth adversarial review).
+	app, _ := newUploadApp(t, audioOK, 200<<20)
+	buf := &lockedBuf{}
+	app.Log = slog.New(slog.NewTextHandler(buf, nil))
+	app.Uploads = failingUploader{err: fmt.Errorf("receive: %w", io.ErrClosedPipe)}
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel()
+	req := uploadReq(t, "t", "a.m4a", []byte("x")).WithContext(ctx)
+
+	rec := do(t, app, req)
+
+	assert.Equal(t, 499, rec.Code, "never an implicit 200 for an upload that was not accepted")
+	assert.Contains(t, buf.String(), "upload abandoned")
+	assert.NotContains(t, buf.String(), "level=ERROR")
+}
+
 func TestUploadPost_ADatabaseDropThatLooksLikeAnEOFIsStillAServerError(t *testing.T) {
 	// pgx turns a connection that drops mid-message into io.ErrUnexpectedEOF. Mapping that
 	// sentinel to "your upload was cut off" (a 400) would hide a real fault: only an error