Skip to content

Bound the NuGet package search timeout - #19178

Draft
Adam Ratzman (adamint) wants to merge 4 commits into
microsoft:mainfrom
adamint:adamint/bound-nuget-package-search
Draft

Bound the NuGet package search timeout#19178
Adam Ratzman (adamint) wants to merge 4 commits into
microsoft:mainfrom
adamint:adamint/bound-nuget-package-search

Conversation

@adamint

@adamint Adam Ratzman (adamint) commented Aug 9, 2026

Copy link
Copy Markdown
Member

Description

aspire add can hang while the bundled NuGet helper is searching package metadata. The helper was already launched with killOnParentExit, but that only prevents a leaked child after a hard-killed CLI; it does not bound the wait for a live but wedged helper. In CI, that left the 10-minute MTP hang timeout as the first clear failure.

This adds a 3-minute timeout to synchronous LayoutProcessRunner.RunAsync helper executions. NuGet search and restore timeouts now fail with a clear message, for example NuGet package search for 'Aspire.Hosting' timed out after 3 minutes contacting configured NuGet sources..., while caller cancellation still flows as cancellation.

The bound lives in LayoutProcessRunner rather than BundleNuGetPackageCache because search, restore, manifest creation, DCP stop, and DCP cleanup all share the same synchronous helper wait path. StartAsync is unchanged because it intentionally returns a background process whose lifetime is owned by the caller.

Validation:

  • Red: the new timeout test failed before the implementation because RunAsync had no timeout parameter.
  • Mutation: removing CancelAfter(effectiveTimeout) makes RunAsync_WhenProcessDoesNotExitWithinTimeout_ThrowsTimeoutException fail with TaskCanceledException after the test guard expires.
  • Green: dotnet test --project tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true" passes: 4,786 succeeded, 33 skipped, 0 failed.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

Add a default timeout to synchronous layout helper executions so hung NuGet searches fail with a clear timeout instead of relying on outer CI hang dumps. Keep caller cancellation flowing as cancellation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings August 9, 2026 04:13
@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19178

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19178"

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a three-minute bound to synchronous layout helper executions, preventing aspire add from hanging on NuGet operations.

Changes:

  • Adds configurable timeout handling with clear errors.
  • Preserves caller cancellation behavior.
  • Adds timeout and cancellation regression tests.
Show a summary per file
File Description
src/Aspire.Cli/Layout/LayoutProcessRunner.cs Implements bounded helper execution and timeout messages.
tests/Aspire.Cli.Tests/Layout/LayoutProcessRunnerTests.cs Tests timeout and caller cancellation behavior.

Review details

Tip

Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread src/Aspire.Cli/Layout/LayoutProcessRunner.cs
Adam Ratzman (adamint) and others added 3 commits August 9, 2026 01:10
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 9, 2026 05:23

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@github-actions

github-actions Bot commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@afscrome

Copy link
Copy Markdown
Collaborator

3 minutes seems like a very long timeout. I'd struggle to wait 3 seconds!. 15 or 30 seconds would seem like a more reasonable timeout to me.

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.

3 participants