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
Exclude error from successful analytics #704
Conversation
"extra": extra, | ||
} | ||
|
||
if e != nil { | ||
payload["error"] = reflect.TypeOf(e).String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we including the string representation of the error's type here? Is that the intent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the goal is providing some information about the error type but not exfiltrating sensitive information
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that would be an absolutely brilliant comment to add at line 352 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like we have some brilliant review comments at #493 (comment) 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
payload["error"] = reflect.TypeOf(e).String() | |
// Get the type of the error as a string, but not the error message itself, because it can be sensitive. | |
payload["error"] = reflect.TypeOf(e).String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couldn't resist
payload["error"] = reflect.TypeOf(e).String() | |
// only use error type (not message) to avoid potentially sensitive info | |
payload["error"] = reflect.TypeOf(e).String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a comment to clarify handling of the error field in the payload.
"extra": extra, | ||
} | ||
|
||
if e != nil { | ||
payload["error"] = reflect.TypeOf(e).String() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, that would be an absolutely brilliant comment to add at line 352 :)
Co-authored-by: Helio Machado <0x2b3bfa0+git@googlemail.com>
Closes #703; @casperdcl invenit