fix unloading

[MatteoDelOmbra]

Review Checklist

• Task version updated (x.x.0)
• CHANGELOG.md updated
• Solution builds
• Warnings resolved (if possible)
• Typos resolved
• Tests cover new code
• Description how to run tests locally added to README.md (if needed)
• All tests pass locally

Summary by CodeRabbit

• Bug Fixes

  • Fixed plugin unloading to ensure clean detachment and prevent lingering resources.

• Improvements

  • More reliable, fully asynchronous query execution and CSV export with async streaming and disposal.
  • Safer parameter handling and null/value normalization to reduce runtime errors.

• Chores

  • Bumped package to 2.3.0 and added changelog entry; minor enum/formatting tidy-ups.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Updated project to version 2.3.0 and added changelog entry. Rewrote ExecuteQueryToFile to use async resource management, refactored CsvFileWriter to IAsyncDisposable with async CSV streaming and renamed SaveQueryToCSV → SaveQueryToCsv; minor formatting changes in enum files.

Changes

Cohort / File(s)	Summary
Version & Changelog Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj, Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md	Bumped project version to 2.3.0 and added changelog entry (2026-01-30) noting a fix for Frends process cleanup and assembly unloading.
Execute flow & async disposal Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs, Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Definitions/CsvFileWriter.cs	Replaced synchronous disposals with async disposal (await using and ConfigureAwait(false)); simplified parameter handling; added CSV return path handling; CsvFileWriter now implements IAsyncDisposable, adds DisposeAsync, renames and rewrites SaveQueryToCSV → SaveQueryToCsv for async streaming and flush.
Enums (formatting) Frends.MicrosoftSQL.ExecuteQueryToFile/.../Enums/FileEncoding.cs, .../Enums/ReturnFormat.cs	Whitespace/formatting changes only; no semantic/API changes.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant ExecuteQueryToFile
    participant SQLServer
    participant CsvFileWriter
    participant FileSystem

    Caller ->> ExecuteQueryToFile: Invoke ExecuteQueryToFile(params, options, ct)
    ExecuteQueryToFile ->> SQLServer: OpenAsync(connection) 
    ExecuteQueryToFile ->> SQLServer: ExecuteReaderAsync(command)
    SQLServer -->> ExecuteQueryToFile: DbDataReader (streaming rows)
    ExecuteQueryToFile ->> CsvFileWriter: Instantiate(writer with path, options)
    ExecuteQueryToFile ->> CsvFileWriter: SaveQueryToCsv(cancellationToken)
    CsvFileWriter ->> CsvFileWriter: Async-write headers and rows
    CsvFileWriter ->> FileSystem: FlushAsync() & close stream
    CsvFileWriter -->> ExecuteQueryToFile: Result (path, metadata)
    ExecuteQueryToFile -->> Caller: Return Result

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• ExecuteQueryToFile - add managed identity authentication #51 — Modifies the same core files (ExecuteQueryToFile and CsvFileWriter); overlaps in CSV writer and execution flow changes.
• Fspes 53 #72 — Related edits around async behavior and CsvFileWriter; touches ConfigureAwait and CSV writer refactor.

Suggested reviewers

• ttossavainen
• MichalFrends1

Poem

🐰 I hopped through async fields at night,
Swapped old usings for await's soft light,
I wrote the rows and flushed with care,
Closed every stream, left none to spare,
Now CSVs sleep tidy and tight. ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'fix unloading' is vague and generic, using non-descriptive terms that don't clearly convey what specific issue is being fixed or what 'unloading' refers to in context.	Revise the title to be more specific and descriptive. Consider something like 'Fix assembly unloading and process cleanup in ExecuteQueryToFile' or 'Improve async resource disposal for proper cleanup'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fspw-669

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs`:
- Line 38: The Result class for ExecuteQueryToFile is missing the required
Success bool and Error object; update the ExecuteQueryToFile.Result type to
include a public bool Success property and an Error property (object/class) with
Message and AdditionalInfo fields, while keeping existing task-specific
properties (EntriesWritten, Path, FileName); mirror the shape used by
ExecuteQuery.Result (or reuse its Error type) so callers can check Success and
read Error.Message/Error.AdditionalInfo when failures occur.

In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj`:
- Line 6: The Result class in Definitions/Result.cs is missing the public
properties referenced by the pipeline; add public bool Success { get; set; } and
public string Error { get; set; } to the Result class (ensure they are public
auto-properties on the Result type so existing callers/serialization continue to
work).

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc51e76 and 49ee547.

📒 Files selected for processing (3)

• Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md
• Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs
• Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj

🧰 Additional context used

📓 Path-based instructions (4)

Frends.*/CHANGELOG.md

⚙️ CodeRabbit configuration file

Frends.*/CHANGELOG.md: Validate format against Keep a Changelog (https://keepachangelog.com/en/1.0.0/)
Include all functional changes and indicate breaking changes with upgrade notes.
Avoid notes like "refactored xyz" unless it affects functionality.

Files:

• Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md

Frends.*/Frends.*/*.cs

⚙️ CodeRabbit configuration file

Frends.*/Frends.*/*.cs: Ensure every public method and class:

• Has
  and XML comments
• If the documentation is very long then it can also use element
• Follows Microsoft C# code conventions
• Uses semantic task result documentation (e.g., Success, Error, Data)

Frends.*/Frends.*/*.cs: Validate all task result classes include:

• Success (bool)
• Task-specific return value (e.g. Data, FilePaths)
• Error object with Message and AdditionalInfo
• Ensure result structure is flat, simple, and avoids 3rd-party types.
  • Use dynamic or JToken only when the structure is unknown.

Files:

• Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs

Frends.*/**/*.cs

⚙️ CodeRabbit configuration file

Frends.*/**/*.cs: Code must follow Microsoft C# coding standards, including:

• PascalCase for public members and task parameters
• Proper naming for abbreviations (Csv, Url, Api)
• Use of var only when type is obvious
• Clean structure and no unused code

Files:

• Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs

Frends.*/Frends.*/*.csproj

⚙️ CodeRabbit configuration file

Frends.*/Frends.*/*.csproj: Ensure the .csproj targets .NET 6 or 8, uses the MIT license, and includes the following fields:

= Frends


= true
= MIT
Follow Microsoft C# project file conventions.

Files:

• Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj

🪛 GitHub Actions: ExecuteQueryToFile_build_test

Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs

[error] 38-38: FT0011: Class should include a 'Success' property

---

[error] 38-38: FT0011: Class should include a 'Error' property

Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.csproj

[error] 38-38: FT0011: Class should include a 'Success' property

---

[error] 38-38: FT0011: Class should include a 'Error' property

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Analyze (csharp)

🔇 Additional comments (3)

Frends.MicrosoftSQL.ExecuteQueryToFile/CHANGELOG.md (1)

3-5: LGTM!

The changelog entry follows the Keep a Changelog format correctly with proper versioning, date format, and categorization under "Fixed".

Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs (2)

19-28: Static constructor pattern for assembly unload subscription looks correct.

The null check on currentContext is appropriate, and subscribing to Unloading in a static constructor is a valid pattern for plugin lifecycle management.

---

84-87: Verify the unload handler implementation is complete.

The handler only unsubscribes itself from the Unloading event but performs no actual resource cleanup. If the goal is to "fix unloading assembly" issues, this handler may be missing cleanup logic (e.g., disposing connections, clearing caches, releasing native resources).

Is this intentional? If the fix requires only removing the event subscription to prevent the assembly from being held in memory, this is sufficient. However, if there are other resources that need cleanup during unload, they should be added here.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[jefim]

So, I see that you have an error handler for unloading, but it does nothing? Why?

[jefim]

x

[MatteoDelOmbra]

at you have an error handler for unlo

Well yes - I messed up with this one 😅 I've started with this template, then must have switch to smth else and when I came back "I saw PR created, tests passed - let's go!" instead of actual finish a fix. sorry for that. Now I've pushed correct changes. I think we don't actual need unloading assemblies mechanism this time

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In
`@Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Definitions/CsvFileWriter.cs`:
- Around line 33-36: The public method DisposeAsync in class CsvFileWriter is
missing XML documentation; add an XML doc comment above the DisposeAsync method
that includes at minimum a <summary> describing that it asynchronously disposes
resources (e.g., disposes SqlCommand if not null) and a <returns> describing the
returned ValueTask; reference the DisposeAsync method and the SqlCommand member
in the comment to make intent clear.
- Around line 38-49: Add a missing XML <summary> for the SaveQueryToCsv method
and ensure the DbDataReader returned by SqlCommand.ExecuteReaderAsync is
disposed: wrap the reader in an await using (or using) block when calling
SqlCommand.ExecuteReaderAsync inside SaveQueryToCsv so reader is disposed after
DataReaderToCsv completes, and update the method's XML documentation to include
a <summary> describing its purpose (SaveQueryToCsv) and any relevant
param/return tags.

♻️ Duplicate comments (1)

Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs (1)

25-28: Result class missing Success and Error properties (pipeline failure).

The pipeline error FT0011 indicates the Result class is missing required properties. This was already flagged in a previous review. Per the coding guidelines, all task result classes must include:

• Success (bool)
• Error object with Message and AdditionalInfo

🧹 Nitpick comments (1)

Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile/Frends.MicrosoftSQL.ExecuteQueryToFile.cs (1)

59-68: Consider adding a default case or exhaustive handling.

The switch only handles ReturnFormat.CSV. If other ReturnFormat values exist or are added, they would silently return an uninitialized Result. Consider adding a default case that either throws NotSupportedException or explicitly handles unsupported formats.

♻️ Suggested improvement

         switch (options.ReturnFormat)
         {
             case ReturnFormat.CSV:
                 {
                     await using var csvWriter = new CsvFileWriter(command, input, options.CsvOptions);
                     result = await csvWriter.SaveQueryToCsv(cancellationToken).ConfigureAwait(false);

                     break;
                 }
+            default:
+                throw new NotSupportedException($"Return format '{options.ReturnFormat}' is not supported.");
         }