Skip to content

Conversation

@bhanurp
Copy link
Collaborator

@bhanurp bhanurp commented Jan 8, 2026

  • All tests passed. If this feature is not already covered by the tests, I added new tests.
  • All static analysis checks passed.
  • Appropriate label is added to auto generate release notes.
  • I used gofmt for formatting the code before submitting the pull request.
  • PR description is clear and concise, and it includes the proposed solution/fix.

Description

This PR fixes a bug where DeleteCommand.Result().SuccessCount() and Result().FailCount() return 0 when 404 errors (or other errors) occur during deletion, even though some files may have been successfully deleted.

When using DeleteCommand and encountering errors (e.g., 404 errors when items are already deleted by another process), both SuccessCount() and FailCount() returned 0

solution

Changed to capture the deletion error separately and return actual counts regardless of whether an error occurred.

result.SetFailCount(failedCount)
result.SetSuccessCount(successCount)
result.SetFailCount(failedCount)
err = deleteErr
Copy link
Collaborator

@itsmeleela itsmeleela Jan 12, 2026

Choose a reason for hiding this comment

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

@bhanurp I didn't understand the need of renaming err to deleteErr and then assigning deleteErr to err?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

to avoid variable shadowing of err at delete.go#L42
even below should work as well

var successCount, failedCount int successCount, failedCount, err = dc.DeleteFiles(reader) // Uses existing 'err' result := dc.Result() result.SetSuccessCount(successCount) result.SetFailCount(failedCount)

moving successCount, failedCount as variable declaration instead of shorthand :=

@github-actions
Copy link
Contributor

👍 Frogbot scanned this pull request and did not find any new security issues.


@bhanurp bhanurp merged commit 12090b4 into jfrog:main Jan 13, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants