From 3aa5749d538d058c26fe79043612ff8191d1a247 Mon Sep 17 00:00:00 2001 From: zeripath Date: Fri, 19 Aug 2022 02:27:27 +0100 Subject: [PATCH] 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 * Update cmd/doctor.go * Update cmd/doctor.go Co-authored-by: delvh Signed-off-by: Andrew Thornton Co-authored-by: delvh Co-authored-by: techknowlogick --- cmd/doctor.go | 53 +++++++++++++++++++++++++++++-------- modules/log/multichannel.go | 6 ++--- 2 files changed, 45 insertions(+), 14 deletions(-) diff --git a/cmd/doctor.go b/cmd/doctor.go index 67a4ecc9c8e..a1184840522 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -5,6 +5,7 @@ package cmd import ( + "errors" "fmt" golog "log" "os" @@ -123,20 +124,11 @@ 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") @@ -144,11 +136,50 @@ func runDoctor(ctx *cli.Context) error { 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) diff --git a/modules/log/multichannel.go b/modules/log/multichannel.go index 273df81df15..519abf663d9 100644 --- a/modules/log/multichannel.go +++ b/modules/log/multichannel.go @@ -33,7 +33,7 @@ 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) @@ -41,9 +41,9 @@ func (l *MultiChannelledLogger) SetLogger(name, provider, config string) error { 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