natefinch / lumberjack

lumberjack is a log rolling package for Go
MIT License
4.79k stars 591 forks source link

Lumberjack does not test if provided Filename is writeable when initializing Logger #72

Open davidsnyder-jask opened 5 years ago

davidsnyder-jask commented 5 years ago

Using Lumberjack v2: gopkg.in/natefinch/lumberjack.v2 v2.0.0

        logFile := &lumberjack.Logger{
            Filename:   "/unwriteable/file.log",
            MaxSize:    10, // megabytes
            MaxBackups: 3,
            MaxAge:     28, //days
            Compress:   true,
        }

Given that "/unwriteable/file.log" has 444 permissions, I found that Logger didn't complain at all that it couldn't write to the provided file. Is this intended behavior?

kolbe commented 5 years ago

I just hit this same thing. It is a very surprising result that specifying an obviously nonsensical log file just causes log entries to be essentially discarded without the app or user ever being the wiser. Yikes!

natefinch commented 5 years ago

Hmm... There should probably be a constructor that returns a logger and an error, so it can do some sanity checks. I would expect that the writes to an invalid file would return an error, but ignoring errors from log output is pretty normal practice (not good practice, but normal).

kolbe commented 5 years ago

I do think a constructor that double-checks whether the given file is writeable would be a big help.

And you're very right that ignoring errors from log output is not good. It should probably be a more standard practice to wrap the chosen log library in something that handles it in an application-specific way.

xiegeo commented 5 years ago

I just hit this same thing. It is a very surprising result that specifying an obviously nonsensical log file just causes log entries to be essentially discarded without the app or user ever being the wiser. Yikes!

I think this is expected behavior, if you printf in a headless program, we do not expect an error.

Assuming logging is the error reporting tool of last resort, I don't see the point. The lack of logs should be informative enough.

What I would do to cover this situation is to add a line of logging during the start of the program, such as "[datetime] abc version xyz started", you can also add error checks for this print or write method call. I think that log object creation should be allowed to have no side effects, so an error check doesn't make sense here.