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

Fix the handling of %T in redaction - get real type instead of redact.escapeArg #53207

Closed
knz opened this issue Aug 21, 2020 · 0 comments · Fixed by #53764
Closed

Fix the handling of %T in redaction - get real type instead of redact.escapeArg #53207

knz opened this issue Aug 21, 2020 · 0 comments · Fixed by #53764
Assignees
Labels
A-error-handling Error messages, error propagations/annotations C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. regression Regression from a release.

Comments

@knz
Copy link
Contributor

knz commented Aug 21, 2020

PR #53199 is introducing the use of the redact package for errors, and it was added for logging before.

However the logic in redact breaks the %T formatting verb from the stdlib.

This needs to be fixed.

Upstream: cockroachdb/redact#1

Symptom of the problem

For example, errors.AssertionFailedf("expected bool, got %T", d) should report

"expected bool, got *tree.DBool"

And instead the following is produced:

"expected bool, got *redact.escapeArg"

Workaround

Until this is fixed, if the type is needed change

errors.Newf(.... "%T", val)
log.Infof(... "%T", val)

to

errors.Newf(.... "%s", redact.Safe(fmt.Sprintf("%T", val))
log.Infof(... "%s", redact.Safe(fmt.Sprintf("%T", val))
@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. regression Regression from a release. A-error-handling Error messages, error propagations/annotations labels Aug 21, 2020
@knz knz self-assigned this Aug 21, 2020
@knz knz changed the title Fix the handling of %T in errors Fix the handling of %T in errors - get real type instead of redact.escapeArg Aug 22, 2020
@knz knz changed the title Fix the handling of %T in errors - get real type instead of redact.escapeArg Fix the handling of %T in redaction - get real type instead of redact.escapeArg Aug 22, 2020
@craig craig bot closed this as completed in e04bc9a Sep 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error-handling Error messages, error propagations/annotations C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. regression Regression from a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant