Skip to content

Throw if failed to kill process during cancellation#308

Merged
Tyrrrz merged 3 commits intomasterfrom
improve-edge-case-handling
Dec 4, 2025
Merged

Throw if failed to kill process during cancellation#308
Tyrrrz merged 3 commits intomasterfrom
improve-edge-case-handling

Conversation

@Tyrrrz
Copy link
Copy Markdown
Owner

@Tyrrrz Tyrrrz commented Dec 4, 2025

No description provided.

Copilot AI review requested due to automatic review settings December 4, 2025 16:33
@Tyrrrz Tyrrrz added the bug label Dec 4, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 87.09677% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.48%. Comparing base (0a57f23) to head (274e666).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
CliWrap/Command.Execution.cs 87.09% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #308      +/-   ##
==========================================
- Coverage   94.75%   94.48%   -0.28%     
==========================================
  Files          46       45       -1     
  Lines        1202     1215      +13     
  Branches       88       89       +1     
==========================================
+ Hits         1139     1148       +9     
- Misses         38       42       +4     
  Partials       25       25              

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

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

This PR improves error handling when process termination fails during cancellation by throwing an explicit exception instead of silently continuing, and removes the custom CancellationTokenExtensions in favor of inline exception throwing.

Key Changes:

  • Added explicit exception handling for process termination timeouts with a new Win32Exception throw when the process cannot be killed within the allotted timeout
  • Replaced the custom ThrowIfCancellationRequested extension method with inline OperationCanceledException throws
  • Enhanced exception handling to better distinguish between internal cancellations (stdin, wait timeout) and user-requested cancellations

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
CliWrap/Utils/Extensions/CancellationTokenExtensions.cs Removed custom extension method that wrapped cancellation token checking
CliWrap/Command.Execution.cs Added System.ComponentModel import; refactored exception handling to throw explicit Win32Exception on termination timeout; replaced extension method calls with inline exception throwing; updated comments for stdin piping behavior

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

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Tyrrrz Tyrrrz merged commit 70e2291 into master Dec 4, 2025
7 checks passed
@Tyrrrz Tyrrrz deleted the improve-edge-case-handling branch December 4, 2025 16:41
RicherTunes pushed a commit to RicherTunes/Lidarr.Plugin.Common that referenced this pull request Mar 25, 2026
Updated [CliWrap](https://github.com/Tyrrrz/CliWrap) from 3.10.0 to
3.10.1.

<details>
<summary>Release notes</summary>

_Sourced from [CliWrap's
releases](https://github.com/Tyrrrz/CliWrap/releases)._

## 3.10.1

## What's Changed
* Throw if failed to kill process during cancellation by @​Tyrrrz in
Tyrrrz/CliWrap#308
* Bump the nuget group with 4 updates by @​dependabot[bot] in
Tyrrrz/CliWrap#314
* Migrate to Centralized NuGet Package Management (CPM) by @​Copilot in
Tyrrrz/CliWrap#319
* Bump the nuget group with 6 updates by @​dependabot[bot] in
Tyrrrz/CliWrap#320
* Remove explicit Microsoft.SourceLink.GitHub package reference by
@​Copilot in Tyrrrz/CliWrap#321
* Use `LibraryImport` instead of `DllImport` for proper AOT support by
@​Copilot in Tyrrrz/CliWrap#323

## New Contributors
* @​Copilot made their first contribution in
Tyrrrz/CliWrap#319

**Full Changelog**:
Tyrrrz/CliWrap@3.10...3.10.1

Commits viewable in [compare
view](Tyrrrz/CliWrap@3.10...3.10.1).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=CliWrap&package-manager=nuget&previous-version=3.10.0&new-version=3.10.1)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants