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

Why does log commands return error? #114

Open
gucio321 opened this issue Oct 27, 2022 · 7 comments
Open

Why does log commands return error? #114

gucio321 opened this issue Oct 27, 2022 · 7 comments

Comments

@gucio321
Copy link

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 😄
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 */ }
@kpango
Copy link
Owner

kpango commented Oct 31, 2022

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.

@gucio321
Copy link
Author

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

@kpango
Copy link
Owner

kpango commented Oct 31, 2022

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

@gucio321
Copy link
Author

gucio321 commented Oct 31, 2022

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 ;-)

@kpango
Copy link
Owner

kpango commented Nov 2, 2022

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.

@gucio321
Copy link
Author

gucio321 commented Nov 2, 2022

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

@gitcfly
Copy link

gitcfly commented Sep 6, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants