Conversation
|
This looks really good @praneeth-0000 Nice work |
merill
left a comment
There was a problem hiding this comment.
@alexandair can you review the code before I merge?
There was a problem hiding this comment.
Pull request overview
Adds a new private PowerShell helper, Get-FormattedError, to turn catch error records into a more readable, user-friendly bullet list (with optional extra details).
Changes:
- Introduces
Get-FormattedErrorto formatErrorRecordinto a concise markdown-ish message. - Attempts to extract HTTP status, Graph-style JSON error messages, and request IDs.
- Adds an
-IncludeDetailsswitch to include category and exception type.
Comments suppressed due to low confidence (2)
src/powershell/private/core/Get-FormattedError.ps1:68
ConvertFrom-Jsonerror behavior can vary by host/version; to guarantee your innertry/catchreliably catches parsing failures, pass-ErrorAction Stop(or call it in a way that ensures a terminating error) so you don’t end up silently continuing with partially populated variables.
$errorJson = $jsonContent | ConvertFrom-Json
src/powershell/private/core/Get-FormattedError.ps1:116
- Joining with
"`n"can produce inconsistent output across environments (e.g., Windows tools expecting CRLF). If this output is intended for logs/test artifacts consumed on multiple platforms, consider using[Environment]::NewLineto normalize line endings.
$formattedError = ($errorDetails | ForEach-Object { "- $_" }) -join "`n"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ($ErrorObject.Exception.Response.StatusCode) { | ||
| $httpStatus = "$($ErrorObject.Exception.Response.StatusCode.value__) $($ErrorObject.Exception.Response.StatusCode)" |
There was a problem hiding this comment.
Exception.Response is not a property on many exception types; accessing a missing property can throw PropertyNotFoundException and force the function into the outer catch (losing the intended formatting). Consider checking for the property/type more defensively (e.g., inspect PSObject.Properties['Response'], or gate this behind known HTTP/Web exception types), and avoid using .value__ (it’s an internal enum field) by casting to [int] instead.
| if ($ErrorObject.Exception.Response.StatusCode) { | |
| $httpStatus = "$($ErrorObject.Exception.Response.StatusCode.value__) $($ErrorObject.Exception.Response.StatusCode)" | |
| if ($ErrorObject.Exception -and | |
| $ErrorObject.Exception.PSObject.Properties['Response'] -and | |
| $ErrorObject.Exception.Response -and | |
| $ErrorObject.Exception.Response.PSObject.Properties['StatusCode']) { | |
| $statusCode = $ErrorObject.Exception.Response.StatusCode | |
| $httpStatus = ("{0} {1}" -f ([int]$statusCode), $statusCode) |
| # Find the actual error JSON by looking for {"error" pattern | ||
| $jsonContent = $null | ||
| $startIndex = $ErrorObject.ErrorDetails.Message.IndexOf('{"error"') | ||
| if ($startIndex -ge 0) { | ||
| # Find the matching closing brace for this JSON object | ||
| $lastIndex = $ErrorObject.ErrorDetails.Message.LastIndexOf('}') | ||
| if ($lastIndex -gt $startIndex) { | ||
| $jsonContent = $ErrorObject.ErrorDetails.Message.Substring($startIndex, $lastIndex - $startIndex + 1) | ||
| } | ||
| } | ||
|
|
||
| if ($jsonContent) { | ||
| $errorJson = $jsonContent | ConvertFrom-Json | ||
|
|
There was a problem hiding this comment.
The JSON extraction logic is brittle: IndexOf('{"error"') won’t match valid bodies with leading whitespace/newlines, different casing, or other JSON shapes, and LastIndexOf('}') can over-capture if the message contains additional braces after the JSON body. A more robust approach is to locate the first { of the JSON payload and then either (1) regex-capture the JSON block, or (2) attempt ConvertFrom-Json on progressively smaller suffixes until it parses, falling back cleanly when it doesn’t.
| # Find the actual error JSON by looking for {"error" pattern | |
| $jsonContent = $null | |
| $startIndex = $ErrorObject.ErrorDetails.Message.IndexOf('{"error"') | |
| if ($startIndex -ge 0) { | |
| # Find the matching closing brace for this JSON object | |
| $lastIndex = $ErrorObject.ErrorDetails.Message.LastIndexOf('}') | |
| if ($lastIndex -gt $startIndex) { | |
| $jsonContent = $ErrorObject.ErrorDetails.Message.Substring($startIndex, $lastIndex - $startIndex + 1) | |
| } | |
| } | |
| if ($jsonContent) { | |
| $errorJson = $jsonContent | ConvertFrom-Json | |
| # Find the actual error JSON by locating the first JSON object and parsing it | |
| $errorJson = $null | |
| $rawMessage = $ErrorObject.ErrorDetails.Message | |
| $startIndex = $rawMessage.IndexOf('{') | |
| if ($startIndex -ge 0) { | |
| # First try to parse from the first '{' to the end | |
| $jsonCandidate = $rawMessage.Substring($startIndex) | |
| try { | |
| $errorJson = $jsonCandidate | ConvertFrom-Json | |
| } | |
| catch { | |
| # If that fails, progressively trim from the end until parsing succeeds | |
| for ($end = $rawMessage.Length; $end -gt $startIndex; $end--) { | |
| $length = $end - $startIndex | |
| $slice = $rawMessage.Substring($startIndex, $length) | |
| try { | |
| $errorJson = $slice | ConvertFrom-Json | |
| break | |
| } | |
| catch { | |
| # Ignore and continue trimming | |
| } | |
| } | |
| } | |
| } | |
| if ($errorJson) { |
Added
Get-FormattedErrorfunction which formats error object from catch block and formats it so it's user friendly and readable.Network Spec Example
Get-FormattedErrorfunctionGet-FormattedErrorfunctionGet-FormattedErrorfunction-IncludeDetailsData Spec Example
Get-FormattedErrorfunctionGet-FormattedErrorfunction