diff --git a/logger.go b/logger.go index e04de7cb3..d87f1f0ee 100644 --- a/logger.go +++ b/logger.go @@ -288,9 +288,18 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry { ce = ce.Should(ent, zapcore.WriteThenPanic) case zapcore.FatalLevel: onFatal := log.onFatal - // nil is the default value for CheckWriteAction, and it leads to - // continued execution after a Fatal which is unexpected. - if onFatal == nil { + // nil or WriteThenNoop will lead to continued execution after + // a Fatal log entry, which is unexpected. For example, + // + // f, err := os.Open(..) + // if err != nil { + // log.Fatal("cannot open", zap.Error(err)) + // } + // fmt.Println(f.Name()) + // + // The f.Name() will panic if we continue execution after the + // log.Fatal. + if onFatal == nil || onFatal == zapcore.WriteThenNoop { onFatal = zapcore.WriteThenFatal } ce = ce.After(ent, onFatal) diff --git a/logger_test.go b/logger_test.go index 50f8c6599..c71306c7d 100644 --- a/logger_test.go +++ b/logger_test.go @@ -535,6 +535,17 @@ func TestLoggerConcurrent(t *testing.T) { }) } +func TestLoggerFatalOnNoop(t *testing.T) { + exitStub := exit.Stub() + defer exitStub.Unstub() + core, _ := observer.New(InfoLevel) + + // We don't allow a no-op fatal hook. + New(core, WithFatalHook(zapcore.WriteThenNoop)).Fatal("great sadness") + assert.True(t, exitStub.Exited, "must exit for WriteThenNoop") + assert.Equal(t, 1, exitStub.Code, "must exit with status 1 for WriteThenNoop") +} + func TestLoggerCustomOnFatal(t *testing.T) { tests := []struct { msg string diff --git a/options.go b/options.go index d6c3449ca..e2028df70 100644 --- a/options.go +++ b/options.go @@ -139,6 +139,18 @@ func OnFatal(action zapcore.CheckWriteAction) Option { } // WithFatalHook sets a CheckWriteHook to run on fatal logs. +// Zap will call this hook after writing a log statement with a Fatal level. +// +// For example, the following builds a logger that will exit the current +// goroutine after writing a fatal log message, but it will not exit the +// program. +// +// zap.New(core, zap.WithFatalHook(zapcore.WriteThenGoexit)) +// +// It is important that the provided CheckWriteHook stops the control flow at +// the current statement to meet expectations of callers of the logger. +// We recommend calling os.Exit or runtime.Goexit inside custom hooks at +// minimum. func WithFatalHook(hook zapcore.CheckWriteHook) Option { return optionFunc(func(log *Logger) { log.onFatal = hook diff --git a/zapcore/entry.go b/zapcore/entry.go index b035915f1..d91efe831 100644 --- a/zapcore/entry.go +++ b/zapcore/entry.go @@ -151,10 +151,24 @@ type Entry struct { Stack string } -// CheckWriteHook allows to customize the action to take after a Fatal log entry -// is processed. +// CheckWriteHook is a custom action that may be executed after an entry is +// written. +// +// Register one on a CheckedEntry with the After method. +// +// if ce := logger.Check(...); ce != nil { +// ce = ce.After(hook) +// ce.Write(...) +// } +// +// You can configure the hook for Fatal log statements at the logger level with +// the zap.WithFatalHook option. type CheckWriteHook interface { - // OnWrite gets invoked when an entry is written + // OnWrite is invoked with the CheckedEntry that was written and a list + // of fields added with that entry. + // + // The list of fields DOES NOT include fields that were already added + // to the logger with the With method. OnWrite(*CheckedEntry, []Field) }