Disable doctor logging on panic (#20847)

* Disable doctor logging on panic

If permissions are incorrect for writing to the doctor log simply disable the log file
instead of panicing.

Related #20570

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Update cmd/doctor.go

* Update cmd/doctor.go

Co-authored-by: delvh <dev.lh@web.de>

Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: delvh <dev.lh@web.de>
Co-authored-by: techknowlogick <techknowlogick@gitea.io>

Author: 3 people

cmd/doctor.go

@@ -5,6 +5,7 @@
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	golog "log"
 	"os"
@@ -123,32 +124,62 @@ func runRecreateTable(ctx *cli.Context) error {
 	})
 }
 
-func runDoctor(ctx *cli.Context) error {
-	stdCtx, cancel := installSignals()
-	defer cancel()
-
-	// Silence the default loggers
-	log.DelNamedLogger("console")
-	log.DelNamedLogger(log.DEFAULT)
-
-	// Now setup our own
+func setDoctorLogger(ctx *cli.Context) {
 	logFile := ctx.String("log-file")
 	if !ctx.IsSet("log-file") {
 		logFile = "doctor.log"
 	}
-
 	colorize := log.CanColorStdout
 	if ctx.IsSet("color") {
 		colorize = ctx.Bool("color")
 	}
 
 	if len(logFile) == 0 {
 		log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"NONE","stacktracelevel":"NONE","colorize":%t}`, colorize))
-	} else if logFile == "-" {
+		return
+	}
+
+	defer func() {
+		recovered := recover()
+		if recovered == nil {
+			return
+		}
+
+		err, ok := recovered.(error)
+		if !ok {
+			panic(recovered)
+		}
+		if errors.Is(err, os.ErrPermission) {
+			fmt.Fprintf(os.Stderr, "ERROR: Unable to write logs to provided file due to permissions error: %s\n       %v\n", logFile, err)
+		} else {
+			fmt.Fprintf(os.Stderr, "ERROR: Unable to write logs to provided file: %s\n       %v\n", logFile, err)
+		}
+		fmt.Fprintf(os.Stderr, "WARN: Logging will be disabled\n       Use `--log-file` to configure log file location\n")
+		log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"NONE","stacktracelevel":"NONE","colorize":%t}`, colorize))
+	}()
+
+	if logFile == "-" {
 		log.NewLogger(1000, "doctor", "console", fmt.Sprintf(`{"level":"trace","stacktracelevel":"NONE","colorize":%t}`, colorize))
 	} else {
 		log.NewLogger(1000, "doctor", "file", fmt.Sprintf(`{"filename":%q,"level":"trace","stacktracelevel":"NONE"}`, logFile))
 	}
+}
+
+func runDoctor(ctx *cli.Context) error {
+	stdCtx, cancel := installSignals()
+	defer cancel()
+
+	// Silence the default loggers
+	log.DelNamedLogger("console")
+	log.DelNamedLogger(log.DEFAULT)
+
+	// Now setup our own
+	setDoctorLogger(ctx)
+
+	colorize := log.CanColorStdout
+	if ctx.IsSet("color") {
+		colorize = ctx.Bool("color")
+	}
 
 	// Finally redirect the default golog to here
 	golog.SetFlags(0)

modules/log/multichannel.go

@@ -33,17 +33,17 @@ func newLogger(name string, buffer int64) *MultiChannelledLogger {
 func (l *MultiChannelledLogger) SetLogger(name, provider, config string) error {
 	eventLogger, err := NewChannelledLog(l.ctx, name, provider, config, l.bufferLength)
 	if err != nil {
-		return fmt.Errorf("Failed to create sublogger (%s): %v", name, err)
+		return fmt.Errorf("failed to create sublogger (%s): %w", name, err)
 	}
 
 	l.MultiChannelledLog.DelLogger(name)
 
 	err = l.MultiChannelledLog.AddLogger(eventLogger)
 	if err != nil {
 		if IsErrDuplicateName(err) {
-			return fmt.Errorf("Duplicate named sublogger %s %v", name, l.MultiChannelledLog.GetEventLoggerNames())
+			return fmt.Errorf("%w other names: %v", err, l.MultiChannelledLog.GetEventLoggerNames())
 		}
-		return fmt.Errorf("Failed to add sublogger (%s): %v", name, err)
+		return fmt.Errorf("failed to add sublogger (%s): %w", name, err)
 	}
 
 	return nil