From a8c8de8b30e6ea4765241fb688f81450340fd1c5 Mon Sep 17 00:00:00 2001 From: Jace Date: Sat, 1 Aug 2026 22:12:53 -0700 Subject: [PATCH] fix(relay): deflake TestSMTPInboundMetric_RejectedLineTooLong MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test writes a single 160KiB line to exercise the 128KiB MaxLineLength cap. The server aborts the connection partway through the transfer, so whether the client's Write finishes before the RST arrives depends on socket buffering and scheduling — under CI contention (make cover runs packages at -p 4) the Write intermittently observes 'connection reset by peer' and the old t.Fatalf fired before the real assertion ever ran (~1 in 4 on recent main; e.g. fddf0edf). A mid-write reset is itself evidence the server aborted, so treat it as such instead of fatal: fail only if both Write and Close succeed (the genuine 'server accepted a 160KiB line' condition), assert the 554 too-long-line text only when the write completed cleanly, and rely on the rejected_line_too_long metric — which is the actual behavior under test and independent of wire timing — on every path. Co-Authored-By: Claude Opus 5 --- internal/relay/metrics_test.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/internal/relay/metrics_test.go b/internal/relay/metrics_test.go index 3671f31a..565935c8 100644 --- a/internal/relay/metrics_test.go +++ b/internal/relay/metrics_test.go @@ -463,19 +463,24 @@ func TestSMTPInboundMetric_RejectedLineTooLong(t *testing.T) { if err != nil { t.Fatalf("DATA: %v", err) } - // A single 160KiB line — over the 128KiB cap, small enough that the - // unread remainder after the server aborts fits in loopback socket - // buffers (the server reads ~128KiB before erroring). + // A single 160KiB line — over the 128KiB cap. The server aborts the + // connection mid-transfer, so depending on socket buffering the client + // may see the reset during Write, or only at Close, or (if the reset + // races the last flush) not as the 554 text at all. None of that is + // what's under test: a mid-write reset is itself evidence the server + // aborted, and the authoritative assertion is the + // rejected_line_too_long metric below, which runs on every path. body := "From: sender@ext.test\r\nTo: " + agentEmail + "\r\nSubject: too long\r\n\r\n" + strings.Repeat("y", 160*1024) - if _, err := w.Write([]byte(body)); err != nil { - t.Fatalf("write body: %v", err) - } + _, werr := w.Write([]byte(body)) cerr := w.Close() - if cerr == nil { + if werr == nil && cerr == nil { t.Fatal("DATA with a 160KiB line succeeded; want 554 too-long-line rejection") } - if !strings.Contains(cerr.Error(), "554") || !strings.Contains(cerr.Error(), "too long a line") { + // Only when the write went through cleanly is Close guaranteed to carry + // the SMTP-level rejection; after a mid-write reset it may return a bare + // connection error instead. + if werr == nil && (!strings.Contains(cerr.Error(), "554") || !strings.Contains(cerr.Error(), "too long a line")) { t.Fatalf("DATA close error = %v, want 554 too-long-line", cerr) }