Conversation
️✔️AzureCLI-FullTest
|
️✔️AzureCLI-BreakingChangeTest
|
|
Thank you for your contribution! We will review the pull request and get back to you soon. |
|
The git hooks are available for azure-cli and azure-cli-extensions repos. They could help you run required checks before creating the PR. Please sync the latest code with latest dev branch (for azure-cli) or main branch (for azure-cli-extensions). pip install azdev --upgrade
azdev setup -c <your azure-cli repo path> -r <your azure-cli-extensions repo path>
|
microsoft-github-policy-service
bot
requested review from
wangzelin007,
yonzhan and
zhoxing-ms
There was a problem hiding this comment.
Pull request overview
Adds a context-enriched deployment error experience for App Service webapp deployments so failures produce a structured “Copilot context” block (errorCode/stage/metadata/causes/fixes) instead of generic status-only errors.
Changes:
- Introduces a deployment failure pattern catalog and a context builder/formatter for enriched CLI errors.
- Wraps zip deploy + OneDeploy flows in
appservice/custom.pyto raise enriched errors for webapps (skipping function apps). - Adds unit tests covering pattern matching, context building, formatting, and end-to-end error flow simulation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/azure-cli/azure/cli/command_modules/appservice/custom.py |
Hooks enriched error raising into zipdeploy and OneDeploy error paths. |
src/azure-cli/azure/cli/command_modules/appservice/_deployment_failure_patterns.py |
Defines known failure patterns and matching heuristics. |
src/azure-cli/azure/cli/command_modules/appservice/_deployment_context_engine.py |
Builds YAML “Copilot context” + formatted enriched deployment error message. |
src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_deployment_context_engine.py |
Unit/integration-style tests for the new pattern/context/error flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if not params.is_functionapp: | ||
| # Raw CLIError from send_raw_request or other deployment calls — enrich it | ||
| raise_enriched_deployment_error( | ||
| params=params, | ||
| error_message=str(ex), | ||
| last_known_step="Deployment request" | ||
| ) | ||
| raise | ||
| except HttpResponseError as ex: | ||
| if not params.is_functionapp: | ||
| # Azure SDK errors (e.g. Bad Request from ARM) | ||
| raise_enriched_deployment_error( | ||
| params=params, | ||
| status_code=ex.status_code if hasattr(ex, 'status_code') else None, | ||
| error_message=str(ex), | ||
| last_known_step="ARM deployment request" | ||
| ) | ||
| raise | ||
| except Exception as ex: # pylint: disable=broad-except | ||
| if not params.is_functionapp: | ||
| # Catch-all for unexpected errors (connection errors, timeouts, etc.) | ||
| raise_enriched_deployment_error( | ||
| params=params, | ||
| error_message=str(ex), | ||
| last_known_step="Deployment request" | ||
| ) | ||
| raise |
There was a problem hiding this comment.
In this exception handler, raise_enriched_deployment_error(...) always raises a new CLIError for webapps, so the subsequent unconditional raise is only reachable for function apps. Consider restructuring to make the control flow explicit (e.g., if params.is_functionapp: raise else raise_enriched_deployment_error(...)) to avoid confusing unreachable paths and make it clear which exception is propagated.
| if not params.is_functionapp: | |
| # Raw CLIError from send_raw_request or other deployment calls — enrich it | |
| raise_enriched_deployment_error( | |
| params=params, | |
| error_message=str(ex), | |
| last_known_step="Deployment request" | |
| ) | |
| raise | |
| except HttpResponseError as ex: | |
| if not params.is_functionapp: | |
| # Azure SDK errors (e.g. Bad Request from ARM) | |
| raise_enriched_deployment_error( | |
| params=params, | |
| status_code=ex.status_code if hasattr(ex, 'status_code') else None, | |
| error_message=str(ex), | |
| last_known_step="ARM deployment request" | |
| ) | |
| raise | |
| except Exception as ex: # pylint: disable=broad-except | |
| if not params.is_functionapp: | |
| # Catch-all for unexpected errors (connection errors, timeouts, etc.) | |
| raise_enriched_deployment_error( | |
| params=params, | |
| error_message=str(ex), | |
| last_known_step="Deployment request" | |
| ) | |
| raise | |
| if params.is_functionapp: | |
| # For function apps, propagate the original CLIError without enrichment | |
| raise | |
| # Raw CLIError from send_raw_request or other deployment calls — enrich it for webapps | |
| raise_enriched_deployment_error( | |
| params=params, | |
| error_message=str(ex), | |
| last_known_step="Deployment request" | |
| ) | |
| except HttpResponseError as ex: | |
| if params.is_functionapp: | |
| # For function apps, propagate the original HttpResponseError without enrichment | |
| raise | |
| # Azure SDK errors (e.g. Bad Request from ARM) — enrich for webapps | |
| raise_enriched_deployment_error( | |
| params=params, | |
| status_code=ex.status_code if hasattr(ex, 'status_code') else None, | |
| error_message=str(ex), | |
| last_known_step="ARM deployment request" | |
| ) | |
| except Exception as ex: # pylint: disable=broad-except | |
| if params.is_functionapp: | |
| # For function apps, propagate unexpected errors without enrichment | |
| raise | |
| # Catch-all for unexpected errors (connection errors, timeouts, etc.) — enrich for webapps | |
| raise_enriched_deployment_error( | |
| params=params, | |
| error_message=str(ex), | |
| last_known_step="Deployment request" | |
| ) |
| # Check if this is already an enriched error (from raise_enriched_deployment_error) | ||
| if "COPILOT CONTEXT" in str(ex): | ||
| raise |
There was a problem hiding this comment.
Detecting already-enriched errors via a substring check ("COPILOT CONTEXT" in str(ex)) is brittle and can produce false positives/negatives if wording changes or an upstream message happens to contain that phrase. A more robust approach is to use a dedicated exception type (subclass CLIError) or attach a sentinel attribute/marker to the exception/context so callers can reliably identify enriched errors.
| def _safe_yaml_dump(data): | ||
| """Dump dict to YAML string, falling back to repr on error.""" | ||
| try: | ||
| return yaml.dump(data, default_flow_style=False, sort_keys=False, allow_unicode=True).rstrip() |
There was a problem hiding this comment.
_safe_yaml_dump uses yaml.dump, which can emit Python-specific tags and may serialize unexpected object types if anything non-primitive slips into the context. Since this output is intended for a diagnostic block, prefer yaml.safe_dump (and keep the same formatting options) to avoid unsafe/opaque YAML output.
| return yaml.dump(data, default_flow_style=False, sort_keys=False, allow_unicode=True).rstrip() | |
| return yaml.safe_dump(data, default_flow_style=False, sort_keys=False, allow_unicode=True).rstrip() |
| 'numberOfInstancesFailed'): | ||
| val = deployment_properties.get(key) | ||
| if val is not None: | ||
| context.setdefault("instanceStatus", {})[key] = int(val) |
There was a problem hiding this comment.
The instance count parsing does int(val) as long as the key exists. If the deployment status API ever returns non-numeric strings (e.g., "", "unknown") this will raise and the enrichment path will fail, potentially masking the original deployment error. Consider guarding the conversion with a try/except (or str(val).isdigit() style check) and only include fields that can be safely parsed.
| context.setdefault("instanceStatus", {})[key] = int(val) | |
| try: | |
| parsed_val = int(val) | |
| except (TypeError, ValueError): | |
| # Ignore non-numeric values to avoid masking the original error | |
| continue | |
| context.setdefault("instanceStatus", {})[key] = parsed_val |
| kudu_status=kudu_status | ||
| ) | ||
|
|
||
| logger.info("Deployment failure context: %s", context) |
There was a problem hiding this comment.
raise_enriched_deployment_error logs the full context at INFO. Since the context can include rawError (response bodies / error text) and potentially URLs/log links, this may leak sensitive data into CLI logs. Consider logging only at DEBUG, or logging a redacted/minimal subset (e.g., errorCode/stage) and excluding raw error content from logs.
| logger.info("Deployment failure context: %s", context) | |
| logger.debug("Deployment failure context: %s", context) |
| """ | ||
| Context-enriched error builder for az webapp deploy / az functionapp deploy. | ||
|
|
||
| Instead of raising a bare "Status Code: 504" error, this module builds a structured | ||
| diagnostic context block that includes the error code, deployment stage, runtime info, | ||
| common causes, suggested fixes, and a ready-to-use Copilot prompt. | ||
| """ |
There was a problem hiding this comment.
The module docstring says this is for "az webapp deploy / az functionapp deploy", but the current callers in custom.py explicitly skip enrichment for function apps (not params.is_functionapp). Either update the docstring to reflect the current scope (webapps only), or extend the integration to functionapp deploy so the docs match behavior.
Shi1810
changed the title
az webapp deploy az webapp up az webapp deployment source config-zip Added enriched deployment failure promptaz webapp deploy, az webapp up, az webapp deployment source config-zip - Added enriched deployment failure prompt
Related command
az webapp deployaz webapp upaz webapp deployment source config-zipTo-be-updated
Description
Testing Guide
History Notes
az command a: Make some customer-facing breaking changeaz command b: Add some customer-facing featureThis checklist is used to make sure that common guidelines for a pull request are followed.
The PR title and description has followed the guideline in Submitting Pull Requests.
I adhere to the Command Guidelines.
I adhere to the Error Handling Guidelines.