imapclient: clean up function signature of New, allowing for future options too

ctl_test.go

@@ -9,6 +9,7 @@ import (
 	"crypto/x509"
 	"flag"
 	"fmt"
+	"log/slog"
 	"math/big"
 	"net"
 	"os"
@@ -524,7 +525,11 @@ func TestCtl(t *testing.T) {
 	testctl(func(xctl *ctl) {
 		a, b := net.Pipe()
 		go func() {
-			client, err := imapclient.New(mox.Cid(), a, true)
+			opts := imapclient.Opts{
+				Logger: slog.Default().With("cid", mox.Cid()),
+				Error:  func(err error) { panic(err) },
+			}
+			client, err := imapclient.New(a, &opts)
 			tcheck(t, err, "new imapclient")
 			client.Select("inbox")
 			client.Logout()

imapclient/client.go

@@ -45,7 +45,7 @@ type Conn struct {
 	xtw          *moxio.TraceWriter
 
 	log       mlog.Log
-	panic     bool
+	errHandle func(err error) // If set, called for all errors. Can panic. Used for imapserver tests.
 	tagGen    int
 	record    bool // If true, bytes read are added to recordBuf. recorded() resets.
 	recordBuf []byte
@@ -67,28 +67,46 @@ func (e Error) Unwrap() error {
 	return e.err
 }
 
+// Opts has optional fields that influence behaviour of a Conn.
+type Opts struct {
+	Logger *slog.Logger
+
+	// Error is called for both IMAP-level and connection-level errors. Is allowed to
+	// call panic.
+	Error func(err error)
+}
+
 // New creates a new client on conn.
 //
-// If xpanic is true, functions that would return an error instead panic. For parse
-// errors, the resulting stack traces show typically show what was being parsed.
-//
 // The initial untagged greeting response is read and must be "OK" or
 // "PREAUTH". If preauth, the connection is already in authenticated state,
 // typically through TLS client certificate. This is indicated in Conn.Preauth.
-func New(cid int64, conn net.Conn, xpanic bool) (client *Conn, rerr error) {
-	log := mlog.New("imapclient", nil).WithCid(cid)
+//
+// Logging is written to log, in particular IMAP protocol traces are written with
+// prefixes "CR: " and "CW: " (client read/write) as quoted strings at levels
+// Debug-4, with authentication messages at Debug-6 and (user) data at level
+// Debug-8.
+func New(conn net.Conn, opts *Opts) (client *Conn, rerr error) {
 	c := Conn{
 		conn:         conn,
-		log:          log,
-		panic:        xpanic,
 		CapAvailable: map[Capability]struct{}{},
 		CapEnabled:   map[Capability]struct{}{},
 	}
-	c.tr = moxio.NewTraceReader(log, "CR: ", &c)
+
+	var clog *slog.Logger
+	if opts != nil {
+		c.errHandle = opts.Error
+		clog = opts.Logger
+	} else {
+		clog = slog.Default()
+	}
+	c.log = mlog.New("imapclient", clog)
+
+	c.tr = moxio.NewTraceReader(c.log, "CR: ", &c)
 	c.br = bufio.NewReader(c.tr)
 
 	// Writes are buffered and write to Conn, which may panic.
-	c.xtw = moxio.NewTraceWriter(log, "CW: ", &c)
+	c.xtw = moxio.NewTraceWriter(c.log, "CW: ", &c)
 	c.xbw = bufio.NewWriter(c.xtw)
 
 	defer c.recover(&rerr)
@@ -116,10 +134,6 @@ func New(cid int64, conn net.Conn, xpanic bool) (client *Conn, rerr error) {
 }
 
 func (c *Conn) recover(rerr *error) {
-	if c.panic {
-		return
-	}
-
 	x := recover()
 	if x == nil {
 		return
@@ -128,6 +142,9 @@ func (c *Conn) recover(rerr *error) {
 	if !ok {
 		panic(x)
 	}
+	if c.errHandle != nil {
+		c.errHandle(err)
+	}
 	*rerr = err
 }
 
@@ -201,11 +218,6 @@ func (c *Conn) xtracewrite(level slog.Level) func() {
 	}
 }
 
-// SetPanic sets whether errors cause a panic instead of returning errors.
-func (c *Conn) SetPanic(panic bool) {
-	c.panic = panic
-}
-
 // Close closes the connection, flushing and closing any compression and TLS layer.
 //
 // You may want to call Logout first. Closing a connection with a mailbox with

imapserver/compress_test.go

@@ -40,11 +40,6 @@ func TestCompressStartTLS(t *testing.T) {
 	tc.transactf("ok", "append inbox (\\seen) {%d+}\r\n%s", len(exampleMsg), exampleMsg)
 	tc.transactf("ok", "noop")
 	tc.transactf("ok", "fetch 1 body.peek[1]")
-
-	// Prevent client.Close from failing the test. The client first closes the
-	// compression stream, which causes the server to close the connection, after which
-	// the messages to close the TLS connection are written to a closed socket.
-	tc.client.SetPanic(false)
 }
 
 func TestCompressBreak(t *testing.T) {

imapserver/fuzz_test.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log/slog"
 	"net"
 	"os"
 	"path/filepath"
@@ -126,7 +127,11 @@ func FuzzServer(f *testing.F) {
 
 				err := clientConn.SetDeadline(time.Now().Add(time.Second))
 				flog(err, "set client deadline")
-				client, _ := imapclient.New(mox.Cid(), clientConn, true)
+				opts := imapclient.Opts{
+					Logger: slog.Default().With("cid", mox.Cid()),
+					Error:  func(err error) { panic(err) },
+				}
+				client, _ := imapclient.New(clientConn, &opts)
 
 				for _, cmd := range cmds {
 					client.Commandf("", "%s", cmd)

imapserver/server_test.go

@@ -7,6 +7,7 @@ import (
 	"crypto/tls"
 	"crypto/x509"
 	"fmt"
+	"log/slog"
 	"math/big"
 	"net"
 	"os"
@@ -536,8 +537,11 @@ func startArgsMore(t *testing.T, uidonly, first, immediateTLS bool, serverConfig
 		serve("test", cid, serverConfig, serverConn, immediateTLS, allowLoginWithoutTLS, viaHTTPS, "")
 		close(done)
 	}()
-	client, err := imapclient.New(connCounter, clientConn, true)
-	tcheck(t, err, "new client")
+	opts := imapclient.Opts{
+		Logger: slog.Default().With("cid", connCounter),
+		Error:  func(err error) { panic(err) },
+	}
+	client, _ := imapclient.New(clientConn, &opts)
 	tc := &testconn{t: t, conn: clientConn, client: client, uidonly: uidonly, done: done, serverConn: serverConn, account: acc}
 	if first {
 		tc.switchStop = switchStop

integration_test.go

@@ -64,7 +64,10 @@ func TestDeliver(t *testing.T) {
 		tcheck(t, err, "dial imap")
 		defer imapconn.Close()
 
-		imapc, err := imapclient.New(mox.Cid(), imapconn, false)
+		opts := imapclient.Opts{
+			Logger: slog.Default().With("cid", mox.Cid()),
+		}
+		imapc, err := imapclient.New(imapconn, &opts)
 		tcheck(t, err, "new imapclient")
 
 		_, _, err = imapc.Login(imapuser, imappassword)