From af41a51636a06c626b65bd6e5fb396e0264e14bd Mon Sep 17 00:00:00 2001 From: Simon Eskildsen Date: Mon, 6 Feb 2017 20:12:41 -0500 Subject: [PATCH] hooks: don't skip entire chain if a hook fails --- CHANGELOG.md | 4 ++++ entry.go | 2 +- hook_test.go | 22 ++++++++++++++++++++++ hooks.go | 10 ++++++++-- 4 files changed, 35 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47a6c4b..9e11656 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +# 0.11.2 + +* bug: don't skip hooks on failure (#474) + # 0.11.1 * bug: fix tty detection with custom out (#471) diff --git a/entry.go b/entry.go index 4edbe7a..d62281e 100644 --- a/entry.go +++ b/entry.go @@ -95,7 +95,7 @@ func (entry Entry) log(level Level, msg string) { if err := entry.Logger.Hooks.Fire(level, &entry); err != nil { entry.Logger.mu.Lock() - fmt.Fprintf(os.Stderr, "Failed to fire hook: %v\n", err) + fmt.Fprintf(os.Stderr, "Failed to fire hook(s): %v\n", err) entry.Logger.mu.Unlock() } buffer = bufferPool.Get().(*bytes.Buffer) diff --git a/hook_test.go b/hook_test.go index 13f34cb..303e273 100644 --- a/hook_test.go +++ b/hook_test.go @@ -1,6 +1,7 @@ package logrus import ( + "errors" "testing" "github.com/stretchr/testify/assert" @@ -120,3 +121,24 @@ func TestErrorHookShouldFireOnError(t *testing.T) { assert.Equal(t, hook.Fired, true) }) } + +type FailingHook struct { + TestHook +} + +func (hook *FailingHook) Fire(entry *Entry) error { + return errors.New("sad walrus") +} + +func TestHookShouldFireAfterFailureHook(t *testing.T) { + failureHook := new(FailingHook) + hook := new(TestHook) + + LogAndAssertJSON(t, func(log *Logger) { + log.Hooks.Add(failureHook) + log.Hooks.Add(hook) + log.Error("test") + }, func(fields Fields) { + assert.Equal(t, hook.Fired, true) + }) +} diff --git a/hooks.go b/hooks.go index 3f151cd..e42d600 100644 --- a/hooks.go +++ b/hooks.go @@ -1,5 +1,9 @@ package logrus +import ( + "fmt" +) + // A hook to be fired when logging on the logging levels returned from // `Levels()` on your implementation of the interface. Note that this is not // fired in a goroutine or a channel with workers, you should handle such @@ -24,11 +28,13 @@ func (hooks LevelHooks) Add(hook Hook) { // Fire all the hooks for the passed level. Used by `entry.log` to fire // appropriate hooks for a log entry. func (hooks LevelHooks) Fire(level Level, entry *Entry) error { + var aggErr error + for _, hook := range hooks[level] { if err := hook.Fire(entry); err != nil { - return err + aggErr = fmt.Errorf("%s | %s", aggErr, err) } } - return nil + return aggErr }