Skip to content

fix(replication/retention): propagate prompt and view errors to prevent abrupt CLI crashes#932

Open
gcharpe1604 wants to merge 5 commits into
goharbor:mainfrom
gcharpe1604:fix/replication-prompt-error-handling
Open

fix(replication/retention): propagate prompt and view errors to prevent abrupt CLI crashes#932
gcharpe1604 wants to merge 5 commits into
goharbor:mainfrom
gcharpe1604:fix/replication-prompt-error-handling

Conversation

@gcharpe1604
Copy link
Copy Markdown
Contributor

@gcharpe1604 gcharpe1604 commented May 15, 2026

Description

This PR fixes replication interactive prompt helpers and views that terminate the CLI abruptly via log.Fatal() or os.Exit(1) during API failures or invalid selections.

Errors are now propagated through Cobra's command lifecycle (RunE) instead of terminating the process directly, resulting in more consistent and automation-friendly CLI behavior.

Fixes #931

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactor (non-breaking change which improves code structure)
  • Documentation update
  • Chore / maintenance

Key Changes in PR #932

1. Removal of Abrupt CLI Crashes (log.Fatal(), os.Exit())

  • Prompt Helpers: Removed all instances of log.Fatal() in prompt helper functions in pkg/prompt/prompt.go when replication APIs fail or return empty lists.
  • View Tier: Replaced direct os.Exit(1) calls inside selection views (such as ReplicationExecutionList and ReplicationTasksList) with clean error propagation via channels.
  • Cobra Integration: Errors are returned up to Cobra's command layer, allowing it to format error messages and return appropriate exit codes.

2. Error & Cancel/Abort Propagation

  • Added an errChan chan<- error channel to the asynchronous view functions to propagate errors back to the caller prompts.
  • If a selection is cancelled/aborted (yielding empty selection choice), an error is sent to the channel instead of calling os.Exit(1).

How to Test

  1. Verify Cancel/Abort Graceful Exit:
    Run an interactive command like harbor replication start without arguments, cancel the prompt selection, and verify the command exits gracefully with user aborted selection instead of calling log.Fatal(), calling os.Exit(1), or silently executing on default values.

  2. Replication Commands:
    Verify harbor replication start, harbor replication stop, and harbor replication log function as expected and terminate cleanly if aborted.

Refactored the replication prompt handlers (GetReplicationPolicyFromUser, GetReplicationExecutionIDFromUser, GetReplicationTaskIDFromUser) to return (int64, error) instead of using log.Fatal(). Updated all replication-related commands to handle these errors and bubble them up via Cobra's RunE, ensuring consistent, automation-friendly exit codes instead of abrupt process termination.

Signed-off-by: Govind Charpe <govind.charpe16@gmail.com>
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 0% with 124 lines in your changes missing coverage. Please review.
✅ Project coverage is 9.30%. Comparing base (60ad0bd) to head (b857026).
⚠️ Report is 183 commits behind head on main.

Files with missing lines Patch % Lines
pkg/prompt/prompt.go 0.00% 41 Missing ⚠️
cmd/harbor/root/replication/logs.go 0.00% 13 Missing ⚠️
pkg/views/replication/execution/select/view.go 0.00% 12 Missing ⚠️
pkg/views/replication/task/select/view.go 0.00% 12 Missing ⚠️
cmd/harbor/root/replication/stop.go 0.00% 10 Missing ⚠️
pkg/views/replication/policies/select/view.go 0.00% 10 Missing ⚠️
cmd/harbor/root/replication/executions/view.go 0.00% 6 Missing ⚠️
cmd/harbor/root/replication/executions/list.go 0.00% 4 Missing ⚠️
cmd/harbor/root/replication/policies/delete.go 0.00% 4 Missing ⚠️
cmd/harbor/root/replication/policies/update.go 0.00% 4 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff            @@
##             main    #932      +/-   ##
=========================================
- Coverage   10.99%   9.30%   -1.69%     
=========================================
  Files         173     314     +141     
  Lines        8671   15823    +7152     
=========================================
+ Hits          953    1473     +520     
- Misses       7612   14215    +6603     
- Partials      106     135      +29     

☔ 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.

@gcharpe1604 gcharpe1604 changed the title fix(replication): propagate interactive prompt errors to prevent abrupt CLI crashes fix(replication/retention): propagate prompt and view errors to prevent abrupt CLI crashes May 23, 2026
@gcharpe1604
Copy link
Copy Markdown
Contributor Author

gcharpe1604 commented May 23, 2026

I’ve pushed a larger round of updates after additional verification of the replication and tag retention flows.

The changes now address several related issues around:

  • graceful cancellation handling (ctrl+c / esc)
  • error propagation instead of abrupt exits
  • panic prevention for failed API fetches
  • safer retention rule deletion behavior
  • proper support for --project-id
  • simplification of the replication prompt flow by removing unnecessary goroutine/channel handling

One important fix here was preventing cancelled selection prompts from unintentionally proceeding with default rule indices during tag retention deletion flows.

The updated implementation now consistently bubbles errors/cancellations back through Cobra instead of terminating inside prompt/view helpers.

@qcserestipy @NucleoFusion

Would appreciate your feedback/review whenever you have time. Thanks!

@gcharpe1604 gcharpe1604 force-pushed the fix/replication-prompt-error-handling branch from 2d4f05d to bc31de4 Compare May 23, 2026 06:37
@qcserestipy
Copy link
Copy Markdown
Collaborator

@gcharpe1604 Thank you for your contribution. However, this PR addresses much more changes than the connected issue. Please reduce this PR to the mentioned changes in the issue. For other changes open a new issue and related PRs.

@qcserestipy qcserestipy added Changes Requesed feedback that must be addressed before merging. slop This PR is most likely AI slop labels May 25, 2026
@gcharpe1604
Copy link
Copy Markdown
Contributor Author

gcharpe1604 commented May 30, 2026

@qcserestipy
Thanks for the review and feedback, and apologies for the delay in following up.

I had university end-semester exams over the last week and wasn't able to spend time on the PR. I'm reviewing the feedback now and re-evaluating the scope based on your comments.

I'll reduce the PR so it stays focused on the issue being addressed. The broader prompt/view improvements and related changes can be proposed separately where appropriate to keep the review scope manageable.

…and bugs

Signed-off-by: Govind Charpe <govind.charpe16@gmail.com>
Signed-off-by: Govind Charpe <govind.charpe16@gmail.com>
@gcharpe1604 gcharpe1604 force-pushed the fix/replication-prompt-error-handling branch from bc31de4 to 0ca5b5c Compare May 30, 2026 17:57
…vent abrupt CLI crashes

Signed-off-by: Govind Charpe <govind.charpe16@gmail.com>
…mpt-error-handling

Signed-off-by: Govind Charpe <govind.charpe16@gmail.com>

# Conflicts:
#	pkg/prompt/prompt.go
@gcharpe1604
Copy link
Copy Markdown
Contributor Author

Hi @qcserestipy,

I've updated the PR based on the review feedback and reduced the scope to focus on the issue in #931.

The current changes are limited to removing the direct log.Fatal() / os.Exit(1) paths in the replication prompt/view flow and propagating errors back through the command layer instead of terminating the process.

I've also removed the unrelated changes from this branch to keep the review focused.

Could you take another look when you get a chance?

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changes Requesed feedback that must be addressed before merging. slop This PR is most likely AI slop

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: Interactive prompt helpers and views crash abruptly or panic on API errors and user aborts

2 participants