Repository navigation
Improved azure error logging - #2151
Conversation
8183115 to
de48c4c
Compare
| { | ||
| log.Error("Error polling for deployment completion"); | ||
| throw; | ||
| throw new CommandException(await BuildDeploymentFailureMessage(resourceGroupResource, deploymentName, ex), ex); |
There was a problem hiding this comment.
Throwing a CommandException allows this to be handled as a known error and prevents stack trace logging. The stack trace shouldn't be needed here since the failure is an Azure error
|
|
||
| throw; | ||
| } | ||
| catch |
There was a problem hiding this comment.
Connection errors, etc should fall back to the original path
| { | ||
| using var errorLoggingCancellation = new CancellationTokenSource(FailureLoggingTimeout); | ||
|
|
||
| var deploymentResponse = await resourceGroupResource.GetArmDeploymentAsync(deploymentName, errorLoggingCancellation.Token); |
There was a problem hiding this comment.
The initial azure failure response does not contain failure information, so we need to request the deployment and failed resource details
|
|
||
| var report = await CollectFailedOperations(deploymentResponse.Value, errorLoggingCancellation.Token); | ||
|
|
||
| if (report.FailureCount == 0) |
There was a problem hiding this comment.
When Azure lists no resources as having failed, we need to rely on its base exception message to tell the user what went wrong
There was a problem hiding this comment.
🟡 Changes recommended
The new RequestFailedException handling can regress logging by emitting no Calamari error context when detailed failure retrieval returns null.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Improves the diagnostic output for failed Azure Resource Group deployments by querying ARM deployment operation results and formatting a concise failure summary that highlights the specific resources and errors involved.
Changes:
- Adds failure-detail retrieval on
RequestFailedExceptionduring deployment polling, with a time-bounded attempt to collect failed deployment operations. - Introduces formatting helpers (
FormatFailedOperation,ExtractAzureErrorInfo) to produce actionable, user-facing failure summaries. - Adds NUnit coverage for the new formatting/error-extraction helpers.
File summaries
| File | Description |
|---|---|
| source/Calamari.AzureResourceGroup/AzureResourceGroupOperator.cs | Adds deployment-failure detail collection from ARM deployment operations and improved error formatting/logging. |
| source/Calamari.AzureResourceGroup.Tests/AzureResourceGroupOperatorFixture.cs | Adds unit tests for the new failure formatting and error extraction helpers. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
benPearce1
left a comment
benPearce1
left a comment
There was a problem hiding this comment.
Nice. This looks like a really useful improvement.
Description
Fixes: HPY-1405
When an Azure deployment fails, Calamari currently logs Azure's
DeploymentFailedstatus, but this does not provide users with any useful information about what went wrong and how the issue might be resolved. This PR improves error logging by retrieving the deployment operation results from Azure and providing detailed logging when more specific error messages are available.Results
Single resource failure
Multiple resource failures
Successful deployment (unchanged)