Unable to parse variable into the ForEach ($Server in $Servers) loop?

Question

I am trying to correct the below script to query the server logon session using Qwinsta.

The goal of this server is to show who is currently logged on.

The script main section:

$queryResults = (qwinsta /server:$ServerName | Select-Object -Skip 1 | ForEach-Object { (($_.trim() -replace "\s+", ",")) } | convertfrom-csv -ErrorAction Stop)

is working fine:

PS C:\WINDOWS\system32> $queryResults

services      0     Disc  
--------      -     ----  
console       1     Conn  
Administrator 3     Disc  
rdp-tcp       65536 Listen

Script:

$HtmlHead = @"
<style>
    body {
        font-family: Arial;
    }
    table {
        width: 100%;
        border-collapse: collapse;
        border: 1px solid;
    }
    th {
        background-color: green;
        border: 1px solid;
        padding: 1px;
    }
    td {
        border: 1px solid;
        padding: 1px;
    }
</style>
"@

$Today = Get-Date -Format 'F'
$SessionList = "`n`nRDP Session List - " + $Today + "`n`n"
$CurrentSN = 0

# Get a list of servers from Active Directory
write-progress -activity "Getting list of servers from Active Directory" -status "... please wait ..."

$Servers = Get-ADComputer -Filter { Enabled -eq $True -and OperatingSystem -like "*Server*" } -SearchBase "OU=Servers,DC=Company,DC=com," | Where-Object { Test-Connection $_.Name -Count 1 -Quiet } | Select-Object -ExpandProperty Name

$NumberOfServers = $Servers.Count

# Iterate through the retrieved list to check RDP sessions on each machine
ForEach ($Server in $Servers) {

    $ServerName = $Server.Name
    Write-Host "Processing $($Server.Name) ..." -ForegroundColor Yellow
    Write-progress -activity "Checking RDP Sessions" -status "Querying $ServerName" -percentcomplete (($CurrentSN / $NumberOfServers) * 100)

    # Run qwinsta and grab the output
    try {
        $queryResults = (qwinsta /server:$ServerName | Select-Object -Skip 1 | ForEach-Object { (($_.trim() -replace "\s+", ",")) } | convertfrom-csv -ErrorAction Stop)

        # get session info from the instance
        ForEach ($QueryResult in $QueryResults) {

            $RDPUser = $($QueryResult.substring(19, 22)).trim()
            $SessionType = $($QueryResult.substring(1, 18)).trim()
            $SessionID = $($QueryResult.substring(41, 5)).trim()
            $ReturnedCurrentState = $($QueryResult.substring(48, 8)).trim()

            $RDPUser = $QueryResult.USERNAME
            $SessionType = $QueryResult.SESSIONNAME
            $SessionID = $QueryResult.ID
            $ReturnedCurrentState = $QueryResult.State

            If ($ReturnedCurrentState -eq $null) { $CurrentState = "Disconnected" } Else { $CurrentState = "Active" }

            # filter out the irrelevant information
            If (($RDPUser -ne $null) -and ($SessionType -ne "console") -and ($SessionType -ne "services") -and ($SessionType -ne "rdp-tcp") -and ($RDPUser -ne "65536")) {
                $SessionList = $SessionList + "`n" + $ServerName + " logged in by " + $RDPUser + " on " + $SessionType + ", session id $SessionID $CurrentState"
            }
        }

    }
    catch {
        $SessionList = $SessionList + "`n Unable to query " + $ServerName
        write-host "Unable to query $ServerName!" -foregroundcolor Red
    }

    $CurrentSN++
}

# Send the output the screen.
$SessionList + "`n`n"

$sendMailArgs = @{
    From       = "$env:USERNAME@$env:userdnsdomain"
    To         = 'SOC@domain.com'
    SmtpServer = 'SMTP.domain.com'
    Priority   = 'High'
    BodyAsHtml = $true
    Body       = ($SessionList | ConvertTo-Html -Head $HtmlHead) -join "`r`n"
    Subject    = "$($SessionList.Count) Logged On users from $($NumberOfServers) online servers as at $($Today)"
}

Send-MailMessage @sendMailArgs

The problem is that the script loop is always going into:

    write-host "Unable to query $ServerName!" -foregroundcolor Red

The email result is always like:

*
424

What is that mean?

Answer 1

Most likely the Substring() method throws an exception because the startIndex parameter values of 41 and 48 seem large. If $QueryResult string ends up shorter, you will get that exception and immediately move to the catch statement.

I would recommend separating error handling for querying the server and building the session list.

UPDATE: Here's how you can separate error handling in your script example. Basically you need to be aware of methods or cmdlets which can theoretically throw an exception with the input parameters you provide and cover those cases.

ForEach ($Server in $Servers) {

    $ServerName = $Server.Name
    Write-Host "Processing $ServerName ..." -ForegroundColor Yellow
    Write-progress -activity "Checking RDP Sessions" -status "Querying $ServerName" -percentcomplete (($CurrentSN / $NumberOfServers) * 100)

    # Run qwinsta and grab the output
    try {
        $queryResults = (qwinsta /server:$ServerName | Select-Object -Skip 1 | ForEach-Object { (($_.trim() -replace "\s+", ",")) } | convertfrom-csv )
    }
    catch {
        # Error handling for failed server connection here
        $SessionList += "`n Unable to query " + $ServerName
        write-host "Unable to query $ServerName!" -foregroundcolor Red
        continue # Skip to next $Server
    }
    # get session info from the instance
    ForEach ($QueryResult in $QueryResults) {
        try {
            # Do something about this
            $RDPUser = $($QueryResult.substring(19, 22)).trim()
            $SessionType = $($QueryResult.substring(1, 18)).trim()
            $SessionID = $($QueryResult.substring(41, 5)).trim()
            $ReturnedCurrentState = $($QueryResult.substring(48, 8)).trim()
    
            $RDPUser = $QueryResult.USERNAME
            $SessionType = $QueryResult.SESSIONNAME
            $SessionID = $QueryResult.ID
            $ReturnedCurrentState = $QueryResult.State
        }
        catch {
            # Insert your error handling here
            # Write-Host "Failed to process query result from $ServerName!" -foregroundcolor Red
             
            Write-Host $Error[0].Exception # If you want to get the exception message. A shorter way to do it below
            # Write-Host $_
        }

        If ($null -eq $ReturnedCurrentState) { $CurrentState = "Disconnected" } Else { $CurrentState = "Active" }

        # filter out the irrelevant information
        If (($null -ne $RDPUser) -and ($SessionType -ne "console") -and ($SessionType -ne "services") -and ($SessionType -ne "rdp-tcp") -and ($RDPUser -ne "65536")) {
            $SessionList = $SessionList + "`n" + $ServerName + " logged in by " + $RDPUser + " on " + $SessionType + ", session id $SessionID $CurrentState"
        }
    }
    $CurrentSN++
} 

I am genuinely concerned about the code block where you assign values to variables like $RDPUser, $SessionType, etc. I'll give you the benefit of the doubt that I don't know your environment, but the logic looks very strange. If $QueryResult is an object with several properties, why substring? The object doesn't have this method. If it's just a long string with values at certain character positions, why try to get its property values like $QueryResult.ID - it doesn't have any.