Infisical / infisical

♾ Infisical is the open-source secret management platform: Sync secrets across your team/infrastructure, prevent secret leaks, and manage internal PKI
https://infisical.com
Other
14.94k stars 839 forks source link

Improve CLI error handling #41

Open maidul98 opened 1 year ago

maidul98 commented 1 year ago

The current error handeling is not comprehensive as there is no way to add a equivalent friendly message from the cause of an error.

For example, here in the login command we check if the user is already logged in. In the method to check if they are logged in, we just retrun the error as is shown https://github.com/Infisical/infisical/blob/22e7137e742a5d2f87aca3a48d89be33f1903ec1/cli/packages/util/credentials.go#L87

This is problematic because at the command level package, we have to guess exactly why the login check failed. It would be much better, if we can add a helpful message from the login check method so that we do not have to guess the exact cause at command package level.

I propose that we add a new Error type like so

type Error struct {
    Err             error
    DebugMessage    string
    FriendlyMessage string
}

This will allow us to add a FriendlyMessage for every error, allowing us to print the exact message we want to show to the end user for any error message. Please ask questions if you need more clarification.

quinton11 commented 1 year ago

If this is available I'll take it

maidul98 commented 1 year ago

The error handling has been improved slightly since. One other thing that will be helpful for users is to see the URLs of the failed calls. This is because sometimes the user doesn't know if their CLI is pointing to the Infisical cloud of self hosted. Did you have other areas of improvement in mind?

quinton11 commented 1 year ago

Well, the only other area I had in mind improvement wise is the uniformness wrt error handling across the packages in the cli. Some sections like the line you linked above just return the error, others use fmt.Errorf. Perhaps for readability sake that could be addressed. Maybe with all using your suggested solution. What do you think?

vishnusharma10 commented 1 month ago

Hi @maidul98

I noticed that this issue regarding improving CLI error handling hasn't been fully addressed yet. I'd like to take on this task and contribute to the project. Could you please provide an update on the current status? Additionally, are there any specific guidelines or areas you would like me to focus on while working on this improvement?

vishnusharma10 commented 1 month ago

Hi @maidul98, any updates on this?