Conversation
Per the Azure error schema (https://schema.mp.microsoft.com/schema/error/2022-07-01) , "details" is optional. Treat its absence as no further nesting rather than raising InvalidSchema. Assisted-by: Claude Sonnet 4.6
Reviewer's GuideUpdates Azure certification-error traversal to treat an omitted optional details field as a leaf node, while continuing to reject invalid non-list details values, and adds regression coverage for the schema-compliant case. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="cloudpub/ms_azure/utils.py" line_range="559-561" />
<code_context>
if code == "invalidState" and "certification" in message.lower():
return True
- if not isinstance(item.get('details'), list):
+ details = item.get('details')
+ if details is None:
+ return False
+ if not isinstance(details, list):
raise InvalidSchema(f"Invalid schema for 'details' inside error object: {item}")
</code_context>
<issue_to_address>
**issue (bug_risk):** An explicitly present `"details": null` value is treated as equivalent to an omitted field and returns `False` instead of raising `InvalidSchema`, even though the Azure schema defines `details` as an optional array rather than a nullable value. This silently accepts malformed error payloads and can cause certification failures hidden behind such a payload to be classified as retryable.
**Triggers:** When Azure returns an error object containing `"details": null`.
**Suggested fix:** Check `if "details" not in item` for the omitted-field case, then retain the non-list validation so an explicit `null` remains invalid unless the schema is confirmed to allow it.
```suggestion
if "details" not in item:
return False
details = item.get('details')
```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: cloudpub/ms_azure/utils.py:561
| details = item.get('details') | ||
| if details is None: | ||
| return False |
There was a problem hiding this comment.
issue (bug_risk): An explicitly present "details": null value is treated as equivalent to an omitted field and returns False instead of raising InvalidSchema, even though the Azure schema defines details as an optional array rather than a nullable value. This silently accepts malformed error payloads and can cause certification failures hidden behind such a payload to be classified as retryable.
Triggers: When Azure returns an error object containing "details": null.
Suggested fix: Check if "details" not in item for the omitted-field case, then retain the non-list validation so an explicit null remains invalid unless the schema is confirmed to allow it.
| details = item.get('details') | |
| if details is None: | |
| return False | |
| if "details" not in item: | |
| return False | |
| details = item.get('details') |
@beerparty @lslebodn PTAL
Per the Azure error schema (https://schema.mp.microsoft.com/schema/error/2022-07-01) , "details" is optional.
Treat its absence as no further nesting rather than raising InvalidSchema.
Assisted-by: Claude Sonnet 4.6
Summary by Sourcery
Treat missing Azure error details as a terminal error branch while preserving validation for malformed non-list details.
Bug Fixes:
detailsfield without raising a schema error.Tests:
detailsfields.