Shoot. Made an error. Fixed in red below.
From: [email protected] [mailto:[email protected]] On
Behalf Of Michael B. Smith
Sent: Tuesday, April 26, 2016 4:52 PM
To: [email protected]
Subject: RE: [powershell] Try/Catch Format Question:
Get-Verb shows you all the approved verbs in your current version of PS.
So, and you can ask Web how much I harp on this, but I think error-reporting
tends to be underdone in PowerShell. I prefer a rich set of mechanisms to
protect yourself.
IMHO, it is doing error detection, reporting, and recovery that takes a
“useful” script and turns it into an enterprise-class script.
So…. At the top of a script:
$SaveEApref = $ErrorActionPreference
$ErrorActionPreference = ‘Stop’
For a given cmdlet call:
$error.Clear()
[bool]$failed = $false
[System.Exception]$e = $null
[object]$result = $null
try
{
$result = Get-AdUser
John-Jacob-JingleHeimer-Schmitt
$failed = !$?
}
catch
{
$failed = $true
$e = $_
}
if( $failed –or $error.Count –gt 0 )
{
## some error occurred
Write-Error “Cmdlet failed: Get-AdUser”
if( $null –ne $e )
{
Write-Error “Exception: $(
$e.ToString() )”
}
if( $error.Count –gt 0 )
{
Write-Error “Last error: $(
$error[ -1 ].ToString() )”
}
return 1 ## failure
}
## now we can use $result
…
return 0 ## success
## at end of script
$ErrorActionPreference = $SaveEAPref
My $0.02. YMMV. Void in the State of Wisconsin.
From: [email protected]<mailto:[email protected]>
[mailto:[email protected]] On Behalf Of Devin Rich
Sent: Tuesday, April 26, 2016 2:46 PM
To: [email protected]<mailto:[email protected]>
Subject: Re: [powershell] Try/Catch Format Question:
I have 2 opinions on the matter and I can say that I am not an expert, so feel
100% free to completely disregard the rest of this email.
First, Powershell generally operates on a Verb-Noun basis. When I read the
function name "Enable-UserFailed" I sit here and wonder how I am going to
'enable' the 'UserFailed'. To me, the verb Enable doesn't describe what the
function is doing and the noun 'UserFailed' doesn't describe what the verb is
acting on. I like to look at this page for a bunch of available verbs when
writing my function titles:
https://msdn.microsoft.com/en-us/library/ms714428(v=vs.85).aspx May I possibly
suggest Send or Out for your verb instead of enable (since these 2 functions
seem to be related to reporting and not actually enabling something). I would
also suggest changing your noun from UserSuccess\UserFailed to something like
UserResult, SuccessfulCreation, etc. Thus you could have a function titled
Send-FailedLyncUserCreation and I can surmise from that title that the function
will alert to somewhere about how creating the user in Lync had a problem.
Second, I use a try block as a conditional procedure block. If I want 2
commands to be attempted always, that I don't put them in the same try block
(since 1 failing could cause the other to fail). I'll either use 2 try blocks,
or maybe a try-catch-finally or just move the 2nd statement before the try. I
have loved this post about try-catch:
http://www.vexasoft.com/blogs/powershell/7255220-powershell-tutorial-try-catch-finally-and-error-handling-in-powershell
I have not personally done an if (!$Error) before, but it seems like that
could work if needed. I would suggest changing how the commands are
ordered\working to avoid it though.
I do think your code looks very good and clean. Keep it up! Don't forget to put
in some documentation before you are done so next time you don't have to try
and remember everything. :)
Devin Rich
Systems Administrator
On Tue, Apr 26, 2016 at 12:00 PM, Orlebeck, Geoffrey
<[email protected]<mailto:[email protected]>> wrote:
All:
I am working on a function for enabling users in Lync based off AD Group
memberships. I am interested in the best practice when attempting to have
try/catch perform an action only if there are no errors. I’ll include the
entire code at the end of the email, but what I am curious about is this
specific piece:
If($Email -ne $Null)
{
try
{
Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool -SipAddress
"sip:$($Email)" -ErrorAction Stop
Enable-UserSuccess
}
catch
{
Enable-UserFailed
}
}
Else
{
Enable-UserFailed -Issue Email
}
Should the Enable-UserSuccess be called outside of the try block like this?
If($Email -ne $Null)
{
try
{
Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool -SipAddress
"sip:$($Email)" -ErrorAction Stop
}
catch
{
Enable-UserFailed
}
If(!$Error)
{
Enable-UserSuccess
}
}
Else
{
Enable-UserFailed -Issue Email
}
My logic for changing to using the If(!$Error) statement is in the first
example if my event logging fails, then the entire try block fails, so the user
would not get enabled in Lync. If the user account can be created, I want it
created. The logging is there for myself/others, but is not critical to the
success of the function. Anyway, here is the full code, in case it’s necessary.
I apologize in advance for any formatting blunders. I’ve been reviewing the PS
Styling documentation on GitHub (for those
interested<https://github.com/PoshCode/PowerShellPracticeAndStyle/tree/master/Style%20Guide>)
and am trying to adhere to it, but still not 100% cleaned up pre-existing
scripts:
Function Enable-LyncUsers
{
Param(
# Group to parse members and enable in Lync
[Parameter(Mandatory=$true,
ValueFromPipeline=$true,
ValueFromPipelineByPropertyName=$true,
Position=0)]
[ValidateNotNull()]
[ValidateNotNullOrEmpty()]
$CSGroup,
# Registrar Pool to assign new Lync users
[Parameter(Mandatory=$false,
ValueFromPipeline=$true,
ValueFromPipelineByPropertyName=$true,
Position=1)]
[ValidateNotNull()]
[ValidateNotNullOrEmpty()]
[string]
$RegistrarPool =
"AHPFILER.aspirehealthplan.org<http://AHPFILER.aspirehealthplan.org>"
)
function Write-Log {
[cmdletbinding()]
Param(
[Parameter(Mandatory=$true,
Position=0)]
[string]
$Message
)
$LogDirectory =
"\\ahpmgmt\logs$\Lync\Enable_Users<file:///\\ahpmgmt\logs$\Lync\Enable_Users>"
$LogFile = "$(Get-Date -UFormat "%Y.%m.%d") - Enable Lync Users.log"
If(-not (Test-Path "$LogDirectory\$($LogFile)"))
{
New-Item -Path $LogDirectory -Name $LogFile -ItemType File
}
Add-Content -Path "$($LogDirectory)\$($LogFile)" -Value "$(Get-Date) |
$Message$newLine"
}
function Enable-UserFailed {
[cmdletbinding()]
Param(
[Parameter(Mandatory=$false,
Position=0)]
[string]
$Source = "Enable Lync User",
[Parameter(Mandatory=$false,
Position=0)]
[string]
$Issue
)
If(-Not [System.Diagnostics.EventLog]::SourceExists($Source))
{
[System.Diagnostics.EventLog]::CreateEventSource($Source,'Lync
Server')
}
If($Issue -eq "Email")
{
$Event = @{
LogName = "Lync Server"
Source = $Source
EventID = 8414
EntryType = "Warning"
Message = "Failed to enable user, email not found:
$($User.GivenName) $($User.Surname)"
}
Write-EventLog @Event
}
Else
{
$Event = @{
LogName = "Lync Server"
Source = $Source
EventID = 8414
EntryType = "Error"
Message = "Failed to enable user: $($User.GivenName)
$($User.Surname)"
}
Write-EventLog @Event
}
}
function Enable-UserSuccess {
[cmdletbinding()]
Param(
[Parameter(Mandatory=$false,
Position=0)]
[string]
$Source = "Enable Lync User"
)
If(-Not [System.Diagnostics.EventLog]::SourceExists($Source))
{
[System.Diagnostics.EventLog]::CreateEventSource($Source,'Lync
Server')
}
$Event = @{
LogName = "Lync Server"
Source = $Source
EventID = 8414
EntryType = "Information"
Message = "User Enabled: $($User.GivenName) $($User.Surname)"
}
Write-EventLog @Event
}
Import-Module ActiveDirectory
Import-Module Lync
$Members = Get-ADGroupMember $CSGroup -Recursive
Foreach($Member in $Members)
{
$Account = Get-CsUser -Identity $Member.SamAccountName -ErrorAction
SilentlyContinue
If($Account.Enabled -ne $True)
{
$User = Get-ADUser $Member.SamAccountName -Properties Mail
$Email = $User.Mail
If($Email -ne $Null)
{
try
{
Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool
-SipAddress "sip:$($Email)" -ErrorAction Stop
Enable-UserSuccess
Write-Log "Lync User Enabled: $($User.GivenName)
$($User.Surname)"
}
catch
{
Enable-UserFailed
Write-Log "Failed to enable user: $($User.GivenName)
$($User.Surname)"
}
}
Else
{
Enable-UserFailed -Issue Email
Write-Log "No email found for user: '$($User.GivenName)
$($User.Surname)'"
}
}
}
}
Confidentiality Notice: This is a transmission from Community Hospital of the
Monterey Peninsula. This message and any attached documents may be confidential
and contain information protected by state and federal medical privacy
statutes. They are intended only for the use of the addressee. If you are not
the intended recipient, any disclosure, copying, or distribution of this
information is strictly prohibited. If you received this transmission in error,
please accept our apologies and notify the sender. Thank you.
The information contained in this message is privileged, confidential, and
protected from disclosure. If you are not the intended recipient, you are
hereby notified that any review, printing, dissemination, distribution, copying
or other use of this communication is strictly prohibited. If you have received
this communication in error, please notify us immediately by replying to the
message and deleting it from your computer.
