Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Send correct message from civogo sdk when api key is invalid #212

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dipu989
Copy link

@dipu989 dipu989 commented Sep 24, 2024

Issue link in cli repo

Description
When Civo users attempt to use the CLI with an incorrect API key, the error message displayed in the terminal is both unclear and exposes internal details, which is not user-friendly. This PR addresses that issue by providing a more meaningful and helpful error message.

This is more of a workaround PR.
Currently, Civo CLI relies on the civogo SDK, which communicates with the Civo backend. When an invalid API key is used, the backend responds with an error message like:

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

This message is then passed along and presented to the user as: (on terminal)

Error: DatabaseAccountNotFoundError: Failed to find the account within the internal database

Changes in this PR and suggestions -
The error message database_account_not_found will now be translated to a more user-friendly message:
InvalidAPIKeyError: "API Key is invalid, please visit https://dashboard.civo.com/security to generate a new one"`

Suggestions:
While this specific message "database_account_not_found" for an invalid api key could have some historical context, it would be better if the backend error were more descriptive, such as using codes like invalid_api_key or wrong_api_key to reflect the actual issue.
For now, this PR focuses on improving the user-facing message to provide clarity and guidance on resolving the issue.

Let me know in case you guys want to change the incoming message from the backend, I will refactor and accommodate for those changes in subsequent PRs.

For now, we should be in a good shape to unblock the issue .

@dipu989
Copy link
Author

dipu989 commented Sep 24, 2024

@uzaxirr @vishalanarase Please review this PR.
For context - Discussion
Have tested for the UTs with go test .

Comment on lines +37 to +38
// Invalid API Key Error
InvalidAPIKeyError = constError("InvalidAPIKeyError")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uzaxirr don't you think that this could be handled directly from the backend, improving the returning error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fulviodenza That would be the best case and won't require handling from cli.
@uzaxirr Let me what what do you think about this?
Let me know in case you guys want to send a specific error from backend, I can accommodate for that change in this PR just in case.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants