barracuda-cloudgen-access / access-cli

Allows to access all CloudGen Access Enterprise Console APIs from the command line.
Apache License 2.0
10 stars 4 forks source link

Do not show cli usage args if error comes from server #11

Closed oNaiPs closed 4 years ago

oNaiPs commented 4 years ago

When a command errors because of a server response, the CLI usage args should not be printed, since the command is actually correct.

gbl08ma commented 4 years ago

Unfortunately cobra gives us no choice about whether the usage string is displayed if an error is returned by the command. One option is to have the command function not return an error in the event of a server error.

I have been looking into this option and it will be complex to implement (which is why it's not done yet), as the error handling functions will have to determine whether the error actually comes from the server or from some (un)serialization function inside the generated client code. This would be easy if such code used the new functionality introduced in Go 1.13 in the errors package (marking all "remote" errors as such, in a way that we could then easily check if the error is of type "remote", similar to how in Java one can distinguish between IOExceptions and more specific exceptions such as FileNotFoundException). Of course, this is not the case and I don't expect go-swagger to update their generator to support this 1.13 feature any time soon. In the meantime, I think I will resort to some "dirty hack" such as checking if the error string follows a format similar to [POST /auth/sign_in][401] (using e.g. a regex), in which case it probably is a remote error and therefore we should avoid returning an error to cobra, so that the usage string is not displayed.

gbl08ma commented 4 years ago

I decided to go for a simpler approach where the command usage is essentially never shown except if the error occurs when parsing the command and its arguments/flags. You will see that the usage will now be shown less often. I figured that even if an error occurs on the client side (e.g. in serialization), it doesn't make much sense to show the usage string unless the error is directly related to how the command was invoked.

I'll leave this issue open for now, until we are sure that the current solution is adequate.