socket: refactor existing code

There are several //TODO comments in the socket package, these should be
fixed before expanding it.

Put send logic into it's own file and rename receiver code.

Fix the rather inelegant error handling when shutting down the server.
Make sure to close sockets.

Signed-off-by: Moritz Poldrack <git@moritz.sh>
Signed-off-by: Robin Jarry <robin@jarry.cc>

3 files changed, 147 insertions, 146 deletions

diff --git a/lib/ipc/receive.go b/lib/ipc/receive.go
new file mode 100644
index 00000000..c074b116
--- /dev/null
+++ b/lib/ipc/receive.go
@@ -0,0 +1,116 @@
+package ipc
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"net"
+	"net/url"
+	"os"
+	"path"
+	"strings"
+	"sync/atomic"
+	"time"
+
+	"git.sr.ht/~rjarry/aerc/log"
+	"github.com/kyoh86/xdg"
+)
+
+type AercServer struct {
+	listener net.Listener
+
+	OnMailto func(addr *url.URL) error
+	OnMbox   func(source string) error
+}
+
+func StartServer() (*AercServer, error) {
+	sockpath := path.Join(xdg.RuntimeDir(), "aerc.sock")
+	// remove the socket if it is not connected to a session
+	if err := ConnectAndExec(""); err != nil {
+		os.Remove(sockpath)
+	}
+	log.Debugf("Starting Unix server: %s", sockpath)
+	l, err := net.Listen("unix", sockpath)
+	if err != nil {
+		return nil, err
+	}
+	as := &AercServer{listener: l}
+	go as.Serve()
+
+	return as, nil
+}
+
+func (as *AercServer) Close() {
+	as.listener.Close()
+}
+
+var lastId int64 = 0 // access via atomic
+
+func (as *AercServer) Serve() {
+	defer log.PanicHandler()
+
+	for {
+		conn, err := as.listener.Accept()
+		switch {
+		case errors.Is(err, net.ErrClosed):
+			log.Infof("shutting down UNIX listener")
+			return
+		case err != nil:
+			log.Errorf("ipc: accepting connection failed: %v", err)
+			continue
+		}
+
+		defer conn.Close()
+		clientId := atomic.AddInt64(&lastId, 1)
+		log.Debugf("unix:%d accepted connection", clientId)
+		scanner := bufio.NewScanner(conn)
+		err = conn.SetDeadline(time.Now().Add(1 * time.Minute))
+		if err != nil {
+			log.Errorf("unix:%d failed to set deadline: %v", clientId, err)
+		}
+		for scanner.Scan() {
+			err = conn.SetDeadline(time.Now().Add(1 * time.Minute))
+			if err != nil {
+				log.Errorf("unix:%d failed to update deadline: %v", clientId, err)
+			}
+			msg := scanner.Text()
+			log.Tracef("unix:%d got message %s", clientId, msg)
+
+			_, err = conn.Write([]byte(as.handleMessage(msg)))
+			if err != nil {
+				log.Errorf("unix:%d failed to send response: %v", clientId, err)
+				break
+			}
+		}
+		log.Tracef("unix:%d closed connection", clientId)
+	}
+}
+
+func (as *AercServer) handleMessage(msg string) string {
+	if !strings.ContainsRune(msg, ':') {
+		return "error: invalid command\n"
+	}
+	prefix := msg[:strings.IndexRune(msg, ':')]
+	var err error
+	switch prefix {
+	case "mailto":
+		mailto, err := url.Parse(msg)
+		if err != nil {
+			return fmt.Sprintf("error: %v\n", err)
+		}
+		if as.OnMailto != nil {
+			err = as.OnMailto(mailto)
+			if err != nil {
+				return fmt.Sprintf("mailto failed: %v\n", err)
+			}
+		}
+	case "mbox":
+		if as.OnMbox != nil {
+			err = as.OnMbox(msg)
+			if err != nil {
+				return fmt.Sprintf("mbox failed: %v\n", err)
+			}
+		}
+	}
+	return "result: success\n"
+}
diff --git a/lib/ipc/send.go b/lib/ipc/send.go
new file mode 100644
index 00000000..5cc97cc0
--- /dev/null
+++ b/lib/ipc/send.go
@@ -0,0 +1,31 @@
+package ipc
+
+import (
+	"bufio"
+	"errors"
+	"fmt"
+	"net"
+	"path"
+
+	"github.com/kyoh86/xdg"
+)
+
+func ConnectAndExec(msg string) error {
+	sockpath := path.Join(xdg.RuntimeDir(), "aerc.sock")
+	conn, err := net.Dial("unix", sockpath)
+	if err != nil {
+		return err
+	}
+	defer conn.Close()
+	_, err = conn.Write([]byte(msg + "\n"))
+	if err != nil {
+		return fmt.Errorf("failed to send message: %w", err)
+	}
+	scanner := bufio.NewScanner(conn)
+	if !scanner.Scan() {
+		return errors.New("No response from server")
+	}
+	result := scanner.Text()
+	fmt.Println(result)
+	return nil
+}
diff --git a/lib/ipc/socket.go b/lib/ipc/socket.go
deleted file mode 100644
index 27692a9b..00000000
--- a/lib/ipc/socket.go
+++ /dev/null
@@ -1,146 +0,0 @@
-package ipc
-
-import (
-	"bufio"
-	"errors"
-	"fmt"
-	"net"
-	"net/url"
-	"os"
-	"path"
-	"strings"
-	"sync/atomic"
-	"time"
-
-	"git.sr.ht/~rjarry/aerc/log"
-	"github.com/kyoh86/xdg"
-)
-
-type AercServer struct {
-	listener net.Listener
-	OnMailto func(addr *url.URL) error
-	OnMbox   func(source string) error
-}
-
-func StartServer() (*AercServer, error) {
-	sockpath := path.Join(xdg.RuntimeDir(), "aerc.sock")
-	// remove the socket if it is not connected to a session
-	if err := ConnectAndExec(""); err != nil {
-		os.Remove(sockpath)
-	}
-	log.Debugf("Starting Unix server: %s", sockpath)
-	l, err := net.Listen("unix", sockpath)
-	if err != nil {
-		return nil, err
-	}
-	as := &AercServer{listener: l}
-	// TODO: stash clients and close them on exit... bleh racey
-	go func() {
-		defer log.PanicHandler()
-
-		for {
-			conn, err := l.Accept()
-			if err != nil {
-				if !strings.Contains(err.Error(),
-					"use of closed network connection") {
-					// TODO: Something more useful, in some
-					// cases, on wednesdays, after 2 PM,
-					// I guess?
-					log.Errorf("Closing Unix server: %v", err)
-				}
-				return
-			}
-			go func() {
-				defer log.PanicHandler()
-
-				as.handleClient(conn)
-			}()
-		}
-	}()
-	return as, nil
-}
-
-func (as *AercServer) Close() {
-	as.listener.Close()
-}
-
-var lastId int64 = 0 // access via atomic
-
-func (as *AercServer) handleClient(conn net.Conn) {
-	clientId := atomic.AddInt64(&lastId, 1)
-	log.Debugf("unix:%d accepted connection", clientId)
-	scanner := bufio.NewScanner(conn)
-	err := conn.SetDeadline(time.Now().Add(1 * time.Minute))
-	if err != nil {
-		log.Errorf("failed to set deadline: %v", err)
-	}
-	for scanner.Scan() {
-		err = conn.SetDeadline(time.Now().Add(1 * time.Minute))
-		if err != nil {
-			log.Errorf("failed to update deadline: %v", err)
-		}
-		msg := scanner.Text()
-		log.Tracef("unix:%d got message %s", clientId, msg)
-		if !strings.ContainsRune(msg, ':') {
-			_, innererr := conn.Write([]byte("error: invalid command\n"))
-			if innererr != nil {
-				log.Errorf("failed to write error message: %v", innererr)
-			}
-			continue
-		}
-		prefix := msg[:strings.IndexRune(msg, ':')]
-		var err error
-		switch prefix {
-		case "mailto":
-			mailto, err := url.Parse(msg)
-			if err != nil {
-				_, innererr := conn.Write([]byte(fmt.Sprintf("error: %v\n", err)))
-				if innererr != nil {
-					log.Errorf("failed to write error message: %v", innererr)
-				}
-				break
-			}
-			if as.OnMailto != nil {
-				err = as.OnMailto(mailto)
-				if err != nil {
-					log.Errorf("mailto failed: %v", err)
-				}
-			}
-		case "mbox":
-			if as.OnMbox != nil {
-				err = as.OnMbox(msg)
-			}
-		}
-		if err != nil {
-			_, err = conn.Write([]byte(fmt.Sprintf("result: %v\n", err)))
-			if err != nil {
-				log.Errorf("failed to send error: %v")
-			}
-		} else {
-			_, err = conn.Write([]byte("result: success\n"))
-			if err != nil {
-				log.Errorf("failed to send successmessage: %v")
-			}
-		}
-	}
-	log.Tracef("unix:%d closed connection", clientId)
-}
-
-func ConnectAndExec(msg string) error {
-	sockpath := path.Join(xdg.RuntimeDir(), "aerc.sock")
-	conn, err := net.Dial("unix", sockpath)
-	if err != nil {
-		return err
-	}
-	_, err = conn.Write([]byte(msg + "\n"))
-	if err != nil {
-		return fmt.Errorf("failed to send message: %w", err)
-	}
-	scanner := bufio.NewScanner(conn)
-	if !scanner.Scan() {
-		return errors.New("No response from server")
-	}
-	result := scanner.Text()
-	fmt.Println(result)
-	return nil
-}