[misc] Make stream logging less intrusive

Author: Christian Weichel

handler/handler.go

@@ -26,24 +26,23 @@ func Setup(cliPath string, _enableLogging bool) {
 	enableLogging = _enableLogging
 }
 
+// CLangdStarter starts clangd and returns its stdin/out/err
+type CLangdStarter func() (stdin io.WriteCloser, stdout io.ReadCloser, stderr io.ReadCloser)
+
 // NewInoHandler creates and configures an InoHandler.
-func NewInoHandler(stdin io.ReadCloser, stdout io.WriteCloser, stdinLog, stdoutLog io.Writer,
-	startClangd func() (io.WriteCloser, io.ReadCloser, io.ReadCloser),
-	clangdinLog, clangdoutLog, clangderrLog io.Writer, board Board) *InoHandler {
+func NewInoHandler(stdin io.ReadCloser, stdout io.WriteCloser, logStreams *StreamLogger, startClangd CLangdStarter, board Board) *InoHandler {
 	handler := &InoHandler{
 		clangdProc: ClangdProc{
-			Start:  startClangd,
-			inLog:  clangdinLog,
-			outLog: clangdoutLog,
-			errLog: clangderrLog,
+			Start: startClangd,
+			Logs:  logStreams,
 		},
 		data: make(map[lsp.DocumentURI]*FileData),
 		config: BoardConfig{
 			SelectedBoard: board,
 		},
 	}
-	handler.StartClangd()
-	stdStream := jsonrpc2.NewBufferedStream(StreamReadWrite{stdin, stdout, stdinLog, stdoutLog}, jsonrpc2.VSCodeObjectCodec{})
+	handler.startClangd()
+	stdStream := jsonrpc2.NewBufferedStream(logStreams.AttachStdInOut(stdin, stdout), jsonrpc2.VSCodeObjectCodec{})
 	stdHandler := jsonrpc2.AsyncHandler(jsonrpc2.HandlerWithError(handler.FromStdio))
 	handler.StdioConn = jsonrpc2.NewConn(context.Background(), stdStream, stdHandler)
 	if enableLogging {
@@ -63,9 +62,9 @@ type InoHandler struct {
 
 // ClangdProc contains the process input / output streams for clangd.
 type ClangdProc struct {
-	Start                 func() (io.WriteCloser, io.ReadCloser, io.ReadCloser)
-	inLog, outLog, errLog io.Writer
-	initParams            lsp.InitializeParams
+	Start      func() (io.WriteCloser, io.ReadCloser, io.ReadCloser)
+	Logs       *StreamLogger
+	initParams lsp.InitializeParams
 }
 
 // FileData gathers information on a .ino source file.
@@ -79,12 +78,12 @@ type FileData struct {
 }
 
 // StartClangd starts the clangd process and connects its input / output streams.
-func (handler *InoHandler) StartClangd() {
+func (handler *InoHandler) startClangd() {
 	clangdWrite, clangdRead, clangdErr := handler.clangdProc.Start()
 	if enableLogging {
-		go io.Copy(handler.clangdProc.errLog, clangdErr)
+		go io.Copy(handler.clangdProc.Logs.ClangdErr, clangdErr)
 	}
-	srw := StreamReadWrite{clangdRead, clangdWrite, handler.clangdProc.inLog, handler.clangdProc.outLog}
+	srw := handler.clangdProc.Logs.AttachClangdInOut(clangdRead, clangdWrite)
 	clangdStream := jsonrpc2.NewBufferedStream(srw, jsonrpc2.VSCodeObjectCodec{})
 	clangdHandler := jsonrpc2.AsyncHandler(jsonrpc2.HandlerWithError(handler.FromClangd))
 	handler.ClangdConn = jsonrpc2.NewConn(context.Background(), clangdStream, clangdHandler)
@@ -178,7 +177,7 @@ func (handler *InoHandler) changeBoardConfig(ctx context.Context, config *BoardC
 	}
 
 	// Restart the clangd process, initialize it and reopen the files
-	handler.StartClangd()
+	handler.startClangd()
 	initResult := new(lsp.InitializeResult)
 	err := handler.ClangdConn.Call(ctx, "initialize", &handler.clangdProc.initParams, initResult)
 	if err != nil {

handler/stream.go

@@ -0,0 +1,147 @@
+package handler
+
+import (
+	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+)
+
+// StreamLogger maintains log files for all streams involved in the language server
+type StreamLogger struct {
+	Default   io.WriteCloser
+	Stdin     io.WriteCloser
+	Stdout    io.WriteCloser
+	ClangdIn  io.WriteCloser
+	ClangdOut io.WriteCloser
+	ClangdErr io.WriteCloser
+}
+
+// Close closes all logging streams
+func (s *StreamLogger) Close() (err error) {
+	var errs []string
+	for _, c := range []io.Closer{s.Default, s.Stdin, s.Stdout, s.ClangdIn, s.ClangdOut, s.ClangdErr} {
+		if c == nil {
+			continue
+		}
+
+		err = c.Close()
+		if err != nil {
+			errs = append(errs, err.Error())
+		}
+	}
+	if len(errs) != 0 {
+		return fmt.Errorf(strings.Join(errs, ", "))
+	}
+
+	return nil
+}
+
+// AttachStdInOut attaches the stdin, stdout logger to the in/out channels
+func (s *StreamLogger) AttachStdInOut(in io.ReadCloser, out io.WriteCloser) io.ReadWriteCloser {
+	return &streamDuplex{
+		io.TeeReader(in, s.Stdin),
+		in,
+		io.MultiWriter(out, s.Stdout),
+		out,
+	}
+}
+
+// AttachClangdInOut attaches the clangd in, out logger to the in/out channels
+func (s *StreamLogger) AttachClangdInOut(in io.ReadCloser, out io.WriteCloser) io.ReadWriteCloser {
+	return &streamDuplex{
+		io.TeeReader(in, s.ClangdIn),
+		in,
+		io.MultiWriter(out, s.ClangdOut),
+		out,
+	}
+}
+
+type streamDuplex struct {
+	in   io.Reader
+	inc  io.Closer
+	out  io.Writer
+	outc io.Closer
+}
+
+func (sd *streamDuplex) Read(p []byte) (int, error) {
+	return sd.in.Read(p)
+}
+
+func (sd *streamDuplex) Write(p []byte) (int, error) {
+	return sd.out.Write(p)
+}
+
+func (sd *streamDuplex) Close() error {
+	ierr := sd.inc.Close()
+	oerr := sd.outc.Close()
+
+	if ierr != nil {
+		return ierr
+	}
+	if oerr != nil {
+		return oerr
+	}
+	return nil
+}
+
+// NewStreamLogger creates files for all stream logs. Returns an error if opening a single stream fails.
+func NewStreamLogger(basepath string) (res *StreamLogger, err error) {
+	res = &StreamLogger{}
+
+	res.Default, err = os.OpenFile(filepath.Join(basepath, "inols.log"), os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.Stdin, err = os.OpenFile(filepath.Join(basepath, "inols-stdin.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.Stdout, err = os.OpenFile(filepath.Join(basepath, "inols-stdout.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdIn, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-in.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdOut, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-out.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+	res.ClangdErr, err = os.OpenFile(filepath.Join(basepath, "inols-clangd-err.log"), os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
+	if err != nil {
+		res.Close()
+		return
+	}
+
+	return
+}
+
+// NewNoopLogger creates a logger that does nothing
+func NewNoopLogger() (res *StreamLogger) {
+	noop := noopCloser{ioutil.Discard}
+	return &StreamLogger{
+		Default:   noop,
+		Stdin:     noop,
+		Stdout:    noop,
+		ClangdIn:  noop,
+		ClangdOut: noop,
+		ClangdErr: noop,
+	}
+}
+
+type noopCloser struct {
+	io.Writer
+}
+
+func (noopCloser) Close() error {
+	return nil
+}

handler/streamlog.go

@@ -15,83 +15,49 @@ var cliPath string
 var initialFqbn string
 var initialBoardName string
 var enableLogging bool
+var loggingBasePath string
 
 func main() {
-	flag.StringVar(&clangdPath, "clangd", "clangd",
-		"Path to clangd executable")
-	flag.StringVar(&cliPath, "cli", "arduino-cli",
-		"Path to arduino-cli executable")
-	flag.StringVar(&initialFqbn, "fqbn", "arduino:avr:uno",
-		"Fully qualified board name to use initially (can be changed via JSON-RPC)")
-	flag.StringVar(&initialBoardName, "board-name", "",
-		"User-friendly board name to use initially (can be changed via JSON-RPC)")
-	flag.BoolVar(&enableLogging, "log", false,
-		"Enable logging to files")
+	flag.StringVar(&clangdPath, "clangd", "clangd", "Path to clangd executable")
+	flag.StringVar(&cliPath, "cli", "arduino-cli", "Path to arduino-cli executable")
+	flag.StringVar(&initialFqbn, "fqbn", "arduino:avr:uno", "Fully qualified board name to use initially (can be changed via JSON-RPC)")
+	flag.StringVar(&initialBoardName, "board-name", "", "User-friendly board name to use initially (can be changed via JSON-RPC)")
+	flag.BoolVar(&enableLogging, "log", false, "Enable logging to files")
+	flag.StringVar(&loggingBasePath, "logpath", ".", "Location where to write logging files to when logging is enabled")
 	flag.Parse()
 
-	var stdinLog, stdoutLog, clangdinLog, clangdoutLog, clangderrLog io.Writer
+	// var stdinLog, stdoutLog, clangdinLog, clangdoutLog, clangderrLog io.Writer
+	var logStreams *handler.StreamLogger
 	if enableLogging {
-		logFile, stdinLogFile, stdoutLogFile, clangdinLogFile, clangdoutLogFile, clangderrLogFile := createLogFiles()
-		defer logFile.Close()
-		defer stdinLogFile.Close()
-		defer stdoutLogFile.Close()
-		defer clangdinLogFile.Close()
-		defer clangdoutLogFile.Close()
-		defer clangderrLogFile.Close()
-		log.SetOutput(logFile)
-		stdinLog, stdoutLog, clangdinLog, clangdoutLog, clangderrLog = stdinLogFile, stdoutLogFile,
-			clangdinLogFile, clangdoutLogFile, clangderrLogFile
+		var err error
+		logStreams, err = handler.NewStreamLogger(loggingBasePath)
+		if err != nil {
+			log.Fatal(err)
+		}
+		defer logStreams.Close()
+
+		log.SetOutput(logStreams.Default)
 	} else {
+		logStreams = handler.NewNoopLogger()
 		log.SetOutput(os.Stderr)
 	}
 
 	handler.Setup(cliPath, enableLogging)
 	initialBoard := handler.Board{Fqbn: initialFqbn, Name: initialBoardName}
-	inoHandler := handler.NewInoHandler(os.Stdin, os.Stdout, stdinLog, stdoutLog, startClangd,
-		clangdinLog, clangdoutLog, clangderrLog, initialBoard)
+	inoHandler := handler.NewInoHandler(os.Stdin, os.Stdout, logStreams, startClangd, initialBoard)
 	defer inoHandler.StopClangd()
 	<-inoHandler.StdioConn.DisconnectNotify()
 }
 
-func createLogFiles() (logFile, stdinLog, stdoutLog, clangdinLog, clangdoutLog, clangderrLog *os.File) {
-	var err error
-	logFile, err = os.OpenFile("inols.log", os.O_WRONLY|os.O_CREATE|os.O_APPEND, 0666)
-	if err != nil {
-		panic(err)
-	}
-	stdinLog, err = os.OpenFile("inols-stdin.log", os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		panic(err)
-	}
-	stdoutLog, err = os.OpenFile("inols-stdout.log", os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		panic(err)
-	}
-	clangdinLog, err = os.OpenFile("inols-clangd-in.log", os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		panic(err)
-	}
-	clangdoutLog, err = os.OpenFile("inols-clangd-out.log", os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		panic(err)
-	}
-	clangderrLog, err = os.OpenFile("inols-clangd-err.log", os.O_WRONLY|os.O_CREATE, 0666)
-	if err != nil {
-		panic(err)
-	}
-	return
-}
-
 func startClangd() (clangdIn io.WriteCloser, clangdOut io.ReadCloser, clangdErr io.ReadCloser) {
 	if enableLogging {
 		log.Println("Starting clangd process:", clangdPath)
 	}
 	clangdCmd := exec.Command(clangdPath)
 	clangdIn, _ = clangdCmd.StdinPipe()
 	clangdOut, _ = clangdCmd.StdoutPipe()
-	if enableLogging {
-		clangdErr, _ = clangdCmd.StderrPipe()
-	}
+	clangdErr, _ = clangdCmd.StderrPipe()
+
 	err := clangdCmd.Start()
 	if err != nil {
 		panic(err)