Skip to content

Refactor: Make Error logging more user friendly for project commands #424

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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

rizul2108
Copy link
Collaborator

@rizul2108 rizul2108 commented Apr 17, 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 projects
  • 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.
  • Some Errors are not documented in some of the API requests by harbor so they gave response like below

Error: failed to get project logs: response status code does not match any response statuses defined for this endpoint in the swagger spec (status 404): {}

image

So added a new func CheckProject which calls API headProject to check if project exists

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 project sub-commands.
  • Simplifies future debugging and maintenance.

Error Output Before vs After

Before:

rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli project delete debugcod
Error: failed to delete some projects: Error: [DELETE /projects/{project_name_or_id}][404] deleteProjectNotFound  &{Errors:[0xc00037e080]}
rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli project view debugcod
ERRO failed to get project: response status code does not match any response statuses defined for this endpoint in the swagger spec (status 404): {} 

After:

rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli project delete debugcod
Error: failed to delete some projects: project debugcod not found
rizul@rizu:~/OSS/harbor-cli$ ./bin/harbor-cli project view debugcod
Error: Project debugcod does not exist

Related Issues

Fix #316
Fix #253
Fix #262
Fix #431
Fix #436
Fix #440

@rizul2108 rizul2108 changed the title Refactor: Harbor Error Parsing and Make Error logging more user friendly for project commands Refactor: Make Error logging more user friendly for project commands Apr 17, 2025
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
@rizul2108
Copy link
Collaborator Author

@bupd, are the logs okay when using the verbose flag, or should I reduce them?

@rizul2108 rizul2108 requested review from Copilot and bupd April 17, 2025 07:12
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.

Pull Request Overview

This PR refactors the Harbor CLI error handling for project-related commands to make error logging more user friendly and consistent. Key changes include:

  • Introducing a centralized error parsing utility (ParseHarborError) and a new CheckProject function.
  • Replacing Run with RunE in commands to allow direct error returns and streamlined error handling.
  • Cleaning up code structure and logging for project operations such as view, list, delete, and create.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
pkg/utils/error.go Added ParseHarborError to centralize error parsing logic
pkg/api/project_handler.go Updated DeleteProject parameter (using useProjectName)
cmd/harbor/root/project/view.go Replaced Run with RunE and integrated CheckProject for existence check
cmd/harbor/root/project/search.go Updated error handling in project search command
cmd/harbor/root/project/logs.go Enhanced existence check and improved debug logging in logs command
cmd/harbor/root/project/list.go Improved pagination logging and error handling in project listing
cmd/harbor/root/project/delete.go Refined delete command with concurrent deletion and detailed reporting
cmd/harbor/root/project/create.go Streamlined project creation with interactive prompt and flag handling
cmd/harbor/root/cmd.go Updated logger configuration to respect verbose mode settings
Comments suppressed due to low confidence (1)

cmd/harbor/root/project/create.go:35

  • [nitpick] Consider using a lower-case variable name (projectName) for local variables to follow Go naming conventions.
var ProjectName string

rizul2108 and others added 2 commits April 17, 2025 12:45
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
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.

@rizul2108 great job on the verbose logging.

But yet we recieve multiple error messages
instead of having multiple error messages can we have a single user frienldy message with a single specific reason.

Instead of

❯ ./bin/harbor-dev project list
ERRO Error fetching project page                   error="current-credential-name is not set in config file"
Error: failed to get projects list: Error: current-credential-name is not set in config file
~/s/code/OSS/harbor-cli/prss fix-project-error
❯ ./bin/harbor-dev project list -v
DEBU[2025-04-22T05:18:02+05:30] Starting project list command
DEBU[2025-04-22T05:18:02+05:30] Using list all projects function
DEBU[2025-04-22T05:18:02+05:30] Fetching projects...
DEBU[2025-04-22T05:18:02+05:30] Page size is 0, will fetch all pages
DEBU[2025-04-22T05:18:02+05:30] Fetching next page of projects                page=1 page_size=100
ERRO[2025-04-22T05:18:02+05:30] Error fetching project page                   error="current-credential-name is not set in config file"
Error: failed to get projects list: Error: current-credential-name is not set in config file

We should have

Error: credentials not found, please login

error messages should be simple and concise

rizul2108 and others added 2 commits April 22, 2025 10:00
Co-authored-by: Prasanth Baskar <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
@Vad1mo Vad1mo requested a review from Copilot April 22, 2025 13:52
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.

Pull Request Overview

This PR refactors error handling for Harbor CLI project commands to provide clearer, consistent, and more user‐friendly error messages.

  • Centralized error parsing via the new ParseHarborError utility
  • Updated command implementations to use RunE and proper error returns
  • Enhanced logging and refined project existence checks and deletion concurrency

Reviewed Changes

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

Show a summary per file
File Description
pkg/views/project/create/view.go Refactored to return errors instead of calling log.Fatal
pkg/utils/error.go Introduced ParseHarborError for centralized error message parsing
pkg/utils/client.go Updated error message with a login hint
pkg/api/project_handler.go Modified flag handling for project deletion
cmd/harbor/root/project/view.go Refactored project view command with existence check and error returns
cmd/harbor/root/project/search.go Improved error handling in search command
cmd/harbor/root/project/logs.go Updated logs command with enhanced debug logging and error returns
cmd/harbor/root/project/list.go Added pagination debug logs and refined project listing
cmd/harbor/root/project/delete.go Enabled batch deletion with concurrency and detailed outcome reporting
cmd/harbor/root/project/create.go Supported both flag-based and interactive project creation
cmd/harbor/root/cmd.go Streamlined logrus formatter initialization
Comments suppressed due to low confidence (1)

cmd/harbor/root/project/logs.go:75

  • The symbol 'auditLog' is used but not defined or imported; ensure the correct package name or import is provided to resolve this error.
auditLog.LogsProject(resp.Payload)

@rizul2108 rizul2108 requested a review from Copilot April 23, 2025 07:29
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.

Pull Request Overview

This PR refactors error logging and handling across the Harbor CLI to improve user friendliness and consistency. Key changes include introducing a centralized error parser (ParseHarborError), replacing Run with RunE in multiple commands, and enhancing prompt functions to return errors.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

File Description
pkg/prompt/prompt.go Updated GetProjectNameFromUser signature to return an error alongside the value.
pkg/api/project_handler.go Adjusted parameters for DeleteProject and added CheckProject for project checks.
cmd/harbor/root/* (various commands) Replaced Run with RunE and improved prompt error handling and logging.

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

lint fix

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

rizul2108 commented Apr 24, 2025

@bupd this PR is ready for review

@rizul2108 rizul2108 requested a review from Copilot April 24, 2025 08:55
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.

Pull Request Overview

This PR refactors error handling in the Harbor CLI to improve user output and consistency. Key changes include centralizing error parsing via ParseHarborError, transitioning commands to use RunE for direct error returns, and adding a new CheckProject function to validate project existence before further processing.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 2 comments.

File Description
pkg/prompt/prompt.go Updated GetProjectNameFromUser to return errors along with the name.
pkg/api/project_handler.go Adjusted project deletion to use a flag inversion for resource naming.
cmd/harbor/root/* Refactored various commands to adopt RunE and standardized error logging and handling.

rizul2108 and others added 3 commits April 24, 2025 14:27
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
@rizul2108 rizul2108 requested a review from Copilot April 28, 2025 10:02
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 encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@rizul2108
Copy link
Collaborator Author

@bupd ready for review

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