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

Invalid json when using only an objet #152

Closed
IxDay opened this issue May 8, 2019 · 4 comments · Fixed by #154
Closed

Invalid json when using only an objet #152

IxDay opened this issue May 8, 2019 · 4 comments · Fixed by #154
Labels

Comments

@IxDay
Copy link
Contributor

IxDay commented May 8, 2019

When I use zerolog with only an object the json output is incorrect.

import (
    "os"

    "github.com/rs/zerolog"
)
type foo struct{}

func (f foo) MarshalZerologObject(e *zerolog.Event) { e.Str("bar", "baz") }

func main() {
    logger := zerolog.New(os.Stdout).With().Object("foo", foo{}).Logger()   
    logger.Info().Msg("qux")
}

This will output:

{"level":"info",,"foo":{"bar":"baz"},"message":"qux"}
               ^-- invalid double comma

When pushing any additional field other than object:

func main() {
    logger := zerolog.New(os.Stdout).With().Str("bar", "foo").Object("foo", foo{}).Logger()   
    logger.Info().Msg("qux")
}

I get a valid json:

{"level":"info","bar":"foo","foo":{"bar":"baz"},"message":"qux"}
@rs rs added the bug label May 23, 2019
@IxDay
Copy link
Contributor Author

IxDay commented May 24, 2019

The bug is actually triggered if Object is the first pushed to the logger, i.e:

logger := zerolog.New(os.Stdout).With().Object("foo", foo{}).Str("bar", "foo").Logger() 
logger.Info().Msg("qux")

Problem seems to come from AppendObjectData I did a quick fix which seems to be working, however, code is not really elegant:

func (Encoder) AppendObjectData(dst []byte, o []byte) []byte {
	if o[0] == '{' {
		if len(dst) == 0 {
			o = o[1:]
		} else {
			o[0] = ','
		}
	} else if len(dst) > 1 {
		dst = append(dst, ',')
	}
	return append(dst, o...)
}

@rs
Copy link
Owner

rs commented May 24, 2019

With some comment is should be ok. If you can add unit test too, it would be awesome (considering you are willing to send a PR).

@IxDay
Copy link
Contributor Author

IxDay commented May 27, 2019

I can do that :) give me a bit of time to implement it

IxDay added a commit to IxDay/zerolog that referenced this issue May 27, 2019
When pushing an object to the logger, and this object was the first
field added. Zerolog was outputting an invalid json blob, issuing an
extra comma before the object. This patch ensure that JSON is still valid
even if an object is pushed first to the logger.

Fixes rs#152
IxDay added a commit to IxDay/zerolog that referenced this issue May 27, 2019
When pushing an object to the logger, and this object was the first
field added. Zerolog was outputting an invalid json blob, issuing an
extra comma before the object. This patch ensure that JSON is still valid
even if an object is pushed first to the logger.

Fixes rs#152
IxDay added a commit to IxDay/zerolog that referenced this issue May 28, 2019
When pushing an object to the logger, and this object was the first
field added. Zerolog was outputting an invalid json blob, issuing an
extra comma before the object. This patch ensure that JSON is still valid
even if an object is pushed first to the logger.

Fixes rs#152
@IxDay
Copy link
Contributor Author

IxDay commented Jun 5, 2019

Is there anything I need to add?

@rs rs closed this as completed in #154 Jun 5, 2019
rs pushed a commit that referenced this issue Jun 5, 2019
When pushing an object to the logger, and this object was the first
field added. Zerolog was outputting an invalid json blob, issuing an
extra comma before the object. This patch ensure that JSON is still valid
even if an object is pushed first to the logger.

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

Successfully merging a pull request may close this issue.

2 participants