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

Error logs mangle return error message #126

Closed
tp opened this issue Sep 4, 2019 · 3 comments · Fixed by #130
Closed

Error logs mangle return error message #126

tp opened this issue Sep 4, 2019 · 3 comments · Fixed by #130
Labels
bug Something isn't working CLI Issues for ghz CLI

Comments

@tp
Copy link
Contributor

tp commented Sep 4, 2019

Describe the bug
We have an error message that includes an encoded URLs. In ghz that seems to be used in a format string and hence leading to this output:

Error distribution:
  [20000]   rpc error: code = Unknown desc = Non-success status code 401 for https://host.com/api/outfits/v1?appId=10&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISSING)=10569&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISSING)=10570&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISSING)=10568&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISSING)=10561&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISSING)=10564&outfitIds%!B(MISSING)%!D(MISSING)%!B(MISSING)%!D(MISS

To Reproduce

Return an error message with % in it

Expected behavior

Should log the error message verbatim

Environment

  • OS: 10.14.6
  • ghz: 0.41.0

Screenshots

image

In BloomRPC for example the response looks just fine:

image

Additional context
Add any other context about the problem here.

@tp
Copy link
Contributor Author

tp commented Sep 4, 2019

I had a quick look into this codebase but couldn't easily spot the file that prints this.

Looks pretty clear what kind of string build / formatting would cause this, if you can point me into the right direction I can have a stab at fixing the issue as well.

@bojand bojand added bug Something isn't working CLI Issues for ghz CLI labels Sep 6, 2019
@bojand
Copy link
Owner

bojand commented Sep 6, 2019

Hmm that's not very useful. It might be something to do with how we print errors using the default template. I will try and take a look soon when I get a bit more time.

@tp
Copy link
Contributor Author

tp commented Sep 6, 2019

@bojand Isn't is just this line: https://github.com/bojand/ghz/blob/master/printer/printer.go#L46 ?

return rp.printf(buf.String())

I guess if you change that from printf to print it will be fine. Should not change anything, as there are no parameters anyway.

arinto added a commit to arinto/ghz that referenced this issue Oct 4, 2019
bojand added a commit that referenced this issue Oct 7, 2019
Use Fprint instead of Fprintf to fix #126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CLI Issues for ghz CLI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants