glg icon indicating copy to clipboard operation
glg copied to clipboard

Why does log commands return error?

Open gucio321 opened this issue 3 years ago • 7 comments

Hi there, The logger is awesome ;-) The only fact, i'm quit dissapointed is that the log commands return errors... It seems a bit strange for me, since loggers are designed to handle errors, not produce them :smile: This is a bit annoying for me, since GoLand complains about "unhandled error" image

What about changing API to something like:

glg.Info("some log")
if glg.Err() != nil { /* handle error if you really need this */ }

gucio321 avatar Oct 27 '22 15:10 gucio321

Hi @gucio321 Thank you for the feedback.

It is true that many people feel uncomfortable with the log command returning an error. The log output in glg is written to the input/output device or io.Writer, because the Go standard log stdout is also written as io.Writer. Glg internally outputs errors on log output to avoid hiding such errors.

Without checking for errors, users cannot be assured that the log of an important failure has been successfully written to stdout, stderr, or any other write destination.

Using the modification you suggested, if a log error occurs between glg.Infof and glg.Err in other goroutine threads, it will be difficult to ensure the consistency of the log, and will cause more complexity for the user than if the error returns from the Function Call implementation.

kpango avatar Oct 31 '22 03:10 kpango

Thanks for your answer! I can see the problem. What about callback-like error handling? E.g.

glg.SetErrorCallback(func(err error) {panic(err)})

The panic could be a default callback and user could change it

gucio321 avatar Oct 31 '22 07:10 gucio321

I think that idea seems to work, but if we implement it and rewrite the current Interface, it would require a v2 release. The other idea is to create a wrapper. In my OSS project, I have created the following wrapper to organize error handling. Wrapper: https://github.com/vdaas/vald/blob/main/internal/log/glg/glg.go Error Handle: https://github.com/vdaas/vald/blob/main/internal/log/retry/retry.go#L46-L85

kpango avatar Oct 31 '22 07:10 kpango

Yah, In my project I'm using a kind of abstraction from the logging library, like this:

logger.go
/*
 * Copyright (c) 2022. The Greater Heptavirate: programming lodge (https://github.com/TheGreaterHeptavirate). All Rights Reserved.
 *
 * All copies of this software (if not stated otherwise) are dedicated ONLY to personal, non-commercial use.
 */

// Package logger contains an abstraction from the currently used logging library.
package logger

import "github.com/kpango/glg"

// Info logs a message at level Info on the standard logger.
func Info(args ...interface{}) {
	if err := glg.Info(args...); err != nil {
		panic(err)
	}
}

// Infof logs a message at level Info on the standard logger.
// It uses fmt.Sprintf to format the message.
func Infof(format string, args ...interface{}) {
	if err := glg.Infof(format, args...); err != nil {
		panic(err)
	}
}

// Debug logs a message at level Debug on the standard logger.
func Debug(args ...interface{}) {
	if err := glg.Debug(args...); err != nil {
		panic(err)
	}
}

// Debugf logs a message at level Debug on the standard logger.
// It uses fmt.Sprintf to format the message.
func Debugf(format string, args ...interface{}) {
	if err := glg.Debugf(format, args...); err != nil {
		panic(err)
	}
}

// Warn logs a message at level Warn on the standard logger.
func Warn(args ...interface{}) {
	if err := glg.Warn(args...); err != nil {
		panic(err)
	}
}

// Warnf logs a message at level Warn on the standard logger.
// It uses fmt.Sprintf to format the message.
func Warnf(format string, args ...interface{}) {
	if err := glg.Warnf(format, args...); err != nil {
		panic(err)
	}
}

// Error logs a message at level Error on the standard logger.
func Error(args ...interface{}) {
	if err := glg.Error(args...); err != nil {
		panic(err)
	}
}

// Errorf logs a message at level Error on the standard logger.
// It uses fmt.Sprintf to format the message.
func Errorf(format string, args ...interface{}) {
	if err := glg.Errorf(format, args...); err != nil {
		panic(err)
	}
}

// Fatal logs a message at level Fatal on the standard logger.
func Fatal(args ...interface{}) {
	glg.Fatal(args...)
}

// Fatalf logs a message at level Fatal on the standard logger.
// It uses fmt.Sprintf to format the message.
func Fatalf(format string, args ...interface{}) {
	glg.Fatalf(format, args...)
}

// Success logs a message at level Info on the standard logger.
// This announces a successful operation.
func Success(args ...interface{}) {
	if err := glg.Success(args...); err != nil {
		panic(err)
	}
}

// Successf logs a message at level Info on the standard logger.
// It uses fmt.Sprintf to format the message.
func Successf(format string, args ...interface{}) {
	if err := glg.Successf(format, args...); err != nil {
		panic(err)
	}
}

// SetLevel sets the logging level.
func SetLevel(l LogLevel) {
	glg.Get().SetLevel(l.Logger())
}
log_level.go
/*
 * Copyright (c) 2022. The Greater Heptavirate: programming lodge (https://github.com/TheGreaterHeptavirate). All Rights Reserved.
 *
 * All copies of this software (if not stated otherwise) are dedicated ONLY to personal, non-commercial use.
 */

package logger

import (
	"github.com/kpango/glg"
)

//go:generate stringer -type=LogLevel -trimprefix=LogLevel -output=log_level_string.go

// LogLevel is an abstraction from the currently used logging library.
type LogLevel byte

// log levels
const (
	LogLevelNotSpecified LogLevel = iota
	LogLevelDebug
	LogLevelInfo
	LogLevelWarn
	LogLevelError
	LogLevelFatal
)

// Logger returns the LogLevel converted to currently
// used logging library's log level.
func (l LogLevel) Logger() glg.LEVEL {
	m := map[LogLevel]glg.LEVEL{
		LogLevelDebug: glg.DEBG,
		LogLevelInfo:  glg.INFO,
		LogLevelWarn:  glg.WARN,
		LogLevelError: glg.ERR,
		LogLevelFatal: glg.FATAL,
	}

	if v, ok := m[l]; ok {
		return v
	}

	panic("unknown log level")
}

However, imo adding this callback-like error handling in v2 would be a nice idea ;-)

gucio321 avatar Oct 31 '22 16:10 gucio321

I see. I will be busy with our main project in November, so the v2 release including this feature is scheduled for early next year. Is this schedule works for you? Or contribution is also welcome.

kpango avatar Nov 02 '22 01:11 kpango

No problem, If I get some time, I can try contributing

gucio321 avatar Nov 02 '22 08:11 gucio321

Feel the contribution made by the big guy, this is indeed a good log library, may I ask this change about the error callback has online time?

gitcfly avatar Sep 06 '23 07:09 gitcfly