Skip to content

feat: Add continueOnError parameter to Folder.Clean()#1716

Merged
thomhurst merged 1 commit into
mainfrom
fix/folder-clean-error-handling
Jan 1, 2026
Merged

feat: Add continueOnError parameter to Folder.Clean()#1716
thomhurst merged 1 commit into
mainfrom
fix/folder-clean-error-handling

Conversation

@thomhurst
Copy link
Copy Markdown
Owner

Summary

  • Adds new Clean(bool removeReadOnlyAttribute, bool continueOnError) overload
  • When continueOnError is true, continues deleting remaining items if some fail
  • Logs warnings for each failed deletion with full path and exception message
  • Aggregates all errors and throws AggregateException at the end
  • Maintains backward compatibility: existing overloads default to continueOnError: false

Use Cases

  • Build pipelines cleaning folders with files locked by other processes
  • Cleaning folders with a mix of deletable and problematic files
  • Better diagnostics when clean operations partially fail

Example

// Old behavior - throws on first error
folder.Clean();

// New behavior - delete what we can, throw aggregate at end
folder.Clean(removeReadOnlyAttribute: true, continueOnError: true);

Test plan

  • Build succeeds
  • CI pipeline passes
  • Existing Folder tests pass

Fixes #1552

🤖 Generated with Claude Code

Adds robust error handling to Folder.Clean() with a new overload:
- continueOnError: When true, continues deleting remaining items even
  if some deletions fail (e.g., files in use by other processes)
- Failed deletions are logged with warnings
- All errors are aggregated and thrown as AggregateException at the end
- Default behavior (continueOnError: false) throws immediately on first error

This helps build pipelines handle locked files gracefully and provides
better diagnostics when cleaning folders with problematic files.

Fixes #1552

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@thomhurst
Copy link
Copy Markdown
Owner Author

REVIEW: PR #1716 - Add continueOnError to Folder.Clean()

SUMMARY:
Adds a continueOnError parameter to enable partial folder cleaning with aggregated error reporting.

CRITICAL ISSUES:
None found - approved

SUGGESTIONS:

  1. Memory Allocation Optimization
    The errors list is allocated unconditionally even when continueOnError is false. Consider lazy initialization for better performance in the common case where no errors occur.

  2. Test Coverage
    No new tests added for the continueOnError parameter. Consider adding tests for:

  • Behavior when continueOnError is true and some deletions fail
  • Verification that AggregateException contains all expected errors
  • Behavior when continueOnError is false still throws on first error

VERDICT:
Approved - The implementation is solid with proper backward compatibility, appropriate error handling, and good documentation. Suggestions are optional improvements.

Copy link
Copy Markdown

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 adds a continueOnError parameter to the Folder.Clean() method, enabling pipelines to continue deleting files even when some deletions fail. When enabled, all errors are aggregated and thrown as a single AggregateException at the end, with individual failures logged as warnings. The implementation maintains full backward compatibility through method overloads that default to the original fail-fast behavior.

Key Changes:

  • Added new Clean(bool removeReadOnlyAttribute, bool continueOnError) overload with error aggregation logic
  • Updated existing overloads to delegate to the new method with continueOnError: false default
  • Added try-catch blocks around directory and file deletion with exception filtering based on continueOnError flag

/// Thrown when <paramref name="continueOnError"/> is true and one or more deletions failed.
/// Contains all individual exceptions encountered during the operation.
/// </exception>
public void Clean(bool removeReadOnlyAttribute, bool continueOnError)
Copy link

Copilot AI Jan 1, 2026

Choose a reason for hiding this comment

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

The new continueOnError parameter lacks test coverage. Consider adding tests that verify:

  1. When continueOnError: true, the method continues deletion after encountering an error (e.g., by creating a file locked by another process)
  2. When continueOnError: true, failed deletions result in an AggregateException containing all the individual errors
  3. When continueOnError: false (default), the method stops and throws on the first error as expected

The existing test file test/ModularPipelines.UnitTests/FolderTests.cs has tests for CleanFiles() and CleanFolders(), making it an appropriate location for these new test cases.

Copilot uses AI. Check for mistakes.
@thomhurst thomhurst merged commit f05c924 into main Jan 1, 2026
14 of 18 checks passed
@thomhurst thomhurst deleted the fix/folder-clean-error-handling branch January 1, 2026 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Code smell: Folder.Clean() doesn't handle read-only or in-use files

2 participants