From ced531341e99935442723f91ef6949326a0f66cc Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Wed, 5 Nov 2014 13:49:58 -0500 Subject: [PATCH 1/4] Saving off entry string for use in panic as the reader is not seekable and without this the panic string is always empty :scream_cat: --- entry.go | 7 +++++-- examples/basic/basic.go | 17 ++++++++++++++--- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/entry.go b/entry.go index a77c4b0..dc639b3 100644 --- a/entry.go +++ b/entry.go @@ -88,10 +88,13 @@ func (entry *Entry) log(level Level, msg string) { entry.Logger.mu.Unlock() } + var panicBuf bytes.Buffer + teeOut := io.TeeReader(reader, &panicBuf) + entry.Logger.mu.Lock() defer entry.Logger.mu.Unlock() - _, err = io.Copy(entry.Logger.Out, reader) + _, err = io.Copy(entry.Logger.Out, teeOut) if err != nil { fmt.Fprintf(os.Stderr, "Failed to write to log, %v\n", err) } @@ -100,7 +103,7 @@ func (entry *Entry) log(level Level, msg string) { // panic() to use in Entry#Panic(), we avoid the allocation by checking // directly here. if level <= PanicLevel { - panic(reader.String()) + panic(panicBuf.String()) } } diff --git a/examples/basic/basic.go b/examples/basic/basic.go index 3594550..a62ba45 100644 --- a/examples/basic/basic.go +++ b/examples/basic/basic.go @@ -12,6 +12,17 @@ func init() { } func main() { + defer func() { + err := recover() + if err != nil { + log.WithFields(logrus.Fields{ + "omg": true, + "err": err, + "number": 100, + }).Fatal("The ice breaks!") + } + }() + log.WithFields(logrus.Fields{ "animal": "walrus", "size": 10, @@ -23,7 +34,7 @@ func main() { }).Warn("The group's number increased tremendously!") log.WithFields(logrus.Fields{ - "omg": true, - "number": 100, - }).Fatal("The ice breaks!") + "animal": "orca", + "size": 9009, + }).Panic("It's over 9000!") } From f302a46d2ab1439e88e51192fa8a4abfe511293d Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Wed, 5 Nov 2014 14:07:40 -0500 Subject: [PATCH 2/4] Switching to io.MultiWriter for clarity --- entry.go | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/entry.go b/entry.go index dc639b3..80638f3 100644 --- a/entry.go +++ b/entry.go @@ -71,6 +71,8 @@ func (entry *Entry) WithFields(fields Fields) *Entry { } func (entry *Entry) log(level Level, msg string) { + var panicBuf bytes.Buffer + entry.Time = time.Now() entry.Level = level entry.Message = msg @@ -88,13 +90,10 @@ func (entry *Entry) log(level Level, msg string) { entry.Logger.mu.Unlock() } - var panicBuf bytes.Buffer - teeOut := io.TeeReader(reader, &panicBuf) - entry.Logger.mu.Lock() defer entry.Logger.mu.Unlock() - _, err = io.Copy(entry.Logger.Out, teeOut) + _, err = io.Copy(io.MultiWriter(entry.Logger.Out, &panicBuf), reader) if err != nil { fmt.Fprintf(os.Stderr, "Failed to write to log, %v\n", err) } From 9836f1ba0e5f6a863b7868d58c6957f9f84a3dc4 Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Wed, 5 Nov 2014 20:39:35 -0500 Subject: [PATCH 3/4] Adding tests for PanicLevel behavior, plus changing behavior! :scream_cat: --- entry.go | 6 ++---- entry_test.go | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 entry_test.go diff --git a/entry.go b/entry.go index 80638f3..e164eec 100644 --- a/entry.go +++ b/entry.go @@ -71,8 +71,6 @@ func (entry *Entry) WithFields(fields Fields) *Entry { } func (entry *Entry) log(level Level, msg string) { - var panicBuf bytes.Buffer - entry.Time = time.Now() entry.Level = level entry.Message = msg @@ -93,7 +91,7 @@ func (entry *Entry) log(level Level, msg string) { entry.Logger.mu.Lock() defer entry.Logger.mu.Unlock() - _, err = io.Copy(io.MultiWriter(entry.Logger.Out, &panicBuf), reader) + _, err = io.Copy(entry.Logger.Out, reader) if err != nil { fmt.Fprintf(os.Stderr, "Failed to write to log, %v\n", err) } @@ -102,7 +100,7 @@ func (entry *Entry) log(level Level, msg string) { // panic() to use in Entry#Panic(), we avoid the allocation by checking // directly here. if level <= PanicLevel { - panic(panicBuf.String()) + panic(entry) } } diff --git a/entry_test.go b/entry_test.go new file mode 100644 index 0000000..2ad192e --- /dev/null +++ b/entry_test.go @@ -0,0 +1,53 @@ +package logrus + +import ( + "bytes" + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestEntryPanicln(t *testing.T) { + errBoom := fmt.Errorf("boom time") + + defer func() { + p := recover() + assert.NotNil(t, p) + + switch pVal := p.(type) { + case *Entry: + assert.Equal(t, "kaboom", pVal.Message) + assert.Equal(t, errBoom, pVal.Data["err"]) + default: + t.Fatal() + } + }() + + logger := New() + logger.Out = &bytes.Buffer{} + entry := NewEntry(logger) + entry.WithField("err", errBoom).Panicln("kaboom") +} + +func TestEntryPanicf(t *testing.T) { + errBoom := fmt.Errorf("boom again") + + defer func() { + p := recover() + assert.NotNil(t, p) + + switch pVal := p.(type) { + case *Entry: + assert.Equal(t, "kaboom true", pVal.Message) + assert.Equal(t, errBoom, pVal.Data["err"]) + default: + t.Fatal() + } + }() + + logger := New() + logger.Out = &bytes.Buffer{} + entry := NewEntry(logger) + entry.WithField("err", errBoom).Panicf("kaboom %v", true) +} From 5164671bc642569fbec92d6805c5da24b38d877c Mon Sep 17 00:00:00 2001 From: Dan Buch Date: Thu, 6 Nov 2014 08:42:52 -0500 Subject: [PATCH 4/4] Making panic level assertions more helpful --- entry_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/entry_test.go b/entry_test.go index 2ad192e..98717df 100644 --- a/entry_test.go +++ b/entry_test.go @@ -20,7 +20,7 @@ func TestEntryPanicln(t *testing.T) { assert.Equal(t, "kaboom", pVal.Message) assert.Equal(t, errBoom, pVal.Data["err"]) default: - t.Fatal() + t.Fatalf("want type *Entry, got %T: %#v", pVal, pVal) } }() @@ -42,7 +42,7 @@ func TestEntryPanicf(t *testing.T) { assert.Equal(t, "kaboom true", pVal.Message) assert.Equal(t, errBoom, pVal.Data["err"]) default: - t.Fatal() + t.Fatalf("want type *Entry, got %T: %#v", pVal, pVal) } }()