From 8c3df8a6b2498551c85ce7f564625ae5b98fe73b Mon Sep 17 00:00:00 2001 From: Aleksei Zhukov Date: Thu, 10 Mar 2022 14:08:24 -0800 Subject: [PATCH] Improved error handling in Server.Serve Now Serve does not exit on non-fatal errors from Accept. This resolves https://github.com/emersion/go-smtp/issues/180 It also logs handleConn errors to the ErrorLog - which is a partial improvement for https://github.com/emersion/go-smtp/issues/167 --- server.go | 25 +++++++++++++-- server_test.go | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 105 insertions(+), 3 deletions(-) diff --git a/server.go b/server.go index d61020c..ee9effd 100644 --- a/server.go +++ b/server.go @@ -109,6 +109,8 @@ func (s *Server) Serve(l net.Listener) error { s.listeners = append(s.listeners, l) s.locker.Unlock() + var tempDelay time.Duration // how long to sleep on accept failure + for { c, err := l.Accept() if err != nil { @@ -117,11 +119,28 @@ func (s *Server) Serve(l net.Listener) error { // we called Close() return nil default: - return err } + if ne, ok := err.(net.Error); ok && ne.Temporary() { + if tempDelay == 0 { + tempDelay = 5 * time.Millisecond + } else { + tempDelay *= 2 + } + if max := 1 * time.Second; tempDelay > max { + tempDelay = max + } + s.ErrorLog.Printf("accept error: %s; retrying in %s", err, tempDelay) + time.Sleep(tempDelay) + continue + } + return err } - - go s.handleConn(newConn(c, s)) + go func() { + err := s.handleConn(newConn(c, s)) + if err != nil { + s.ErrorLog.Printf("handler error: %s", err) + } + }() } } diff --git a/server_test.go b/server_test.go index 48cdb12..1b2f47a 100644 --- a/server_test.go +++ b/server_test.go @@ -2,6 +2,7 @@ package smtp_test import ( "bufio" + "bytes" "errors" "io" "io/ioutil" @@ -146,6 +147,57 @@ func (s *session) LMTPData(r io.Reader, collector smtp.StatusCollector) error { return nil } +type failingListener struct { + c chan error + closed bool +} + +func newFailingListener() *failingListener { + return &failingListener{c: make(chan error)} +} + +func (l *failingListener) Send(err error) { + if !l.closed { + l.c <- err + } +} + +func (l *failingListener) Accept() (net.Conn, error) { + return nil, <-l.c +} + +func (l *failingListener) Close() error { + if !l.closed { + close(l.c) + l.closed = true + } + return nil +} + +func (l *failingListener) Addr() net.Addr { + return &net.TCPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 12345, + } +} + +type mockError struct { + msg string + temporary bool +} + +func newMockError(msg string, temporary bool) *mockError { + return &mockError{ + msg: msg, + temporary: temporary, + } +} + +func (m *mockError) Error() string { return m.msg } +func (m *mockError) String() string { return m.msg } +func (m *mockError) Timeout() bool { return false } +func (m *mockError) Temporary() bool { return m.temporary } + type serverConfigureFunc func(*smtp.Server) var ( @@ -226,6 +278,37 @@ func testServerEhlo(t *testing.T, fn ...serverConfigureFunc) (be *backend, s *sm return } +func TestServerAcceptErrorHandling(t *testing.T) { + errorLog := bytes.NewBuffer(nil) + be := new(backend) + s := smtp.NewServer(be) + s.Domain = "localhost" + s.AllowInsecureAuth = true + s.ErrorLog = log.New(errorLog, "", 0) + + l := newFailingListener() + var serveError error + go func() { + serveError = s.Serve(l) + l.Close() + }() + + temporaryError := newMockError("temporary mock error", true) + l.Send(temporaryError) + permanentError := newMockError("permanent mock error", false) + l.Send(permanentError) + s.Close() + + if serveError == nil { + t.Fatal("Serve had exited without an expected error") + } else if serveError != permanentError { + t.Fatal("Unexpected error:", serveError) + } + if !strings.Contains(errorLog.String(), temporaryError.String()) { + t.Fatal("Missing temporary error in log output:", errorLog.String()) + } +} + func TestServer_helo(t *testing.T) { _, s, c, scanner := testServerGreeted(t) defer s.Close()