Surface build output on failure for Docker, Podman, and dotnet publish#16093
Surface build output on failure for Docker, Podman, and dotnet publish#16093
Conversation
Introduce ProcessFailedException that captures stdout/stderr from failed build processes. The exception's Message property includes the last 50 lines of output so it surfaces through the pipeline step reporter without requiring --log-level debug. All three container build paths now throw ProcessFailedException consistently: - Docker buildx (deduped to use base class ExecuteContainerCommand) - Podman build - dotnet publish /t:PublishContainer Uses ConcurrentQueue<string> for thread-safe output buffering since OnOutputData/OnErrorData fire on different threads. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16093Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16093" |
There was a problem hiding this comment.
Pull request overview
This PR improves diagnostics for container image builds by capturing and surfacing build stdout/stderr when Docker, Podman, or dotnet publish /t:PublishContainer fails, so users can see actionable error output without re-running with --log-level debug.
Changes:
- Added
ProcessFailedExceptionto carry exit code plus captured build output and include formatted tail output inMessage. - Updated Docker and Podman build paths (including buildkit instance creation) to capture output and throw
ProcessFailedExceptionon non-zero exit codes. - Updated
dotnet publishcontainer build path to capture output and throwProcessFailedException, plus added an integration test asserting build output is present.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.Tests/Publishing/ResourceContainerImageManagerTests.cs | Adds Docker build failure integration test validating captured output is present on failure. |
| src/Aspire.Hosting/Publishing/ResourceContainerImageManager.cs | Captures dotnet publish output and throws ProcessFailedException on failure. |
| src/Aspire.Hosting/Publishing/ProcessFailedException.cs | Introduces new exception type that formats and surfaces tail build output in Message. |
| src/Aspire.Hosting/Publishing/PodmanContainerRuntime.cs | Captures Podman build output and throws ProcessFailedException on failure. |
| src/Aspire.Hosting/Publishing/DockerContainerRuntime.cs | Refactors Docker buildx path to shared execution helper, captures output, throws ProcessFailedException (including for buildkit creation failures). |
| src/Aspire.Hosting/Publishing/ContainerRuntimeBase.cs | Extends command execution helper to optionally buffer stdout/stderr lines for callers to include in exceptions. |
Copilot's findings
Comments suppressed due to low confidence (1)
tests/Aspire.Hosting.Tests/Publishing/ResourceContainerImageManagerTests.cs:1457
- This Docker build-failure test uses a remote base image (
mcr.microsoft.com/cbl-mariner/...). If the pull fails (network, throttling, outage), the build may fail before theCOPY nonexistent-file-12345.txtstep and the assertion on that filename can become flaky. Consider usingFROM scratch(or another already-cached test image used elsewhere) so the failure deterministically comes from the missing file.
await File.WriteAllTextAsync(dockerfilePath, """
FROM mcr.microsoft.com/cbl-mariner/base/core:2.0
COPY nonexistent-file-12345.txt /app/
""");
- Files reviewed: 6/6 changed files
- Comments generated: 6
tests/Aspire.Hosting.Tests/Publishing/ResourceContainerImageManagerTests.cs
Outdated
Show resolved
Hide resolved
…gging - Add using to TestTempDirectory in test to prevent temp folder leaks - Add catch/log pattern in BuildProjectContainerImageAsync for consistent error logging across build paths Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The existing BuildImageAsync_ProjectBuildFailureIncludesResourceName test expected InvalidOperationException but now gets ProcessFailedException after our changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🎬 CLI E2E Test Recordings — 68 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24314128290 |
| IEnumerable<string> lines = BuildOutput.Count > maxLines | ||
| ? BuildOutput.Skip(BuildOutput.Count - maxLines) | ||
| : BuildOutput; |
There was a problem hiding this comment.
If the number of lines is truncated then should that be included in the message? e.g. Build output truncated: showing last 50 of 200 lines.
JamesNK
left a comment
There was a problem hiding this comment.
1 issue found (correctness). The new exception type won't pass through the pipeline step executor cleanly — it will get wrapped with a redundant step-name prefix instead of surfacing the formatted build output directly.
| /// <summary> | ||
| /// Exception thrown when a container image build or dotnet publish operation fails. | ||
| /// </summary> | ||
| internal sealed class ProcessFailedException : Exception |
There was a problem hiding this comment.
ProcessFailedException extends Exception directly, but the pipeline step executor in DistributedApplicationPipeline.ExecuteStepAsync re-throws DistributedApplicationException subtypes unchanged while wrapping everything else in InvalidOperationException($"Step '{step.Name}' failed: {ex.Message}").
Since build image calls flow through pipeline steps (e.g. ContainerResourceBuilderExtensions and ProjectResource.BuildProjectImage), this exception will be wrapped and the user-facing message will include a redundant "Step 'build-myapi' failed:" prefix — even though the step name is already shown by the pipeline reporter context ((build-myapi) ✗).
Consider either making ProcessFailedException extend DistributedApplicationException, or adding it to the pipeline's pass-through catch clause so the formatted build output surfaces cleanly without the extra wrapping.
Description
Surface build output on failure for Docker, Podman, and dotnet publish container builds.
Problem
When a container image build fails, users see only:
The actual build output (compiler errors, missing files, Dockerfile issues) is logged at Debug level only. Users have to re-run with
--log-level debugto see what went wrong.Solution
Introduce
ProcessFailedExceptionthat captures stdout/stderr from failed build processes. The exception'sMessageproperty includes the last 50 lines of output so it surfaces through the pipeline step reporter automatically.Changes
ProcessFailedException— new internal exception type withExitCode,BuildOutput(raw lines), andGetFormattedOutput().Messageis overridden to include formatted output.ExecuteContainerCommandWithExitCodeAsync, throwsProcessFailedExceptionwith captured outputProcessFailedExceptionwith captured outputProcessFailedExceptionwith captured outputProcessFailedExceptionConcurrentQueue<string>for thread-safe output buffering (callbacks fire on different threads)Before
After
Checklist