Skip to content

Commit

Permalink
Improve error handling (#228)
Browse files Browse the repository at this point in the history
# Description
Summary of changes:
- Improve how errors are captured and displayed on screen
- Created new `Trace-Exception` that can be used to pass exceptions to.
Example:
```
try {
    1 / 0
}
catch {
    $_ | Trace-Exception
}
```
- `Trace-Exception` will then parse the data and pass to `Trace-Output
-Level:Exception -Exception $Exception` with the appropriate properties
defined
- `Trace-Output` has been updated to better provide a formatted message
and then insert the script stack trace into the message content of the
trace file, rather than onto the screen.
- All existing functions leveraging `Trace-Output -Level:Exception` have
been converted to `Trace-Output -Level:Error` which will just leverage
`Write-Host -ForegroundColor Red` to display any errors to the screen.
- `Trace-Output -Level:Exception` will call `Write-Error` 

# Change type
- [ ] Bug fix (non-breaking change)
- [ ] Code style update (formatting, local variables)
- [x] New Feature (non-breaking change that adds new functionality
without impacting existing)
- [ ] Breaking change (fix or feature that may cause functionality
impact)
- [ ] Other

# Checklist:
- [x] My code follows the style and contribution guidelines of this
project.
- [x] I have tested and validated my code changes.
  • Loading branch information
arudell authored Jan 9, 2024
1 parent 5712fe9 commit 3082326
Show file tree
Hide file tree
Showing 157 changed files with 249 additions and 211 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function Copy-UserProvidedCertificateToFabric {
Copy-CertificateToFabric -CertFile $nodeCertConfig.Cert.FileInfo.FullName -CertPassword $CertPassword -FabricDetails $FabricDetails -NetworkControllerNodeCert -Credential $Credential
}
else {
"Unable to locate self-signed certificate file for {0}. Node certificate may need to be manually installed to other Network Controllers manually." -f $controller | Trace-Output -Level:Exception
"Unable to locate self-signed certificate file for {0}. Node certificate may need to be manually installed to other Network Controllers manually." -f $controller | Trace-Output -Level:Error
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,6 @@ function Export-RegistryKeyConfigDetails {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Common/private/Get-CommonConfigState.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ function Get-CommonConfigState {

"Collect general configuration state details" | Trace-Output
if (-NOT (Initialize-DataCollection -FilePath $OutputDirectory.FullName -MinimumMB 100)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -75,7 +75,7 @@ function Get-CommonConfigState {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}

$ProgressPreference = 'Continue'
Expand Down
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/private/Get-TraceProviders.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,6 @@ function Get-TraceProviders {
return $traceProvidersArray
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,6 @@ function Start-EtwTraceSession {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/private/Start-NetshTrace.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,6 @@ function Start-NetshTrace {
return $object
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ function Stop-EtwTraceSession {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/private/Stop-NetshTrace.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ function Stop-NetshTrace {
return $object
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ function Convert-SdnEtwTraceToTxt {
return $object
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/public/Enable-SdnVipTrace.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,6 @@ function Enable-SdnVipTrace {
return $traceFileInfo
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Common/public/Get-SdnCertificate.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ function Get-SdnCertificate {
}

if ($filteredCert.NotAfter -le (Get-Date)) {
"Certificate [Thumbprint: {0} | Subject: {1}] is currently expired" -f $filteredCert.Thumbprint, $filteredCert.Subject | Trace-Output -Level:Exception
"Certificate [Thumbprint: {0} | Subject: {1}] is currently expired" -f $filteredCert.Thumbprint, $filteredCert.Subject | Trace-Output -Level:Error
}

return $filteredCert
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ function Get-SdnDiagnosticLogFile {
# we want to call the initialize datacollection after we have identify the amount of disk space we will need to create a copy of the logs
$minimumDiskSpace = [float](Get-FolderSize -FileName $logFiles.FullName -Total).GB * 3.5
if (-NOT (Initialize-DataCollection -FilePath $OutputDirectory.FullName -MinimumGB $minimumDiskSpace)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -92,6 +92,6 @@ function Get-SdnDiagnosticLogFile {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Common/public/Get-SdnEventLog.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function Get-SdnEventLog {
"Collect event logs between {0} and {1} UTC" -f $fromDateUTC, $toDateUTC | Trace-Output

if (-NOT (Initialize-DataCollection -Role $Role -FilePath $OutputDirectory.FullName -MinimumGB 1)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -87,6 +87,6 @@ function Get-SdnEventLog {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/public/Invoke-SdnGetNetView.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,6 @@ function Invoke-SdnGetNetView {
return $compressedArchive.FullName
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/public/New-SdnCertificate.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,6 @@ function New-SdnCertificate {
return $selfSignedCert
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,6 @@ function Repair-SdnDiagnosticsScheduledTask {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,6 @@ function Set-SdnCertificateAcl {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Common/public/Start-SdnDataCollection.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ function Start-SdnDataCollection {
}

if (-NOT (Initialize-DataCollection -FilePath $OutputDirectory.FullName -MinimumGB $minGB)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -338,6 +338,6 @@ function Start-SdnDataCollection {
}
catch {
$stopwatch.Stop()
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function Start-SdnEtwTraceCapture {
# we want to calculate the max size on number of factors to ensure sufficient disk space is available
$diskSpaceRequired = $maxTraceSize*($traceProvidersArray.Count)*1.5
if (-NOT (Initialize-DataCollection -Role $Role -FilePath $OutputDirectory -MinimumMB $diskSpaceRequired)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand All @@ -41,6 +41,6 @@ function Start-SdnEtwTraceCapture {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Common/public/Start-SdnNetshTrace.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ function Start-SdnNetshTrace {
}

if (-NOT ( Initialize-DataCollection -Role $Role -FilePath $OutputDirectory -MinimumMB ($MaxTraceSize*1.5) )) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Start-NetshTrace @params
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,6 @@ function Stop-SdnEtwTraceCapture {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
2 changes: 1 addition & 1 deletion src/modules/SdnDiag.Common/public/Stop-SdnNetshTrace.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,6 @@ function Stop-SdnNetshTrace {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function Get-GatewayConfigState {
"Collect configuration state details for role {0}" -f $config.Name | Trace-Output

if (-NOT (Initialize-DataCollection -Role 'Gateway' -FilePath $OutputDirectory.FullName -MinimumMB 100)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -73,7 +73,7 @@ function Get-GatewayConfigState {
}
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}

$ProgressPreference = 'Continue'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ function Disable-SdnRasGatewayTracing {
return $object
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ function Enable-SdnRasGatewayTracing {
# ensure that the appropriate windows feature is installed and ensure module is imported
$config = Get-SdnModuleConfiguration -Role 'Gateway'
if (-NOT (Initialize-DataCollection -Role 'Gateway' -FilePath $config.properties.commonPaths.rasGatewayTraces -MinimumMB 250)) {
"Unable to initialize environment for data collection" | Trace-Output -Level:Exception
"Unable to initialize environment for data collection" | Trace-Output -Level:Error
return
}

Expand Down Expand Up @@ -36,6 +36,6 @@ function Enable-SdnRasGatewayTracing {
return $object
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
4 changes: 2 additions & 2 deletions src/modules/SdnDiag.Health/private/Test-EncapOverhead.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ function Test-EncapOverhead {
$sdnHealthObject.Result = 'FAIL'
$sdnHealthObject.Remediation += "Ensure EncapOverhead and JumboPacket for interface {0} on {1} are enabled and configured correctly." -f $interface.NetworkInterface, $object.Name

"EncapOverhead and JumboPacket for interface {0} on {1} are disabled or not configured correctly." -f $interface.NetworkInterface, $object.Name | Trace-Output -Level:Exception
"EncapOverhead and JumboPacket for interface {0} on {1} are disabled or not configured correctly." -f $interface.NetworkInterface, $object.Name | Trace-Output -Level:Error
}

$array += $interface
Expand All @@ -62,6 +62,6 @@ function Test-EncapOverhead {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ function Test-HostRootStoreNonRootCert {
foreach($nonRootCert in $nonRootCerts) {
$sdnHealthObject.Remediation += "Remove Certificate Thumbprint:{0} Subject:{1} from Host:{2}" -f $nonRootCert.Thumbprint, $nonRootCert.Subject, $node
}

$array += $object
}
}
Expand All @@ -61,6 +61,6 @@ function Test-HostRootStoreNonRootCert {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function Test-MuxBgpConnectionState {
foreach ($router in $peerRouters) {
$connectionExists = Invoke-PSRemoteCommand -ComputerName $virtualServerConnection -Credential $Credential -ScriptBlock $netConnectionExistsScriptBlock -ArgumentList $router
if (-NOT $connectionExists) {
"{0} is not connected to {1}" -f $virtualServerConnection, $router | Trace-Output -Level:Exception
"{0} is not connected to {1}" -f $virtualServerConnection, $router | Trace-Output -Level:Error
$sdnHealthObject.Result = 'FAIL'
$sdnHealthObject.Remediation += "Fix BGP Peering between $($virtualServerConnection) and $($router)."

Expand All @@ -70,6 +70,6 @@ function Test-MuxBgpConnectionState {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ function Test-NcHostAgentConnectionToApiService {
# in which case we should mark this test as failed and return back to the caller with guidance to fix the SlbManagerService
$primaryReplicaNode = Get-SdnServiceFabricReplica -NetworkController $SdnEnvironmentObject.EnvironmentInfo.NetworkController -ServiceTypeName 'ApiService' -Credential $NcRestCredential -Primary
if ($null -ieq $primaryReplicaNode) {
"Unable to return primary replica of ApiService" | Trace-Output -Level:Exception
"Unable to return primary replica of ApiService" | Trace-Output -Level:Error
$sdnHealthObject.Result = 'FAIL'
$sdnHealthObject.Remediation = "Fix the primary replica of ApiService within Network Controller."
return $sdnHealthObject
Expand All @@ -57,7 +57,7 @@ function Test-NcHostAgentConnectionToApiService {
[System.Array]$connectionAddress = Get-SdnServer -NcUri $SdnEnvironmentObject.NcUrl.AbsoluteUri -ResourceId $server.resourceId -ManagementAddressOnly -Credential $NcRestCredential
$connectionExists = Invoke-PSRemoteCommand -ComputerName $connectionAddress[0] -Credential $Credential -ScriptBlock $netConnectionExistsScriptBlock
if (-NOT $connectionExists) {
"{0} is not connected to ApiService of Network Controller" -f $server.resourceRef | Trace-Output -Level:Exception
"{0} is not connected to ApiService of Network Controller" -f $server.resourceRef | Trace-Output -Level:Error
$sdnHealthObject.Result = 'FAIL'
$sdnHealthObject.Remediation += "Ensure NCHostAgent service is started. Investigate and fix TCP connectivity or x509 authentication between $($primaryReplicaNode.ReplicaAddress) and $($server.resourceRef)."

Expand All @@ -77,6 +77,6 @@ function Test-NcHostAgentConnectionToApiService {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,6 @@ function Test-NcUrlNameResolution {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,6 @@ function Test-NetworkControllerCertCredential {
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ function Test-NetworkInterfaceAPIDuplicateMacAddress {
| Select-Object @{n="ResourceRef";e={"`t$($_.resourceRef)"}} `
| Select-Object -ExpandProperty ResourceRef `
| Out-String `
) | Trace-Output -Level:Exception
) | Trace-Output -Level:Error
}
}

$sdnHealthObject.Properties = $array
return $sdnHealthObject
}
catch {
"{0}`n{1}" -f $_.Exception, $_.ScriptStackTrace | Trace-Output -Level:Error
$_ | Trace-Exception
}
}
Loading

0 comments on commit 3082326

Please sign in to comment.