Misc fixes

logger.go

@@ -288,12 +288,12 @@ func (log *Logger) check(lvl zapcore.Level, msg string) *zapcore.CheckedEntry {
 		ce = ce.Should(ent, zapcore.WriteThenPanic)
 	case zapcore.FatalLevel:
 		onFatal := log.onFatal
-		// The nil is the default value for CheckWriteAction, and it leads to
+		// nil is the default value for CheckWriteAction, and it leads to
 		// continued execution after a Fatal which is unexpected.
 		if onFatal == nil {
 			onFatal = zapcore.WriteThenFatal
 		}
-		ce = ce.Should(ent, onFatal)
+		ce = ce.After(ent, onFatal)
 	case zapcore.DPanicLevel:
 		if log.development {
 			ce = ce.Should(ent, zapcore.WriteThenPanic)

logger_test.go

@@ -213,23 +213,15 @@ func TestLoggerAlwaysFatals(t *testing.T) {
 	// Users can disable writing out fatal-level logs, but calls to logger.Fatal()
 	// should still terminate the process.
 	withLogger(t, FatalLevel+1, nil, func(logger *Logger, logs *observer.ObservedLogs) {
-		stub := exit.WithStub(func() { logger.Fatal("foo") })
+		stub := exit.WithStub(func() { logger.Fatal("") })
 		assert.True(t, stub.Exited, "Expected calls to logger.Fatal to terminate process.")
-		assert.Equal(t, 1, stub.Code, "Expected calls to logger.Fatal to terminate process with predictable retcode.")
-
-		logger = logger.WithOptions(OnFatal(zapcore.WriteThenPosixExit))
-		err := errors.New("bar")
-		stub = exit.WithStub(func() { logger.Fatal("foo", Error(err)) })
-		assert.True(t, stub.Exited, "Expected calls to logger.Fatal to terminate process.")
-		assert.Equal(t, 129, stub.Code, "Expected calls to logger.Fatal to terminate process with predictable retcode.")
 
 		stub = exit.WithStub(func() {
 			if ce := logger.Check(FatalLevel, ""); ce != nil {
 				ce.Write()
 			}
 		})
 		assert.True(t, stub.Exited, "Expected calls to logger.Check(FatalLevel, ...) to terminate process.")
-		assert.Equal(t, 1, stub.Code, "Expected calls to logger.Check(FatalLevel, ...) to terminate process with predictable retcode.")
 
 		assert.Equal(t, 0, logs.Len(), "Shouldn't write out logs when fatal-level logging is disabled.")
 	})
@@ -587,6 +579,29 @@ func TestLoggerCustomOnFatal(t *testing.T) {
 	}
 }
 
+type customWriteHook struct {
+	called bool
+}
+
+func (h *customWriteHook) OnWrite(_ *zapcore.CheckedEntry, _ []Field) {
+	h.called = true
+}
+
+func TestLoggerWithFatalHook(t *testing.T) {
+	h := &customWriteHook{}
+	withLogger(t, InfoLevel, opts(WithFatalHook(h)), func(logger *Logger, logs *observer.ObservedLogs) {
+		recovered := make(chan interface{})
+		go func() {
+			defer func() {
+				recovered <- recover()
+			}()
+			logger.Fatal("")
+		}()
+		<-recovered
+		assert.True(t, h.called)
+	})
+}
+
 func TestNopLogger(t *testing.T) {
 	logger := NewNop()
 

options.go

@@ -133,12 +133,13 @@ func IncreaseLevel(lvl zapcore.LevelEnabler) Option {
 }
 
 // OnFatal sets the action to take on fatal logs.
+// Deprecated: Use WithFatalHook instead.
 func OnFatal(action zapcore.CheckWriteAction) Option {
-	return OnFatalHook(action)
+	return WithFatalHook(action)
 }
 
-// OnFatalHook allows to set a custom action to take on fatal logs.
-func OnFatalHook(hook zapcore.CheckWriteHook) Option {
+// WithFatalHook sets a CheckWriteHook to run on fatal logs.
+func WithFatalHook(hook zapcore.CheckWriteHook) Option {
 	return optionFunc(func(log *Logger) {
 		log.onFatal = hook
 	})

zapcore/entry.go

@@ -22,7 +22,6 @@ package zapcore
 
 import (
 	"fmt"
-	"hash/crc32"
 	"runtime"
 	"strings"
 	"sync"
@@ -155,6 +154,7 @@ type Entry struct {
 // CheckWriteHook allows to customize the action to take after a Fatal log entry
 // is processed.
 type CheckWriteHook interface {
+	// OnWrite gets invoked when an entry is written
 	OnWrite(*CheckedEntry, []Field)
 }
 
@@ -172,25 +172,26 @@ const (
 	WriteThenPanic
 	// WriteThenFatal causes an os.Exit(1) after Write.
 	WriteThenFatal
-	// WriteThenPosixExit causes an os.Exit(code) after Write. The exit code is
-	// calculated deterministically from the Fatal message, or from error Field
-	// if given.
-	WriteThenPosixExit
 )
 
-func (a CheckWriteAction) OnWrite(ce *CheckedEntry, fields []Field) {
+// OnWrite implements the OnWrite method to keep CheckWriteAction compatible
+// with the new CheckWriteHook interface which deprecates CheckWriteAction.
+func (a CheckWriteAction) OnWrite(ce *CheckedEntry, _ []Field) {
 	switch a {
 	case WriteThenGoexit:
 		runtime.Goexit()
 	case WriteThenPanic:
 		panic(ce.Message)
 	case WriteThenFatal:
-		exit.With(1)
-	case WriteThenPosixExit:
-		exit.With(retcode(ce, fields))
+		exit.Exit()
 	}
 }
 
+var _ CheckWriteHook = WriteThenNoop
+var _ CheckWriteHook = WriteThenGoexit
+var _ CheckWriteHook = WriteThenPanic
+var _ CheckWriteHook = WriteThenFatal
+
 // CheckedEntry is an Entry together with a collection of Cores that have
 // already agreed to log it.
 //
@@ -201,17 +202,15 @@ type CheckedEntry struct {
 	Entry
 	ErrorOutput WriteSyncer
 	dirty       bool // best-effort detection of pool misuse
-	should      CheckWriteHook
+	after       CheckWriteHook
 	cores       []Core
 }
 
 func (ce *CheckedEntry) reset() {
 	ce.Entry = Entry{}
 	ce.ErrorOutput = nil
 	ce.dirty = false
-	if ce.should != nil {
-		ce.should = WriteThenNoop
-	}
+	ce.after = nil
 	for i := range ce.cores {
 		// don't keep references to cores
 		ce.cores[i] = nil
@@ -249,12 +248,11 @@ func (ce *CheckedEntry) Write(fields ...Field) {
 		ce.ErrorOutput.Sync()
 	}
 
-	putCheckedEntry(ce)
-
-	// Terminal operation
-	if ce.should != nil {
-		ce.should.OnWrite(ce, fields)
+	hook := ce.after
+	if hook != nil {
+		hook.OnWrite(ce, fields)
 	}
+	putCheckedEntry(ce)
 }
 
 // AddCore adds a Core that has agreed to log this CheckedEntry. It's intended to be
@@ -269,25 +267,22 @@ func (ce *CheckedEntry) AddCore(ent Entry, core Core) *CheckedEntry {
 	return ce
 }
 
-// Should sets this CheckedEntry's CheckWriteHook, which controls whether a
+// Should sets this CheckedEntry's CheckWriteAction, which controls whether a
 // Core will panic or fatal after writing this log entry. Like AddCore, it's
 // safe to call on nil CheckedEntry references.
-func (ce *CheckedEntry) Should(ent Entry, should CheckWriteHook) *CheckedEntry {
+// Deprecated: Use After(ent Entry, after CheckWriteHook) instead.
+func (ce *CheckedEntry) Should(ent Entry, should CheckWriteAction) *CheckedEntry {
+	return ce.After(ent, should)
+}
+
+// After sets this CheckEntry's CheckWriteHook, which will be called after this
+// log entry has been written. It's safe to call this on nil CheckedEntry
+// references.
+func (ce *CheckedEntry) After(ent Entry, hook CheckWriteHook) *CheckedEntry {
 	if ce == nil {
 		ce = getCheckedEntry()
 		ce.Entry = ent
 	}
-	ce.should = should
+	ce.after = hook
 	return ce
 }
-
-func retcode(ce *CheckedEntry, fields []Field) int {
-	msg := ce.Message
-	for _, field := range fields {
-		if field.Type == ErrorType {
-			msg = field.Interface.(error).Error()
-			break
-		}
-	}
-	return int(crc32.ChecksumIEEE([]byte(msg)))%254 + 1
-}

zapcore/entry_test.go

@@ -64,7 +64,7 @@ func TestPutNilEntry(t *testing.T) {
 			assert.NotNil(t, ce, "Expected only non-nil CheckedEntries in pool.")
 			assert.False(t, ce.dirty, "Unexpected dirty bit set.")
 			assert.Nil(t, ce.ErrorOutput, "Non-nil ErrorOutput.")
-			assert.Nil(t, ce.should, "Unexpected terminal behavior.")
+			assert.Nil(t, ce.after, "Unexpected terminal behavior.")
 			assert.Equal(t, 0, len(ce.cores), "Expected empty slice of cores.")
 			assert.True(t, cap(ce.cores) > 0, "Expected pooled CheckedEntries to pre-allocate slice of Cores.")
 		}
@@ -131,20 +131,10 @@ func TestCheckedEntryWrite(t *testing.T) {
 		assert.Equal(t, 1, stub.Code, "Expected to exit when WriteThenFatal is set.")
 	})
 
-	t.Run("WriteThenPosixExit", func(t *testing.T) {
-		var ce *CheckedEntry
-		ce = ce.Should(Entry{Message: "foo"}, WriteThenPosixExit)
-		stub := exit.WithStub(func() {
-			ce.Write()
-		})
-		assert.True(t, stub.Exited, "Expected to exit when WriteThenPosixExit is set.")
-		assert.Equal(t, 38, stub.Code, "Expected to exit with specific code when WriteThenPosixExit is set.")
-	})
-
-	t.Run("Custom", func(t *testing.T) {
+	t.Run("After", func(t *testing.T) {
 		var ce *CheckedEntry
 		hook := &customHook{}
-		ce = ce.Should(Entry{}, hook)
+		ce = ce.After(Entry{}, hook)
 		ce.Write()
 		assert.True(t, hook.called, "Expected to call custom action after Write.")
 	})