fix(provision): surface nested ARM errors in provision --preview - #9324
fix(provision): surface nested ARM errors in provision --preview#9324hemarina wants to merge 4 commits into
Conversation
`azd provision --preview` built its error message with a bespoke loop that walked only the first level of the ARM what-if `ErrorResponse`. That type is recursive, and preflight validation puts the actionable cause (quota, SKU, region) in `details[].details[]`, so the real reason was dropped. Users saw "Preflight validation failed. Please refer to the details for the specific errors." with no details following it, even under --debug. A failed what-if returns HTTP 200 with the failure in the payload, so the SDK returns no Go error and the response never reached `createDeploymentError`. The preview path therefore missed the recursive formatter that the deploy path has used all along. Route the preview failure through `azapi.AzureDeploymentError` via a new `NewAzureDeploymentErrorFromResponse` constructor. Marshalling the SDK model back to JSON lets it reuse the single existing parser rather than adding a second recursive walker. Because the error is now typed, preview failures also pick up the error-suggestion pipeline and ARM telemetry classification, which they previously bypassed entirely. The title mapping in `createDeploymentError` is extracted to `deploymentErrorTitle` so both entry points share it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: c8f14514-13c9-4f54-87f4-d11ef4b03ac3
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: ab4a1645-0302-4199-9f11-69bb7fc4cd23
There was a problem hiding this comment.
Pull request overview
Routes ARM what-if failures through the existing recursive deployment-error pipeline, exposing nested causes and suggestions.
Changes:
- Adds an
ErrorResponse-based deployment error constructor. - Recursively renders nested preview errors with raw-payload fallback.
- Adds regression and pipeline integration coverage.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go |
Uses structured preview errors. |
cli/azd/pkg/azapi/deployments.go |
Adds shared titles and response constructor. |
cli/azd/pkg/azapi/deployment_error_integration_test.go |
Tests nested quota handling and suggestions. |
cli/azd/pkg/azapi/azure_deployment_error.go |
Falls back when parsed errors render empty. |
cli/azd/pkg/azapi/azure_deployment_error_test.go |
Covers unrenderable payloads. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). 21 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
cli/azd/resources/error_suggestions.yaml:267
- This operation-only rule is a catch-all, and the pipeline uses first-match-wins ordering. Its current position masks every specific rule below it—including
ValidationError,ResourceNotFound,Conflict, and all Container Apps codes—so preview failures with those nested causes receive this generic advice instead of the existing actionable suggestion. Move this guard, together with the self-referentialInvalidTemplatefallback it protects, after all specificDeploymentErrorLinerules and cover a code currently below this point.
- errorType: "AzureDeploymentError"
properties:
Operation: "preview"
message: "Azure validation rejected the deployment template."
suggestion: "Review the nested error codes above for the specific cause."
cli/azd/pkg/azapi/deployments.go:428
- A nil response still needs validation here.
json.Marshal(nil)producesnull, yielding anAzureDeploymentErrorwithDetails == nil; the telemetry path then callsclassifyArmDeployError, which iterates[]*DeploymentErrorLine{nil}and dereferencesdetail.Inner, causing a panic. Reject nil here (or harden that classifier) before returning the typed error.
raw, err := json.Marshal(errResp)
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Fix #9011
Problem
azd provision --previewreported preflight failures without the actual cause:The details it points to were never printed — not even with
--debug. In the reportedcase the underlying cause was a storage account quota limit.
Root cause
BicepProvider.Previewformatted the error with a bespoke loop that read onlyCodeand
Messageof each direct child of the ARMErrorResponse. That type isrecursive (
Details []*ErrorResponse), and preflight validation nests the actionablecause under
error.details[].details[]— exactly the level being discarded.azd provision(without--preview) was unaffected because it routes ARM failuresthrough
azapi.NewAzureDeploymentError→getErrorsFromMap, which walks the treerecursively. Preview never reached that code: a failed what-if returns HTTP 200
with the failure in the response body, so the SDK returns no Go error and
createDeploymentErroris never invoked.--debugdidn't help either, becausearm.ClientOptionsleavespolicy.LogOptions.IncludeBodyunset, so the response body is never logged.Fix
Route the preview failure through
azapi.AzureDeploymentErrorusing a newNewAzureDeploymentErrorFromResponseconstructor. Marshalling the SDK model back toJSON lets it reuse the one existing recursive parser instead of adding a second walker.
Because the error is now typed rather than a flat string, preview failures also gain
three things they previously bypassed:
error_suggestions.yamlpipeline (which already has aSubscriptionIsOverQuotaForSkurule),classifyArmDeployError,TelemetryMiddleware.Net: −21 lines of bespoke formatting, +1 reusable constructor.
Before / after
Testing
Added
TestPipeline_PreviewErrorResponse_NestedQuota, which drives a realistic ARMwhat-if payload through the real embedded
error_suggestions.yamlrules rather thaninline test rules. It specifically covers a top-level code that
getErrorsFromMapdoesnot blank out (unlike
DeploymentFailed), so it verifies the matcher keeps searchingpast the outer node — a case the existing tests did not cover.
go test ./pkg/azapi/... ./pkg/errorhandler/... ./pkg/infra/... ./internal/cmd/... -short— passgofmt -s -l,golangci-lint run,cspell— cleanNotes for reviewers
StackDeployments.WhatIf*returnsErrPreviewNotSupported.service.arm.preview.failedwith
ServiceErrorCodepopulated, instead of falling into a generic bucket. This is anew value in the existing
service.arm.<operation>.failedfamily (deploymentandvalidatealready exist), not a new field or event, so I did not update the telemetrydocs — happy to if you'd prefer.
targetandadditionalInfoare still not rendered, so the outputdoesn't say which resource hit the quota. That's a pre-existing gap in
getErrorsFromMapaffecting the deploy path equally; I left it alone to keep thischange focused.
errors nest JSON inside
message, so wrapper nodes (Conflict,BadRequest)intentionally render nothing, and emitting their codes turned the
arm_sample_error_*expectations from 5 lines into 14 of noise.Update — review follow-up (
0caaeeac4)A review pass found a latent gap in the renderer this PR now routes preview through.
AzureDeploymentError.Error()fell back to the raw payload only when the JSON failed toparse. When it parsed into a tree where nothing was renderable, the whole error
collapsed to a bare heading:
That is reachable when every node is code-only, carries a blanked code (
DeploymentFailed,ResourceDeploymentFailure) with no details, or the body isnull. It is pre-existing onthe deploy path —
arm_sample_error_02.jsonalready contains aRequestTimeoutsubtreethat renders nothing — so routing preview through the same renderer would have inherited it.
Error()now also falls back when the parsed tree yields zero lines, not only on parsefailure. The fallback is gated on
len(lines) == 0, so it is unreachable whenever the parserproduces any output. Confirmed by running identical inputs through both versions:
arm_sample_error_02(deploy path)ResourceNameInvalid: …nullAll four
arm_sample_error_*expectations are unchanged.To be clear about scope: no recorded real-world ARM payload hits the changed cases, since
ARM errors carry a
message. This is defensive hardening against a blank error, not a fixfor something users are hitting today.
Tests added / tightened
Test_Parsed_But_Unrenderable_Error_Falls_Back_To_Json— code-only, blanked code with nodetails,
additionalInfo-only, andnullbody.TestPipeline_PreviewErrorResponse_NestedQuotanow asserts the error codes in orderrather than four independent
Containschecks, which would still pass if the walkerstopped descending.
Considered and skipped
NewAzureDeploymentErrorFromResponseto return*AzureDeploymentError— it isused directly as an
errorat the call site, so returning a concrete pointer type risks thetyped-nil-in-interface footgun. A
nilguard is also no longer needed:json.Marshal(nil)yields
"null", which the new fallback surfaces.noise to the
arm_sample_error_*expectations. The fallback only fires when the entiretree is empty, so it does not reintroduce that.
Updated totals: 5 files, +148/−30.