Refactor antctl error message #509
Conversation
|
Thanks for your PR. The following commands are available:
These commands can only be run by members of the vmware-tanzu organization. |
pkg/antctl/command_message.go
Outdated
| var code int | ||
| _ = result.StatusCode(&code) | ||
| if err := generate(opt.commandDefinition, opt.args, code); err != nil { | ||
| return err |
There was a problem hiding this comment.
the logic is fine, but it's a bit weird that 'err' is not an error code but the actual return value of the function! Perhaps errMsg could be a better name. (I know this is a bit pedant, sorry...)
There was a problem hiding this comment.
It makes sense. Code updated.
| }, | ||
| code: http.StatusNotFound, | ||
| expected: "NotFound: foo \"bar\" not found", | ||
| }, |
There was a problem hiding this comment.
more nit: add another case for code != http.StatusNotFound
There was a problem hiding this comment.
I added a case for error code not considered. As for them, command will get "Unknown error" message.
| switch code { | ||
| case http.StatusNotFound: | ||
| return fmt.Errorf("NotFound: %s \"%s\" not found", cd.use, args["name"]) | ||
| } |
There was a problem hiding this comment.
what about other error codes?
There was a problem hiding this comment.
I added case for http.StatusInternalServerError and the rest will get Unknown error.
You mentioned this case last time which returns http.StatusBadRequest:
# antctl get pod-interface -A my-pod
Error: error when requesting https://127.0.0.1:10443/podinterface?all-namespaces=true&name=my-pod&namespace=default: the server rejected our request for an unknown reason
But we don't have -A flag now so it is not considered.
These error codes are so far what I found in handlers and we can add more in the future.
There was a problem hiding this comment.
I saw that you added a default case with return fmt.Errorf("Unknown error"), that's what I was looking for
Turn error message into a more friendly one, including http.StatusNotFound, http.StatusInternalServerError.
| switch code { | ||
| case http.StatusNotFound: | ||
| return fmt.Errorf("NotFound: %s \"%s\" not found", cd.use, args["name"]) | ||
| } |
There was a problem hiding this comment.
I saw that you added a default case with return fmt.Errorf("Unknown error"), that's what I was looking for
|
/test-all |
Turn error message into a more friendly one, including http.StatusNotFound, http.StatusInternalServerError.
This reverts commit 2d09669.
This reverts commit 2d09669.
issue #506
Turn error message into a more friendly one:
Error: NotFound: resource "resouce name" not found