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

JSONFormatter isn't handling functions as Field values #642

Closed
xandrewrampulla opened this issue Sep 29, 2017 · 6 comments
Closed

JSONFormatter isn't handling functions as Field values #642

xandrewrampulla opened this issue Sep 29, 2017 · 6 comments
Assignees

Comments

@xandrewrampulla
Copy link

When using the JSONFormatter, if I accidentally pass a function as a Field value, the entire log message is lost. It would be better if only that one field was lost (even better would be to log that the field couldn't be translated into JSON properly).

Simple example code

package main

import (
	log "github.com/sirupsen/logrus"
)

func myfun() string {
	return ""
}

func main() {

	log.SetFormatter(&log.JSONFormatter{})

	log.WithFields(log.Fields{
		"animal": "walrus",
		"myfunc": myfun,
	}).Info("A walrus appears")
}

Current output
Failed to obtain reader, Failed to marshal fields to JSON, json: unsupported type: func() string

@dgsb dgsb self-assigned this Sep 27, 2018
@dgsb
Copy link
Collaborator

dgsb commented Sep 30, 2018

Indeed this is annoying

dgsb added a commit that referenced this issue Sep 30, 2018
We skip those unprintable fields and an error field
instead of dropping the whole trace.

Fixes #642
@dgsb dgsb closed this as completed in #832 Sep 30, 2018
smacker added a commit to smacker/logrus that referenced this issue Dec 5, 2018
Before there was introduced a fix for JSONFormatter when func type value
passed as Field. This commit does the same but for pointer to func.

Ref:
sirupsen#642
sirupsen#832
@smacker
Copy link
Contributor

smacker commented Dec 5, 2018

I got the same error but with a pointer to function:

package main

import (
	log "github.com/sirupsen/logrus"
)

func main() {
	myfun := func() {}
	log.SetFormatter(&log.JSONFormatter{})
	log.WithFields(log.Fields{
		"animal": "walrus",
		"myfunc": &myfun,
	}).Info("A walrus appears")
}

results in Failed to obtain reader, Failed to marshal fields to JSON, json: unsupported type: func()

Because logrus loses all context it's very hard to find in the code base where the error has happened.
I propose the same fix: #870

@dimatkach
Copy link

I am still seeing this issue.

      import (
         log "github.com/sirupsen/logrus"
      )
      type Foo struct {
         foo func() string
      }


     func main() {
        foo :=  Foo {
          func () string { return "foo" }
         }
   
        log.SetFormatter(&log.JSONFormatter{})
        log.WithField("foo", foo).Info("Foo!")
      }

It looks like the PR referenced above only fixes the case where field itself is a func, but if it is a struct that has a func field somewhere inside of it, there is still a problem. The solution either needs to be more involved, and go recursively inside each of the struct.

@linnaname
Copy link

I am still seeing this issue in v1.7.0

Test Code:
log.SetFormatter(&log.JSONFormatter{})
req,_ := http.NewRequest("POST", "127.0.0.1",nil)
log.WithFields(log.Fields{ "req": req, }).Error("new request failed")

Output:Failed to obtain reader, failed to marshal fields to JSON, json: unsupported type: func() (io.ReadCloser, error)

@oxzi
Copy link

oxzi commented Feb 13, 2021

Today I came across the same error for a struct which has a function as a field type. As a workaround, a tag can be added to the field, so that this is not taken into account for the JSON, json:"-".

oxzi added a commit to dtn7/dtn7-gold that referenced this issue Feb 13, 2021
Exclude the peer discovery Manager's function field from the
JSONFormatter used by logrus. Otherwise, the struct cannot be encoded.

This issue is already known for logrus:
<sirupsen/logrus#642>
@vishalkanaujia
Copy link

vishalkanaujia commented Jul 5, 2021

If you are logging HTTP request/response, use httputil.DumpRequest/ httputil.DumpResponse instead of zap.Any

cgxxv pushed a commit to cgxxv/logrus that referenced this issue Mar 25, 2022
We skip those unprintable fields and an error field
instead of dropping the whole trace.

Fixes sirupsen#642
cgxxv pushed a commit to cgxxv/logrus that referenced this issue Mar 25, 2022
Before there was introduced a fix for JSONFormatter when func type value
passed as Field. This commit does the same but for pointer to func.

Ref:
sirupsen#642
sirupsen#832
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants