civo / cli

Our Command Line Interface (CLI) for interacting with your Civo resources
Apache License 2.0
173 stars 86 forks source link

Return a more helpful error message when an invalid API key is used #312

Open richardlewisjones opened 1 year ago

richardlewisjones commented 1 year ago

When an invalid API key is used, users see this error message:

Please check if you are using the latest version of CLI and retry the command 
If you are still facing issues, please report it on our community slack or open a GitHub issue (https://github.com/civo/cli/issues) 
Error: DatabaseAccountNotFoundError: Failed to find the account within the internal database

The error message does not mention the API key, and a reference to 'the internal database' is not helpful. A better message would be something like API key is invalid. See https://www.civo.com/api#authentication

dipu989 commented 2 weeks ago

Hey @alejandrojnm Do you think issue is still a valid one? If yes, I can raise a PR to fix this. I also think that internal db error messages should not be exposed to clients, let me know what do you think?

alejandrojnm commented 2 weeks ago

Hi thanks for asking, I'm not sure we can ask to @uzaxirr about this

dipu989 commented 2 weeks ago

Sure thing! @uzaxirr Let us know your thoughts on this?

uzaxirr commented 2 weeks ago

hey @dipu989 on the latest version of CLI, if you use a wrong API key you will get.

Screenshot 2024-09-24 at 12 48 22 AM

the API will return the below response

{
    "code": "database_account_not_found",
    "reason": "Failed to find the account within the internal database"
}

Please let me know your approach and raise a PR

dipu989 commented 2 weeks ago

@uzaxirr How about error like - WrongAPIKeyError: API Key is invalid.

I am not sure if WrongAPIKeyError will be a valid error type in the code base but probably we can add this type of Error in the enum? What do you think? This actually gives the user, a proper reason for failure without exposing any internal implementation detail.

Forbidden or Permission Denied doesn't make a lot of sense here, as this ain't a typical API response 🤔 Let me know if you think otherwise. Thanks.

uzaxirr commented 2 weeks ago

@dipu989 please raise a PR