Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added checks for logger not being attached to the entry type and handling gracefully #1033

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
added checks for logger not being attached to the entry type
Shubham Sinha committed Oct 2, 2019
commit 183a5ec12e2eeb0efa72ad4ab17be9844666f9bb
29 changes: 29 additions & 0 deletions entry.go
Original file line number Diff line number Diff line change
@@ -3,6 +3,7 @@ package logrus
import (
"bytes"
"context"
"errors"
"fmt"
"os"
"reflect"
@@ -30,6 +31,10 @@ const (
knownLogrusFrames int = 4
)

var (
loggerNotAttached = errors.New("entry not attached to any logger")
)

func init() {
bufferPool = &sync.Pool{
New: func() interface{} {
@@ -88,6 +93,10 @@ func NewEntry(logger *Logger) *Entry {
// Returns the string representation from the reader and ultimately the
// formatter.
func (entry *Entry) String() (string, error) {
// add a check for logger being nil because entry is exposed
if entry.Logger == nil {
return "", loggerNotAttached
}
serialized, err := entry.Logger.Formatter.Format(entry)
if err != nil {
return "", err
@@ -241,6 +250,11 @@ func (entry Entry) log(level Level, msg string) {
}

func (entry *Entry) fireHooks() {
// add a check for logger being nil because entry is exposed
if entry.Logger == nil {
fmt.Fprintf(os.Stderr, "Logger not attached\n")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this shouldn't happen. If no logger is attached, I don't expect any output.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an error scenario. So at least the logs should go to the standard error output. No error should be blindly ignored.

return
}
entry.Logger.mu.Lock()
defer entry.Logger.mu.Unlock()
err := entry.Logger.Hooks.Fire(entry.Level, entry)
@@ -250,6 +264,11 @@ func (entry *Entry) fireHooks() {
}

func (entry *Entry) write() {
// add a check for logger being nil because entry is exposed
if entry.Logger == nil {
fmt.Fprintf(os.Stderr, "Logger not attached\n")
return
}
entry.Logger.mu.Lock()
defer entry.Logger.mu.Unlock()
serialized, err := entry.Logger.Formatter.Format(entry)
@@ -345,6 +364,11 @@ func (entry *Entry) Errorf(format string, args ...interface{}) {

func (entry *Entry) Fatalf(format string, args ...interface{}) {
entry.Logf(FatalLevel, format, args...)
// add a check for logger being nil because entry is exposed
if entry.Logger == nil {
fmt.Fprintf(os.Stderr, "Logger not attached\n")
return
}
entry.Logger.Exit(1)
}

@@ -355,6 +379,11 @@ func (entry *Entry) Panicf(format string, args ...interface{}) {
// Entry Println family functions

func (entry *Entry) Logln(level Level, args ...interface{}) {
// add a check for logger being nil because entry is exposed
if entry.Logger == nil {
fmt.Fprintf(os.Stderr, "Logger not attached\n")
return
}
if entry.Logger.IsLevelEnabled(level) {
entry.Log(level, entry.sprintlnn(args...))
}