Skip to content

Refactor: Centralized Harbor Error Parsing for all commands #407

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

Closed
wants to merge 4 commits into from

Conversation

rizul2108
Copy link
Collaborator

@rizul2108 rizul2108 commented Apr 15, 2025

Overview

This PR introduces a cleaner, more reliable, and consistent approach to error handling across the Harbor CLI. It standardizes parsing and reporting API errors, making the CLI more user-friendly and developer-maintainable.

Key Changes

  • Added ParseHarborError utility

    • Centralized error parsing logic using reflection to extract the Payload from Harbor API responses.
    • Produces clean and readable error messages for users (e.g.,
      • "user not found field"
      • "permission denied field")
  • Replaced Run with RunE across commands

    • Enables direct error returns from command handlers.
    • Ensures all errors are parsed through the new ParseHarborError function for consistent logging.
  • Code cleanup and simplification

    • Removed redundant lines and unused logs.
    • Improved flow readability and reduced clutter in error blocks.

Motivation

  • Prior CLI behavior often exposed raw Go structs or verbose pointers in error logs.
  • This refactor ensures a consistent and user-friendly experience across all commands.
  • Simplifies future debugging and maintenance.

Error Output Before vs After

Before:

Error: login failed: login failed, please check your credentials: [GET /users/current][401] getCurrentUserInfoUnauthorized  &{Errors:[0x14000158240]}
rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli user delete 1123
ERRO failed to delete user: [DELETE /users/{user_id}][404] deleteUserNotFound  &{Errors:[0xc00045a1a0]} 

After:

./bin/harbor-cli login demo.goharbor.io -u harbor-cli -p Harbor1234
Error: login failed: login failed, please check your credentials and server address, one of them is incorrect
rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli user delete 1123
ERRO failed to get user id for '1123': User 1123 not found 

Related Issues

Fix #393
Fix #316
Fix #253
Fix #262

Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>

lint fixes

Signed-off-by: Rizul Gupta <[email protected]>

lint fixes

Signed-off-by: Rizul Gupta <[email protected]>
@rizul2108 rizul2108 force-pushed the modify-error-login branch from e68efa3 to f923743 Compare April 15, 2025 05:06
@rizul2108 rizul2108 requested review from bupd and Copilot April 15, 2025 05:16
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

cmd/harbor/root/health.go:32

  • [nitpick] There is an inconsistency in using %s in the error formatting here while other files use %v. Standardizing the formatter (e.g., using %v) could improve consistency across the codebase.
return fmt.Errorf("%s", utils.ParseHarborError(err))

Signed-off-by: Rizul Gupta <[email protected]>
Copy link
Collaborator

@bupd bupd left a comment

Choose a reason for hiding this comment

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

my suggestion would be to utilise logrus for logging steps when given -v flag. and fmt for errors.

@rizul2108 rizul2108 closed this Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants