fix(runcommand): output correct invalid creds error

[MarinX]

closes #392

The issue:
Invalid creds returns error at runcommand.go only if resolver is used.
If the resolver is not used (ex., server list), it will go into the worker queue and render an error with error output.
This is why some commands return expected output and some just an error message.

The fix:
output the error and stop progress log to print error message as other commands do without the resolver.

[kangasta]

Thank you for the pull request!

I would prefer the fix to be other way around, i.e. show the same error message that commands that use resolver (e.g. server show) output also for commands that do not use resolver (e.g. server list). That way also commands that do not use resolver would exit with 103 exit code. Maybe this could be done by validating credentials (for example by calling GetAccount) where single and multiple argument commands call resolver 🤔

[kangasta]

Created #411 for this as I think #392 is more about the content of the error message than its formatting.

[MarinX]

Appreciate the response and suggested fix.
Looks like it's a better approach than just erroring out and returning the wrong status code.

Let me work on the PR and get back with the changes.

In the meantime, if someone else wants to work on it, just let me know so we don't do double work.
A little bit slow this week as other things got in the way (sorry) :)

[ghost]

🎉 Snyk checks have passed. No issues have been found so far.

✅ security/snyk check is complete. No issues have been found. (View Details)

✅ license/snyk check is complete. No issues have been found. (View Details)

[MarinX]

Seems like there are multiple paths to fixing this issue.

Option 1.

By adding a resolver to every service (I added just for an account), the resolver can be called and exit with the correct error code.
Since there are a lot of services, this PR can potentially grow big with changes, as every service needs a resolver.
For the services without a resolver (sample in account/permission/list), should we default to CachingAccount resolver?

Option 2

Most straightaway solution.
If the resolve mode is none, execute a GetAccount() and return err.
While it is a "single line" fix, tests need to be updated and can cause a debugging headache because of the extra "hidden" call for a GetAccount.

Can I get your input on this @kangasta ?

[kangasta]

Thank you for further looking into this! 🙌

I think second option would be better as it is more simple. For the tests there could be a helper function that both initializes the mock service and sets up mocking for the GetAccount calls, but this is also something that can be added later. We already use similar approach in our Terraform provider, where we call the GetAccount right after configuring the API client to validate the credentials.

Adding resolver to list commands would also feel a bit off since these commands do not take arguments so there are no arguments to resolve 🤔

[MarinX]

updated PR to call GetAccount if the resolve mode is none

➜  upctl git:(fix/execute-error) ✗ ./upctl account show
Error: invalid user credentials, authentication failed using the given username and password
➜  upctl git:(fix/execute-error) ✗ echo $?
103
➜  upctl git:(fix/execute-error) ✗ ./upctl server list
Error: invalid user credentials, authentication failed using the given username and password
➜  upctl git:(fix/execute-error) ✗ echo $?
103
➜  upctl git:(fix/execute-error) ✗ ./upctl version
  
  Version:      dev-9d0ec785-dirty 
  Build date:   unknown            
  Built with:   go1.24.3           
  System:       linux              
  Architecture: amd64