Skip to content

fix: return errors instead of panicking in list command#865

Merged
bupd merged 1 commit into
goharbor:mainfrom
cotishq:fix/project-robot-list-nil-panic
May 6, 2026
Merged

fix: return errors instead of panicking in list command#865
bupd merged 1 commit into
goharbor:mainfrom
cotishq:fix/project-robot-list-nil-panic

Conversation

@cotishq

@cotishq cotishq commented May 3, 2026

Copy link
Copy Markdown
Contributor

Description

This PR fixes a panic in the harbor project robot list command where errors from API calls were logged but execution continued, leading to a nil pointer dereference.

Specifically

  • api.GetProject() errors were logged but project.Payload was still accessed
  • api.ListRobot() errors were logged but robots.Payload was still used

Fixes #862

Type of Change

Please select the relevant type.

  • Bug fix
  • New feature
  • Refactor
  • Documentation update
  • Chore / maintenance

Changes

  • Return error on api.GetProject() failure instead of continuing execution
  • Added nil check for project.Payload before accessing ProjectID
  • Return error on api.ListRobot() failure instead of continuing execution
  • Ensured CLI exits gracefully with a proper error message instead of panicking

Verification

Command:

env XDG_CONFIG_HOME=/tmp/harbor-cli-verify/config XDG_DATA_HOME=/tmp/harbor-cli-verify/data ./bin/harbor-cli project robot list does-not-exist

observed(before fix):

Screenshot From 2026-05-03 11-20-32

(After fix):

Screenshot From 2026-05-04 00-57-17

@cotishq
fix: return errors instead of panicking in list command
3e13e37 
Signed-off-by: cotishq <tanishqp101204@gmail.com>
NucleoFusion
NucleoFusion approved these changes May 4, 2026
View reviewed changes

@NucleoFusion NucleoFusion left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm! thanks for the contribution

@qcserestipy qcserestipy self-requested a review May 5, 2026 14:39
qcserestipy
qcserestipy approved these changes May 5, 2026
View reviewed changes

@qcserestipy qcserestipy left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM! Very nice example of well executed PR. Thank you

bupd
bupd approved these changes May 6, 2026
View reviewed changes

@bupd bupd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/lgtm

@bupd bupd merged commit 8338315 into goharbor:main May 6, 2026
3 of 4 checks passed
@codecov

codecov Bot commented May 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.00%. Comparing base (60ad0bd) to head (3e13e37).
⚠️ Report is 150 commits behind head on main.

Files with missing lines Patch % Lines
cmd/harbor/root/project/robot/list.go 0.00% 4 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##             main    #865      +/-   ##
=========================================
- Coverage   10.99%   9.00%   -1.99%     
=========================================
  Files         173     272      +99     
  Lines        8671   13443    +4772     
=========================================
+ Hits          953    1211     +258     
- Misses       7612   12117    +4505     
- Partials      106     115       +9

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@cotishq cotishq deleted the fix/project-robot-list-nil-panic branch May 6, 2026 07:11
@BrewTestBot BrewTestBot mentioned this pull request May 14, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qcserestipy qcserestipy qcserestipy approved these changes

@bupd bupd bupd approved these changes

+1 more reviewer

@NucleoFusion NucleoFusion NucleoFusion approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[bug]: project robot list can panic instead of returning an error

4 participants

@cotishq @qcserestipy @bupd @NucleoFusion