fix(internal/syncwriter): stop corrupting test output on sink write errors

syncwriter reported sink write/sync failures with the builtin println,
which writes raw, unsynchronized bytes to fd 2. The message and its
trailing newline are emitted as separate syscalls, so under concurrency a
test framework's "--- PASS:"/"--- FAIL:" line can interleave inside the
message. test2json then fails to record that test's terminal action, and
tools like gotestsum report an innocent passing test as unknown/failed.

- Write now suppresses benign "destination closed" errors
  (io.ErrClosedPipe, os.ErrClosed, syscall.EPIPE), mirroring the existing
  Sync suppression of EINVAL/ENOTTY/EBADF.
- The default error handler reports through os.Stderr as a single
  newline-terminated write instead of the builtin println.

Fixes #225.

Author: EhabY

internal/syncwriter/syncwriter.go

@@ -24,7 +24,9 @@ func New(w io.Writer) *Writer {
 		w: w,
 
 		errorf: func(f string, v ...interface{}) {
-			println(fmt.Sprintf(f, v...))
+			// Avoid the builtin println, which writes to fd 2
+			// unsynchronized and can corrupt concurrent go test output.
+			fmt.Fprintf(os.Stderr, f+"\n", v...)
 		},
 	}
 }
@@ -34,6 +36,12 @@ func (w *Writer) Write(name string, p []byte) {
 	defer w.mu.Unlock()
 	_, err := w.w.Write(p)
 	if err != nil {
+		// A closed destination is benign: the reader is gone, so the
+		// entry is undeliverable and not worth reporting.
+		// See https://github.com/coder/slog/issues/225
+		if errorsIsAny(err, io.ErrClosedPipe, syscall.EPIPE) {
+			return
+		}
 		w.errorf("%v: failed to write entry: %+v", name, err)
 	}
 }

internal/syncwriter/syncwriter_test.go

@@ -1,8 +1,11 @@
 package syncwriter
 
 import (
+	"fmt"
 	"io"
 	"os"
+	"strings"
+	"syscall"
 	"testing"
 
 	"cdr.dev/slog/v3/internal/assert"
@@ -74,6 +77,83 @@ func TestWriter_Sync(t *testing.T) {
 	})
 }
 
+// Benign "destination closed" errors must not be reported.
+func TestWriter_Write_suppressesClosedDestinationErrors(t *testing.T) {
+	t.Parallel()
+
+	benign := []error{
+		io.ErrClosedPipe,
+		syscall.EPIPE,
+		fmt.Errorf("wrapped: %w", io.ErrClosedPipe),
+	}
+
+	for _, target := range benign {
+		target := target
+		t.Run(fmt.Sprintf("%v", target), func(t *testing.T) {
+			t.Parallel()
+
+			errors := 0
+			w := New(syncWriter{
+				wf: func([]byte) (int, error) { return 0, target },
+				sf: func() error { return nil },
+			})
+			w.errorf = func(string, ...interface{}) { errors++ }
+
+			w.Write("sloghuman", []byte("entry"))
+			assert.Equal(t, "errors", 0, errors)
+		})
+	}
+}
+
+func TestWriter_Write_reportsOtherErrors(t *testing.T) {
+	t.Parallel()
+
+	// os.ErrClosed is not suppressed: it usually means a write to an
+	// already-closed writer, which is worth surfacing.
+	for _, target := range []error{io.EOF, os.ErrClosed} {
+		target := target
+		t.Run(fmt.Sprintf("%v", target), func(t *testing.T) {
+			t.Parallel()
+
+			errors := 0
+			w := New(syncWriter{
+				wf: func([]byte) (int, error) { return 0, target },
+				sf: func() error { return nil },
+			})
+			w.errorf = func(string, ...interface{}) { errors++ }
+
+			w.Write("sloghuman", []byte("entry"))
+			assert.Equal(t, "errors", 1, errors)
+		})
+	}
+}
+
+// The default handler must report through the os.Stderr variable, not the
+// builtin println that bypasses it. Reverting to println fails this test.
+//
+// Not parallel: it swaps the process-wide os.Stderr.
+func TestWriter_defaultErrorReportsThroughStderr(t *testing.T) {
+	r, w, err := os.Pipe()
+	assert.Success(t, "pipe", err)
+
+	orig := os.Stderr
+	os.Stderr = w
+	New(syncWriter{
+		wf: func([]byte) (int, error) { return 0, io.EOF },
+		sf: func() error { return nil },
+	}).Write("sinkname", []byte("entry"))
+	os.Stderr = orig
+	assert.Success(t, "close", w.Close())
+
+	out, err := io.ReadAll(r)
+	assert.Success(t, "read", err)
+
+	got := string(out)
+	assert.True(t, "reported via stderr", strings.Contains(got, "sinkname: failed to write entry"))
+	assert.True(t, "single trailing newline", strings.HasSuffix(got, "\n"))
+	assert.Equal(t, "newline count", 1, strings.Count(got, "\n"))
+}
+
 func Test_errorsIsAny(t *testing.T) {
 	t.Parallel()