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

Ability to use Err() method without Msg() #410

Closed
1k-off opened this issue Feb 10, 2022 · 5 comments · Fixed by #413
Closed

Ability to use Err() method without Msg() #410

1k-off opened this issue Feb 10, 2022 · 5 comments · Fixed by #413

Comments

@1k-off
Copy link

1k-off commented Feb 10, 2022

Hi guys.

There's one frustrating thing with logger.
Let's see the code:

package main

import (
	"errors"
	"github.com/rs/zerolog"
	"os"
)

func main() {
	logger := zerolog.New(zerolog.ConsoleWriter{
		Out:        os.Stdout,
		TimeFormat: "2006-01-02 15:04:05",
	}).With().Timestamp().Logger()
	zerolog.SetGlobalLevel(zerolog.InfoLevel)

	err := errors.New("test error") 

	logger.Fatal().Err(err)      // interested string

	str := "some functionality after fatal"
	logger.Info().Msg(str)
}

Program continues after calling the Fatal().Err() method. But when I add Msg() method, like logger.Fatal().Err(err).Msg("") - everything works as expected.

The same thing with logger.Error().Err(err). Error is not displayed in the log.

But. Let's compare output of Info log and Error log with Msg:
8Pq2jSS

You may see one extra space at the beginning of error line. It's a bit strange.

Could you please add possibility to call Err() method without Msg()? Or at least, delete extra space? ;)

@mitar
Copy link
Contributor

mitar commented Feb 19, 2022

I am also bothered by this, but it is not because of Msg("") call, in fact, zerolog already skips printing a part if it is empty in writePart:

	if len(s) > 0 {
		buf.WriteString(s)
		if p != w.PartsOrder[len(w.PartsOrder)-1] { // Skip space for last part
			buf.WriteByte(' ')
		}
	}

The issue is bad interaction between buf.WriteByte(' ') above and writeFields:

	if len(fields) > 0 {
		buf.WriteByte(' ')
	}

One is appending space delimiter, the other one is prepending. Also, writePart skips a space only for the last part in the slice, and not for the last written part.

I think writePart (or its caller) should be prepending, and only if it is not the first part to be written out.

@mitar
Copy link
Contributor

mitar commented Feb 19, 2022

I made #413 to fix this.

@rs rs closed this as completed in #413 Feb 19, 2022
@ykadowak
Copy link

Why don't we make it possible to use Err() without Msg() as in the title of this issue?
Is it either

  1. Does not match the design of this library
  2. Nobody has ever implemented
    ...or something else?

Sorry to ask this without looking at the codebase. Thanks.

@mitar
Copy link
Contributor

mitar commented Jan 24, 2023

So there are two parts to this issue. One is that previously (before this issue was closed), there was an extra space printed when there was no message just error. This has been fixed.

The other is how to log just an error. For that, you can just use .Err(error).Send(). I do not think there is a need to be more succinct then that. I think the issue is just that novice users are maybe surprised that they have to finish building the log entry with .Msg(...) or .Send(). Without calling either of those, nothing is logged (just a log entry is being constructed). This is the design of the library so I do not think .Err warrants a special case here.

@ykadowak
Copy link

Didn't realize there's .Send() method until now. Also realized that there's a specific linter for it in this repo https://github.com/rs/zerolog/tree/master/cmd/lint. So it's no longer a concern. Thank you!

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

Successfully merging a pull request may close this issue.

3 participants