Skip to content

Return cli List error closer to where was returned#4822

Merged
thardeck merged 1 commit into
rancher:mainfrom
0xavi0:return-list-error-early-in-code
Mar 18, 2026
Merged

Return cli List error closer to where was returned#4822
thardeck merged 1 commit into
rancher:mainfrom
0xavi0:return-list-error-early-in-code

Conversation

@0xavi0
Copy link
Copy Markdown
Contributor

@0xavi0 0xavi0 commented Mar 16, 2026

When looking for a different issue I found this code that confused me. In case List returns an error we're still trying to iterate the returned list (that should be empty, we cannot guarantee) This PR returns the error earlier and avoids accessing the list, that in future implementations of List could be returning nil and make this panic.

Additional Information

Checklist

- [ ] I have updated the documentation via a pull request in the
fleet-docs repository.

When looking for a different issue I found this code that confused me.
In case List returns an error we're still trying to iterate the returned list (that should be empty, we cannot guarantee)
This PR returns the error earlier and avoids accessing the list, that in future implementations of List could be returning nil and make this panic.

Signed-off-by: Xavi Garcia <xavi.garcia@suse.com>
@0xavi0 0xavi0 added this to the v2.14.1 milestone Mar 16, 2026
@0xavi0 0xavi0 self-assigned this Mar 16, 2026
@0xavi0 0xavi0 added this to Fleet Mar 16, 2026
@0xavi0 0xavi0 marked this pull request as ready for review March 16, 2026 11:00
@0xavi0 0xavi0 requested a review from a team as a code owner March 16, 2026 11:00
Copilot AI review requested due to automatic review settings March 16, 2026 11:00
Copy link
Copy Markdown
Contributor

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

Adjusts the CLI apply pruning logic to stop processing bundles when the initial bundle list operation fails, preventing iteration over potentially invalid results and aligning behavior with the intended error handling described in the PR.

Changes:

  • Return immediately if c.List(...) fails in pruneBundlesNotFoundInRepo to avoid iterating over bundleList.Items on error.
  • Simplify delete error handling by inlining the c.Delete(...) error check.
  • Return nil explicitly at the end of the prune function instead of propagating a stale err.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread internal/cmd/cli/apply/apply.go
@thardeck thardeck moved this to 👀 In review in Fleet Mar 17, 2026
@thardeck thardeck merged commit a3227f3 into rancher:main Mar 18, 2026
39 of 40 checks passed
@github-project-automation github-project-automation Bot moved this from 👀 In review to ✅ Done in Fleet Mar 18, 2026
@thardeck thardeck modified the milestones: v2.14.1, v2.15.0 Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants