Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Improve Error Messages for Harbor CLI #262

Open
bupd opened this issue Nov 19, 2024 · 4 comments
Open

Improve Error Messages for Harbor CLI #262

bupd opened this issue Nov 19, 2024 · 4 comments

Comments

@bupd
Copy link
Member

bupd commented Nov 19, 2024

The current error messages in the Harbor CLI provide detailed technical information, which is useful for debugging but not ideal for end-users. These error messages are too verbose and can overwhelm or confuse users. We need to modify the error messages to be more user-facing and focused on guiding the user through potential issues.

Expected outcome:

  • error messages should be user faced and not for developers.
  • info messages should just show the info for the users and not the info command.
  • Remove logrus
log "github.com/sirupsen/logrus"

The prefix we see in the front of all error messages is created by this library we are using.
TO remove the prefix, We should either switch completely to fmt or use slog.

@Roaster05
Copy link
Contributor

@bupd These messages can overwhelm or confuse end-users as they often contain details such as system codes, addresses, or stack traces that are not meaningful to them.

Proposed Solution

The aim is to make error messages more user-facing by:

  1. Introducing specific error codes for classification.
  2. Providing clear and human-readable responses.
  3. Removing technical details such as system codes and addresses that are irrelevant for the end-user.

The updated error-handling approach will focus on guiding the user through potential issues rather than exposing internal system details. For example, instead of returning raw error information, we will return concise, actionable messages tailored to specific error types.

Example Implementation

Below is an example of how we can handle errors more effectively:

Current Implementation

if err != nil {
    return err
}

Proposed Implementation

if err != nil {
    // Check if the error is from a specific error type or code
    if apiErr, ok := err.(*project.DeleteProjectBadRequest); ok {
        log.Errorf("Bad request while trying to delete project %s: %v", projectName, apiErr)
        return errors.New("Invalid request to delete project")
    }

    if apiErr, ok := err.(*project.DeleteProjectNotFound); ok {
        log.Errorf("Project %s not found: %v", projectName, apiErr)
        return errors.New("Project not found or already deleted")
    }

    if apiErr, ok := err.(*project.DeleteProjectForbidden); ok {
        log.Errorf("Permission denied for project %s: %v", projectName, apiErr)
        return errors.New("Insufficient permissions to delete project")
    }

    // Generic error handling if the error type is unknown
    log.Errorf("Failed to delete project %s: %v", projectName, err)
    return errors.New("An unexpected error occurred while deleting the project")
}

Expected Outcome

  1. Improved User Experience: Users will see simple, meaningful messages such as:

    • "Invalid request to delete project"
    • "Project not found or already deleted"
    • "Insufficient permissions to delete project"
    • "An unexpected error occurred while deleting the project"
  2. Consistency: Similar error handling will be applied across all CLI commands.

  3. Better Debugging: Technical details (e.g., logs) will still be available for developers but hidden from end-users.

Example of User-Facing Messages

  • Previous:
    ERRO[0000] failed to delete project: [DELETE /projects/{project_name_or_id}][404] deleteProjectNotFound &{Errors:[0xc000228a60]} 
    
  • Updated:
    ERRO[0000] Project not found or already deleted.
    

@bupd
Copy link
Member Author

bupd commented Dec 13, 2024

@Roaster05 Thanks for the suggestions. If you are up for it create a PR.

Also,

We don't need the [0000] in the message. we can keep the message to only text.

ERROR: Project not found or already deleted

The above would be one good example of the error message

@Roaster05
Copy link
Contributor

@Roaster05 Thanks for the suggestions. If you are up for it create a PR.

Also,

We don't need the [0000] in the message. we can keep the message to only text.

ERROR: Project not found or already deleted

The above would be one good example of the error message

Hey @bupd i have made a draft PR #282 for the same could you please have a look at it

@bupd
Copy link
Member Author

bupd commented Dec 29, 2024

log "github.com/sirupsen/logrus"

The prefix we see in the front of all error messages is created by this library we are using.
TO remove the prefix, We should either switch completely to fmt or use slog.

I would prefer using slog.

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

No branches or pull requests

2 participants